Breakage related to where Self: Sized bounds on trait associated types and methods

[obi1kenobi]

Rust appears to allow trait items (so far, types and methods, in the future possibly also constants) to be excluded from trait objects by applying Self: Sized bounds like so:

pub trait Example {
    type Item where Self: Sized;
    
    fn method(&self) -> Self::Item where Self: Sized;
}

As far as I can tell, it is not required to repeat where Self: Sized on the items of trait implementations, so the breakage has to come from attempts to call or use the defined functionality, rather than being immediately caused by a definition lacking a where Self: Sized bound.

SemVer hazards:

• Adding where Self: Sized is generally a major breaking change for trait methods.
• Is removing where Self: Sized a major breaking change for trait methods? If so, how?
• Is adding or removing where Self: Sized a major breaking change for trait associated types by itself, meaning without a simultaneous where Self: Sized bound change on a trait method as well? If so, how?

Adding where Self: Sized to a trait method is breaking

pub trait Example {
    // If you uncomment the bound below:
    fn method(&mut self) -> Option<i64> // where Self: Sized;
}

fn demo(value: &mut dyn Example) {
    // then you get the following error on the next line:
    value.method();
    // error: the `method` method cannot be invoked on a trait object
    //   --> src/lib.rs:20:11
    //    |
    // 2  |     fn method(&mut self) -> Option<i64> where Self: Sized;
    //    |                                                     ----- this has a `Sized` requirement
    // ...
    // 20 |     value.method();
    //    |           ^^^^^^
}

playground

Caveat: if the method is made non-callable from outside its own crate (i.e. the trait is partially-sealed as described in this post), then this is not a major breaking change since no external crate could have called the method and been broken by its signature change.

This is currently not expressible in a Trustfall query, so it cannot be linted without further work on the query schema.

[obi1kenobi]

cc @ehuss this might be of interest — it appears to be a new set of SemVer hazards that might bear mentioning in the cargo reference's SemVer section.

[ehuss]

Thanks for the ping! Just to clarify, is this just a variant of trait-object-safety? That section explicitly says "adding an item", but could be extended to be "adding, or changing an existing item to be non-object-safe"?

Also, one of the todo items in rust-lang/cargo#8736 is to clarify what changes are allowed to a trait item. The old RFC just said no "non-trivial" changes, but didn't define what "trivial" meant. I think adding or removing trait bounds would by default be non-trivial, though I can see where there could be some subtle exceptions.

[obi1kenobi]

Just to clarify, is this just a variant of trait-object-safety?

I don't believe so. Even with the Self: Sized bound on trait Example's method, the trait itself remains object safe.

In other words, the following code compiles just fine: playground

pub trait Example {
    fn method(&mut self) -> Option<i64> where Self: Sized;
}

fn demo(value: &mut dyn Example) {}

Any other trait items that do not require Self: Sized would continue to be usable, so this could be extended to a realistic, non-trivial example as well.

Also, one of the todo items in rust-lang/cargo#8736 is to clarify what changes are allowed to a trait item. The old RFC just said no "non-trivial" changes, but didn't define what "trivial" meant. I think adding or removing trait bounds would by default be non-trivial, though I can see where there could be some subtle exceptions.

The edge cases are annoyingly difficult to pin down. I've tried and given up several times — it's a couple-week-long rabbit hole minimum, in my estimation.

For example, partially-sealed traits complicate everything. Methods that cannot be called from outside the trait's crate, or methods that cannot be overridden from outside the trait's crate, both make it quite complex to determine when changes to bounds could possibly affect a downstream crate.

This is one of the things I'd love to sit down and properly work out if I can find some funding to make it my full-time job for a few months. If the Rust project or Foundation might be willing to sponsor that work, I'd be happy to do it and even work out how to make the rules machine-checkable in cargo-semver-checks as well.

[QuineDot]

As requested. These examples don't change object safety but are still breaking changes.

Example 1: Removing Self: Sized from an associated type is a breaking change.

// Now that you can omit `where Self: Sized` associated types from `dyn Trait`,
// it is a breaking change to remove `Self: Sized` from associated types of
// object-safe traits
pub trait Trait {
    type Assoc where Self: Sized;
}

// If the `Self: Sized` bound on `Assoc` is removed, this must change to be
// `dyn Trait<Assoc = SomeConcreteType>`
pub fn example(_: &dyn Trait) {}

Example 2: removing Self: Sized from self: Self receiver methods is a breaking change.

pub trait Trait {
    fn consume(self) where Self: Sized;
    fn other(&self) {}
}

// Check that it's object safe
pub fn example(_: &dyn Trait) {}

// How do you implement `Trait` for non-sized types?
impl Trait for str {
    // We can't just omit the method (yet)...
    // https://github.com/rust-lang/rfcs/issues/2829
    
