Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breakage caused by refined impls (RFC 3245, RFC 3425) #816

Open
2 tasks
obi1kenobi opened this issue Jul 13, 2024 · 0 comments
Open
2 tasks

Breakage caused by refined impls (RFC 3245, RFC 3425) #816

obi1kenobi opened this issue Jul 13, 2024 · 0 comments
Labels
A-lint Area: new or existing lint

Comments

@obi1kenobi
Copy link
Owner

obi1kenobi commented Jul 13, 2024

Not ready to implement yet — blocked until it becomes stable Rust.


Examples of the feature:

trait Iterable {
    fn iter(&self) -> impl Iterator;
}

impl<T> Iterable for MyVec<T> {
    #[refine]
    fn iter(&self) -> impl Iterator + ExactSizeIterator { ... }
}
trait Sink {
    fn consume(&mut self, input: impl Iterator + ExactSizeIterator);
}

impl Sink for SimpleSink {
    #[refine]
    fn consume(&mut self, input: impl Iterator) { ... }
}

Finally, methods marked unsafe in traits can be refined as safe APIs, allowing code to call them without using unsafe blocks.

Breaking changes

Removing #[refine] from the impl of a trait method

  • implemented?

This could also be caught as "trait method changed its signature", but we probably want our diagnostic to reference #[refine] for ergonomics reasons.

Witness: Callers that relied on the refinement will be broken.

Removing a refinement from a trait method impl in general, without removing #[refine]

  • implemented?

For example, the following change:

trait PassThrough {
    fn consume(&mut self, input: impl Iterator + ExactSizeIterator) -> impl Iterator;
}

// Originally, refining the `input` argument:
impl PassThrough for OldStruct {
    #[refine]
    fn consume(&mut self, input: impl Iterator) -> impl Iterator { ... }
}

// Now, refining only the return type.
// Any callers relying on the refinement of `input` are broken.
impl PassThrough for NewStruct {
    #[refine]
    fn consume(&mut self, input: impl Iterator + ExactSizeIterator) -> impl Iterator + ExactSizeIterator { ... }
}

N.B.: We want to ensure there's no overlap with the above case, so the lint should enforce that #[refine] is still present or else we defer to the earlier case.

This applies whether the removed refinement is on a function parameter, or on the return type.

A general-case lint here would require #149, which is very hard. As an intermediate stepping stone, we could check whether parameters / return type got refined at all, without checking the precise nature of the refinement. That would miss cases like "return type refinement changed from impl Iterator + ExactSizeIterator to impl Iterator + DoubleEndedIterator". But it would also be much easier to implement and might not require #149.

Notes:

  • We treat "changing a refinement" as a combination of "adding" and "removing a refinement."
  • We must be careful of refinement + changes from async fn to -> impl Future. The latter by itself is not necessarily breaking.
  • Moving away from refining to a concrete type is still almost always breaking, since the concrete type likely has pub API items of its own (fields, variants, methods, other traits it impls, etc.) that will no longer be usable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: new or existing lint
Projects
None yet
Development

No branches or pull requests

1 participant