1.83 makes removing certain trait bounds a breaking change

language design
1

The change in 1.83

Consider this example:

pub trait Trait
where
    // Has always been implied
    Self::Assoc: Iterator,
    // Used to not be implied, but is now implied
    <Self::Assoc as Iterator>::Item: Clone,
{
    type Assoc;
}

The below was a non-breaking change pre-1.83, but is now a breaking change:

 where
     Self::Assoc: Iterator,
-    <Self::Assoc as Iterator>::Item: Clone,

It used to be non-breaking because downstream had to repeat the bound. Now that it's implied, downstream does not have to repeat the bound, and can implicitly rely on it.

This is the PR. It was not announced nor mentioned in the release notes.

Bounds on the associated types of supertraits

The change seems to be pretty narrowly focused, but I still find it concerning. Let me set the stage for why by talking about bounds on the associated types of supertraits. Note: as far as my testing goes, these were not affected by the change in 1.83 (but I'm not completely confident about what all was affected).

Consider this arrangement:

pub trait Trait {
    type Assoc;
}

pub trait SubTrait: Trait
where
    // Not implied (consumers must repeat the bound)
    Self::Assoc: Clone,
{}

Today, the SubTrait maintainer can make either of these non-breaking changes:

// Remove the bound (commit to never having it back)
pub trait SubTrait: Trait {}

// Make the bound implied (commit to never removing it) [MSRV: 1.79]
pub trait SubTrait: Trait<Assoc: Clone> {}

That is, they have three choices regarding the Assoc: Clone bound.

  1. Don't have the bound (breaking change to add it)
  2. Have the bound but not implied (non-breaking to drop it or to make it implied)
  3. Have the bound in implied form (breaking change to remove it or make it non-implied)

(1) and (3) are commitments, but you can go from (2) to either (1) or (3).[1] (3) has only been possible since 1.79, so you could consider (2) to be the default position; all stable pre-1.79 code with supertrait associated type bounds is in the uncommitted (2) camp.

The ability to go from (2) to (1) in a non-breaking way has been present since Rust 1.0.

Why the change is concerning

If bounds such as the one in the first SubTrait snippet were to become implied, Rust would be committing everyone in camp (2) to keeping the bound without them having a choice in the matter. Effectively camp (2) is deleted and everyone in it is moved to camp (3). There is no longer a code option which is not a commitment.[2]

Moreover, I consider the current situation with associated type bounds on supertraits to be a kind of test run of additional implied bounds elsewhere. And... I am pleased with how it has turned out! So far that is: I'm pleased that maintainers have the choice between (1), (3), or "by default" (2) -- "I'll decide later".

Making the removal of bounds a breaking change is a major downside of implied bounds generally. Maintainers should have to opt in to such a commitment (or over an edition, have the ability to opt out of the commitment). Or in other words, maintainers should have the choice to not hard commit to the bounds.

That is why the change in 1.83 concerns me (especially given how "silently" the change was made). It implies that the choice may be taken away from maintainers, committing them to their current bounds. Both with the associated type supertrait bounds we have today, and with whatever new implied bounds may come in the future.

I'm making this post to draw attention to those future possibilities (in addition to the 1.83 change).

  1. MSRV permitting ↩︎

  2. And if you want to commit to not having the bound, you have to do it before the change in implied bounds lands. ↩︎

15 Likes
2

I think this was always breaking. Consider:

fn clone_first<T: Trait>(iter: T::Assoc) -> Option<T::Assoc::Item> {
  Some(iter.first()?.clone())
}

Nope, I’m wrong, this one also required you to explicitly guarantee Clone in the signature of clone_first.

3

Well, it will once/if my PR Implement consecutive shorthand projections (associated type paths) like `T::AssocA::AssocB` by fmease · Pull Request #126651 · rust-lang/rust · GitHub lands (needs polish and FCP). Not really relevant actually.

4
Please ignore

I don't quite understand? Does that comment suggest it's acceptable to remove any supertrait bounds? If so, that's simply not true. I'm sure I'm missing something.

// upstream
pub trait Trait: SuperTrait { } // <-- can't remove `SuperTrait` backward compatibly
pub trait SuperTrait { type Assoc; } // or any kind of associated item

// downstream
fn f<T: Trait>() { let _: T::Assoc; } // or
fn g<T: Trait<Assoc:>>() {} // or …

Edit: Ah, I missed that it was part of the other example.

5

IMO, it would be reasonable if

pub trait Trait
where
    Self::Assoc: Iterator,
    <Self::Assoc as Iterator>::Item: Clone,
{
    type Assoc;
}

was changed back to its original behavior; but

pub trait Trait
where
    Self::Assoc: Iterator<Item: Clone>,
{
    type Assoc;
}

was to keep the new implying behavior (even though between 1.79-1.82 it hasn't been that way), because it's very unlikely that someone wrote a trait in that time - using the new Iterator<Item: Clone>-syntax - whilst deliberately trying to avoid the additional bound to be implied;
and also, it would help keep some of the consistency that's being restored, like

pub trait Trait {
    type Assoc: Iterator<Item: Clone>;
}

works that way since 1.79, and before that, it used to be the case that

pub trait Trait {
    type Assoc: …something…;
}

and

pub trait Trait
where
    Self::Assoc: …something…,
{
    type Assoc;
}

were always equivalent.

The distinction of

pub trait Trait
where
    Self::Assoc: Iterator,
    <Self::Assoc as Iterator>::Item: Clone,
{
    type Assoc;
}

vs

pub trait Trait
where
    Self::Assoc: Iterator<Item: Clone>,
{
    type Assoc;
}

would also be consistent with the distinction of

pub trait Trait
where
    Self: Iterator,
    <Self as Iterator>::Item: Clone,
{
}

vs

pub trait Trait
where
    Self: Iterator<Item: Clone>,
{
}
6 Likes
6

I have to say this give me "being consistently inconsistent" vibe. I would really expect these two are exactly equivalent. It seems really hard to teach :frowning:.

To OP: I think the best case scenario is that: we don't need to spell out the additional where bounds, but we can't rely on it either (other than proving T: Trait itself). This can be extended to all implied bounds.

10 Likes
7

especially given how "silently" the change was made

As far as I understand from PR thread, no one knew about this potential breaking. Everyone agreed, it went through fcp with everyone involved believing that this is positive change without downsides like this. So I don't see any problem with "silence", as there were nothing to communicate.

Implied bounds feature has a lot of upsides, some amount of sacrifice is acceptable imho. It depends on how many are there examples of maintainers actually using that opportunity to remove the bound.

Additionally, maybe some sort of mechanism to opt out of implicit behavior would help? For example, putting #[not_imply_bounds] on top of the trait declaration.

1 Like
8

In my opinion, <Self::Assoc as Iterator>::Item: Clone is the same in spirit as Self::Assoc: Iterator, so I think it makes sense that it's treated the same.

I do agree that it should have been in the release notes.

2 Likes
9

Well, the rationale is very simple: Trait<Item: Bound> syntax is new, so it gets to have the new implied-bounds meaning.

Perhaps just like -> impl Trait in traits was new, so it got to have the new lifetime-capturing meaning. In other words: a different implicit default meaning is reasonable, but should only be introduced over an edition.

Yes, but only in a new edition. Opt-out then can be a migration strategy for traits that want to keep the old behavior.

Because enabling impliedness on a given trait non-breaking change. Disabling impliedness is a breaking change. Without edition change, or forcing breaking changes upon users (or creating breaking changes in existing version sets that used to be properly semver-compliant), an opt-in the the proper option, not an opt-out.

It's common to use this opportunity in the general context of “implied bounds everywhere”, for structs. In particular, users that might start out with some

struct Foo<T: Bar> { … }

might later realize it’s a good idea to move this T: Bar to only the relevant methods of Foo that need the T: Bar. This is okay, and not a breaking change. Making Bar always implied is bad, because then it would be a breaking change, breaking users like

fn foo<T>(_: Foo<T>) { <T as Bar>::some_method() }

To be fair, I'm not sure whether or not this is actually realistically a thing that crates would have been doing for traits (rather than for structs). But there's still some concerns:

  • automatically implied bounds everywhere only for traits might be a reasonable end-goal, but that would be a deliberate design decision, and making where clauses for structs and traits behave differently
  • whether or not there are instances of such changes (lifting where restrictions on traits) being made – with the understanding that this isn't breaking – is a question one can't answer by just thinking about it. And also it's not a question that a normal crater run will answer, either (AFAIK). We would need some sort of analysis that runs on semver-compatible version pairs.
2 Likes
10

I just noticed that these also work through simplifying other associated-type projections, unlike any other of the "implied/uplifted" bounds to compare to. In code:

trait Trait
where
    Self: Other, // implied
{
    type Assoc;
}

(playground)

trait Indirectly {
    type This: ?Sized;
}
impl<T: ?Sized> Indirectly for T {
    type This = T;
}

trait Trait
where
    <Self as Indirectly>::This: Other, // not implied
{
    type Assoc;
}

(playground)

trait Trait
where
    Self::Assoc: Other, // implied
{
    type Assoc;
}

(playground)

trait Trait
where
    <Self::Assoc as Indirectly>::This: Other, // not implied
{
    type Assoc;
}

(playground)

trait Intermediate {
    type Deep;
}

trait Trait
where
    <Self::Assoc as Intermediate>::Deep: Other, // implied (change in 1.83!)
{
    type Assoc: Intermediate;
}

(playground)

trait Trait
where
    // still implied!! (change in 1.83!)
    <<Self::Assoc as Intermediate>::Deep as Indirectly>::This: Other,
{
    type Assoc: Intermediate;
}

(playground)

trait Trait
where
    // still implied!! (change in 1.83!)
    <<Self::Assoc as Indirectly>::This as Intermediate>::Deep: Other,
{
    type Assoc: Intermediate;
}

(playground)

trait Trait
where
    // still implied!! (change in 1.83!)
    <<<Self::Assoc as Indirectly>::This as Intermediate>::Deep as Indirectly>::This: Other,
{
    type Assoc: Intermediate;
}

(playground)

In the context of @quinedot's original example, this means that (perhaps surprisingly) there's inconsistency between

pub trait Trait
where
    // Used to not be implied, but is now implied
    <Self::Assoc as IntoIterator>::Item: Clone,
    // Is not implied
    <Self::Assoc as IntoIterator>::IntoIter: Clone,
{
    type Assoc: Iterator;
}
3 Likes
11

There really are interesting tradeoffs here, as to what bounds "should" be implied. Gating new implications on edition boundaries and/or choice of syntax is a good conservative approach, but it means users need to hit the extra roadbump of needing to (by straightforward machine applicable compiler error) add the extra bound depending on when the library was published.

Traits are very much a contract between implementer and consumer, so I'd err on the side of thinking that traits very rarely have unimplied bounds that could be removed; if they didn't make semantic sense, why would the author include the bounds, when use sites still have to repeat the bound if they want to use it anyway?

Whereas struct generics can get some "unnecessary" bounds much more easily. Either improperly added as a bound for derived impls, or for functionality used in Drop. But otoh, if a trait bound is structurally required (e.g. used for a pub type projection), then implying it makes sense, since the bound cannot be removed compatibly[1].

TL;DR: agree.

  1. Without a convoluted replacement bound that is essentially equivalent. ↩︎

1 Like
12

I had a footnote that didn't make it into the post that was along the lines of

An attribute would be better than a different syntax, but we'll take what we can get

So I think we're in agreement here: something more explicit would be better. But like @steffahn said, for the existing functionality, it would need to be introduced over an edition to avoid taking away the choice.

Hmm. What does it mean to rely on it, though. For example, if you leave off the bound, can you call method?

pub trait Sub: Super where <Self as Super>::Assoc: Clone {
    fn method(&self, assoc: &Self::Assoc) {
        self.some_other_method(assoc.clone());
    }
    fn bar(&self) { /* ... */ }
}

If so, this is a breaking change.

pub trait Sub: Super {
    fn method(&self, assoc: &Self::Assoc) where <Self as Super>::Assoc: Clone {
        self.some_other_method(assoc.clone());
    }
    fn bar(&self) { /* ... */ }
}

("You can't call method because it uses the bound" isn't an adequate alternative IMO. I don't think anyone wants that kind of analysis anyway, but it would also making changing the default body to make use of the bound a breaking change.)

13

Of course it's more complicated than I thought.

My instinct says "yes, you can call method", and "yes, of course adding a new bound is a breaking change". (I know technically it's less restricting than before, but method more restrict "out of context".)

14

I agree with this analysis.

However, I also agree that there are cases where a library might want to loosen a trait’s bounds, and it should be possible to do it backward-compatibly. Which is a primary motivation behind my RFC 3437.