    // This errors because you can't pass unsized types by value...
    // fn consume(self) {}

    // This errors becuase trivial bounds isn't implemented yet...
    // https://github.com/rust-lang/rust/issues/48214
    // fn consume(self) where Self: Sized {}

    // But this workaround for trivial bounds works
    // (you still can't call the method)
    fn consume(self) where for<'a> Self: Sized {}
}

// But if you remove `where Self: Sized` from the trait, you will break the
// implementation because it is "no longer as general" as the trait.
//
// N.b. the reference says that a "receiver type of `Self` (i.e. `self`)
// implies [`where Self: Sized`]", but as this example demonstrates, that is
// not true.  There is some other special carve-out that leaves traits that
// have methods with `self` receivers object safe.
//
// https://doc.rust-lang.org/nightly/reference/items/traits.html#object-safety
//
// I.e. also comment out `impl Trait for str` and the `&dyn Trait` will still
// compile.

(As far as I know, there is currently no way to implement a trait with a self: Self receiver that lacks a where Self: Sized bound for unsized types. Perhaps suggesting the where Self: Sized bound should be a lint...)

[obi1kenobi]

Amazing, thank you! A few follow-up questions to make sure I understand all the nuances and can implement the lints accurately:

• Example 1 shows breakage from removing the bound from an associated type. Is adding the bound to an associated type also breaking in some way?
• In Example 1, is the requirement to explicitly use dyn Trait<Assoc = SomeConcreteType> instead of just dyn Trait coming from a linguistic necessity, or from a shortcoming of existing tooling? As you know, Rust's SemVer rules say that breakage from tooling shortcomings are not major. Inference failures that require explicitly specifying types are specifically called out in the API evolution RFC as breaking but non-major, and I'd like to figure out if this one might be in that same bucket too.
• In Example 2, the breakage you showed depend on the trait being implemented in a downstream crate. Does that mean that if the trait were sealed, removing the Self: Sized bound would not be breaking? I.e. should the lint be "removed trait method bound" or "removed trait method bound on non-sealed trait"?
• While following the links in the excellent example 2 code, I came across this recently-merged change that's slated for Rust 1.80. Am I understanding correctly that as of that merged PR, any kind of where bound on trait methods excludes the method from the dyn-callable portion of the trait? In other words, I believe the new rule is "if the trait method uses where, you can't call it on dyn Trait."

[QuineDot]

• If you add the bound, the associated type is no longer defined for unsized implementors, and that is a breaking change.

  // Uncomment the two `Self: Sized` lines and `main` will break
  
  trait Trait {
      type Assoc: Default
      where
          //Self: Sized,
      ;
  }
  
  impl Trait for str {
      type Assoc = ()
      where
          // This workaround is needed for associated types too...
          //for<'a> Self: Sized,
      ;
  }
  
  fn main() {
      // ...but the workaround doesn't make the associated type usable
      let _ = <<str as Trait>::Assoc>::default();
  }

• It's linguistic necessity, both due to the reasoning here and the fact that things like method ABI and return types differ between erased base types with different associated types.

• Example 2 breaks all unsized implementors, local or downstream. If the trait is sealed and all implementations were for Sized types, I don't think the example matters. If there were implementations for unsized types, they must have also removed those. That's a breaking change on its own, but perhaps you detect that independently.

• It's definitely not "all where clauses opt the method out of being dyn-dispatchable". It looks closer to "where TypeInvolvingSelf: NonAutoTrait make a method non-object safe". (The PR author would be a better person to ask about the precise rules.) So now a where bound involving Self in that way on a method is like having a generic type parameter or using Self in a non-receiver argument or in return position -- your trait is not object safe unless you opt the method out of dyn-dispatch by also adding where Self: Sized.

  Note that this is more targeted than merely mentioning Self. where clauses on associated types are unaffected, for example. It also doesn't apply to lifetime bounds. Or auto-trait bounds. (Again, you should ask the PR author for the precise details.) Example:

  #![deny(where_clauses_object_safety)]
  use core::fmt::Debug;
  
  trait Trait {
      type Assoc;
      
      // These are object safe (before and after the PR)
      fn foo_1(&self) where Self: Send;
      fn foo_2(&self) where Self::Assoc: Debug;
      fn foo_3<'a>(&self, _: &'a str) where Self: 'a;
  
      // This is no longer object safe after the PR
      // fn bar(&self) where Self: Debug;
  
      // You can still opt it out of `dyn`-dispatch with `Self: Sized`
      // so that the trait as a whole is object safe
      fn bar(&self) where Self: Debug + Sized;
  }
  
  fn example(_: &dyn Trait<Assoc = ()>) {}

  (I'm also assuming the PR matches the lint, since the PR is not in the Playground Beta or Nightly apparently.)