cargo icon indicating copy to clipboard operation
cargo copied to clipboard

Additional SemVer compatibility items.

Open ehuss opened this issue 3 years ago • 6 comments

This is a dump of some elements that I considered adding to the SemVer compatibility chapter, but didn't have time or knowledge to finish.

  • [ ] Major: Moving existing code behind a feature flag. This would fail if someone is using no-default-features. #8997
    • Unchecked this. I think this could use a specific callout, and mention --no-default-features explicitly.
  • [x] Major: Moving a feature out of the default set if that changes functionality or public items. #8997
  • [ ] Stability of size-of, alignment-of, and hash of types.
  • [ ] Probably lots of things to say about lifetimes (currently no mentions of lifetimes!).
    • Change to method-bound lifetime. Example: Change from returning SomeType<'static> to fn some<'a>(i: &'a str) -> SomeType<'a>.
    •  fn foo<'a>(a: &'a str, b: &'a str) -> &'a str { todo!() }
      
       // to
      
       fn foo<'a, 'x>(a: &'a str, b: &'x str) -> &'a str { todo!() }
      
    • Possibly only allowed to increase lifetime in covariant positions?
  • [ ] https://github.com/rust-lang/rust-forge/blob/master/src/libs/maintaining-std.md#are-there-new-impls-for-stable-traits
  • [ ] What is allowed/disallowed with proc-macros and macro_rules?
  • [ ] Soundness fixes.
  • [ ] Changes to unsafe.
  • [ ] Go through attributes and figure out which ones matter for adding/removing/changing.
  • [ ] The stability RFC says any "non-trivial" change to a trait item signature, but doesn't define what a "trivial" change is.
  • [ ] Changes to function front-matter.
    • Adding "const" should be ok? Removing "const" is not.
  • [ ] Fundamental types, adding blanket trait impls, and the re-rebalancing RFC. There is a section on fundamental types that has changed as part of RFC 1023 and RFC 2451 that I don't fully understand. See also niko's blog.
  • [ ] Implementing any trait (https://rust-lang.github.io/rfcs/1105-api-evolution.html#minor-change-implementing-any-non-fundamental-trait), I think this is missing.
  • [ ] "Builder pattern" needs a link to something that explains what it is, but I don't think there are any official docs on it.
  • [ ] impl trait vs generic parameters. Converting between foo(x: impl Trait) and foo<T: Trait>(x: T).
  • [ ] -> impl trait return value converted to a concrete type -> Foo
  • [ ] Conversion between struct/enum/union: https://internals.rust-lang.org/t/pre-rfc-stable-rustdoc-urls/13099/
  • [ ] Major: Adding or removing some trait implementations. For example, Drop, Copy, Send, Sync, etc. I think removing any public trait is a breaking change? I believe adding Drop is a major change, not sure what the rules are there.
  • [ ] Add "adding a defaulted const generic parameter", and discuss how that prevents adding a defaulted type parameter in the future.
  • [ ] Adding a trait impl can introduce ambiguity, see https://internals.rust-lang.org/t/changing-public-api-by-adding-impl/14246
  • [ ] Switching an item to a type alias of the same item can sometimes be a breaking change (https://github.com/rust-lang/rust/issues/83248)
  • [ ] Changing the value of a const can be a breaking change (https://internals.rust-lang.org/t/should-we-be-more-strict-with-const-and-semver/14302).
    • [ ] Changing the value of an enum discriminant?
  • [ ] Maybe discuss the impact of adding this impl has: #75180
  • [ ] Check that these are covered: https://std-dev-guide.rust-lang.org/breaking-changes/new-trait-impls.html
  • [ ] Adding a viarant to a non_exhaustive enum if the enum is fieldless (can no longer cast it, etc).
  • [ ] Making a semver-major change to a dependency whose types (or other entities or impls) are publicly exported. This can cause a version incompatibility hazard.
  • [ ] Changing panic behavior (adding or removing a panic from a function for example).
  • [x] Mention that changes that affect lints are always allowed? Or should there be any nuance there? #11596
  • [ ] Adding or removing private fields of a struct can be a breaking change if those fields change the auto traits.
  • [ ] Removing private fields of a struct can be a breaking change if those fields are leaked through some other means. For example, if using serde with #[derive(Serialize)], private fields will leak in the serialized output.
  • [ ] this demonstrates an interesting failure with a change to a private field. struct B<T> { x: Box<T> } with a user having struct A { b: B<A> }. Change Box<T> to just T causes A to be infinitely sized.
  • [ ] To what degree are inference changes a breaking change? For example, I think maybe generic-generalize-identical or fn-generalize-compatible could potentially have inference issues (needs investigation, I believe struct and fn inference work differently).
  • [ ] Removing a feature from a dependency. This has some subtle impacts due to unification. Package A → Package B, removes the feature "f1" from B. Package C → Package A and B, and only builds correctly if Package B has "f1" enabled, but does not explicitly state that. Removing "f1" now causes Package C to fail to build when it does an update of A. I'm not really sure how this should be handled.
  • [ ] Removing a cargo target. For example, if you remove the [lib] target, that would break anything depending on that package. This will be more important with artifact-dependencies.
  • [ ] Maybe have a catch-all rule about changing the signature of an item or type is not allowed unless explicitly allowed by some other rule? Otherwise, would need to enumerate all examples, which could be quite a lot. For example, changing the arity of a tuple, or the size of an array, changing from a u16 to a u8, changing from one type to another, etc.
  • [ ] Namespace hazards. See https://users.rust-lang.org/t/semver-hazard-around-proc-macro-trait-symbols-ambiguity/72178 for an example. (Would be nice if rustc could detect reexports that are identical, but I think there are other non-identical cases that are hazards as well.)
  • [ ] SemVer compatibility downgrade: If you have a dependency, say with a req 1.5.2, and then you later publish a version that depends on 1.5.0, is that a breaking change? I think unlikely unless you re-export the dependency, in which case it then becomes an interesting question. Let's say 1.5.2 had a new item or type that wasn't in 1.5.0. It would be a breaking change because users could be trying to access those entities through the re-export, and they may be using something like minimal-versions which would break.
  • [ ] Compatibility of types of fields within a repr(packed) type. See https://github.com/rust-lang/rust/issues/115305#issuecomment-1696860270 and https://github.com/rust-lang/rust/pull/115315.
  • [ ] A seemingly unrelated trait impl broke inference: https://github.com/serde-rs/json/issues/357

As an alternative to documentation, we could make these lints in a tool like cargo-crate-api

ehuss avatar Sep 26 '20 16:09 ehuss

Another thing that might be worth mentioning is the standback crate in the MSRV bit. It does exactly what is recommended — copies code from stdlib — but also re-exports when possible based on the compiler version in use. I created it specifically for that reason, and have yet to run into any issues. Imports are essentially identical, as a bonus.

jhpratt avatar Sep 28 '20 10:09 jhpratt

  • Fundamental types, adding blanket trait impls, and the re-rebalancing RFC. There is a section on fundamental types that has changed as part of RFC 1023 and RFC 2451 that I don't fully understand.
  • Maybe discuss the impact of adding this impl has: #75180

#75180 is a blanket trait impl (and thus a major breaking change) as per RFC 2451: it's an impl for &T, but as & is fundamental, &T is not covered, and thus the impl is a blanket impl. This comment (and some of the crater results) illustrate how it can break existing downstream implementations.

QuineDot avatar Mar 25 '21 02:03 QuineDot

Another thing I believe is missing is updating dependencies whose types are part of your public API.

Skepfyr avatar Jun 03 '21 14:06 Skepfyr

We talked about this in the cargo team meeting and we are leaning towards automating this, see https://github.com/rust-lang/cargo/issues/374, like with cargo-crate-api

epage avatar Apr 21 '22 17:04 epage

I don't know if this falls under here (or if there's a duplicate issue or any prior discussion), but due to the way that the cargo resolver currently works removing a feature flag even from a non-public dependency appears to constitute a breaking change.

Consider crate a and b:

[package]
name = "a"
version = "1.0.0"

[dependencies]
uuid = { version = "1.0.0", features = ["serde"] }
[package]
name = "a"

[dependencies]
a = "1.0.0"
uuid = "1.0.0"

Crate b can now use uuid::Uuid as if it implements Serialize / Deserialize since the serde feature is transitively activated.

If crate a then changes their manifest to:

[package]
name = "a"
version = "1.0.1"

[dependencies]
uuid = "1.0.0"

Once crate b updates its dependency to crate a, uuid::Uuid no longers implements the Serialize / Deserialize traits and any code which assumed so stops compiling. Removing a dependency in a constitutes removing a constraint requiring any non-default features.

If we bring -Z min-versions into the mix, removing or relaxing a dependency has additional versioning hazards since it might potentially relax a minimal version constraint.

Example of breakage in the wild: tokio-rs/tokio#4663

udoprog avatar May 08 '22 21:05 udoprog

Compatibility of types of fields within a repr(packed) type. See https://github.com/rust-lang/rust/issues/115305#issuecomment-1696860270 and https://github.com/rust-lang/rust/pull/115315.

FWIW I don't think this should lead to a docs change; it should lead to a rustc change instead, similar to https://github.com/rust-lang/rust/pull/99020.

RalfJung avatar Sep 05 '23 19:09 RalfJung