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

Additional SemVer compatibility items. #8736

Open
2 of 43 tasks
ehuss opened this issue Sep 26, 2020 · 6 comments
Open
2 of 43 tasks

Additional SemVer compatibility items. #8736

ehuss opened this issue Sep 26, 2020 · 6 comments
Labels
A-documenting-cargo-itself Area: Cargo's documentation A-new-subcommand Area: new subcommand A-semver Area: semver specifications, version matching, etc. S-triage Status: This issue is waiting on initial triage.

Comments

@ehuss
Copy link
Contributor

ehuss commented Sep 26, 2020

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. Stabilize -Zfeatures and -Zpackage-features. #8997
    • Unchecked this. I think this could use a specific callout, and mention --no-default-features explicitly.
  • Major: Moving a feature out of the default set if that changes functionality or public items. Stabilize -Zfeatures and -Zpackage-features. #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 (Support importing enum variants through type aliases rust#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).
  • Mention that changes that affect lints are always allowed? Or should there be any nuance there? Add semver rule for lints #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 Handling of packed fields causes semver hazard: layout details from other crates affect type-checking rust#115305 (comment) and closure field capturing: don't depend on alignment of packed fields rust#115315.
  • A seemingly unrelated trait impl broke inference: serde_json 1.0.3 broke Diesel's build serde-rs/json#357
  • Expanding the set of "used" things in impl trait (via use<>) can be a breaking change. discussion
  • Mention inferred outlives rules described here? https://github.com/rust-lang/rfcs/blob/master/text/2093-infer-outlives.md#impact-on-semver

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

@ehuss ehuss added the A-documenting-cargo-itself Area: Cargo's documentation label Sep 26, 2020
@jhpratt
Copy link
Member

jhpratt commented Sep 28, 2020

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.

@ehuss ehuss added the A-semver Area: semver specifications, version matching, etc. label Mar 20, 2021
@QuineDot
Copy link

  • 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.

@Skepfyr
Copy link

Skepfyr commented Jun 3, 2021

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

@epage epage added the A-new-subcommand Area: new subcommand label Mar 30, 2022
@epage
Copy link
Contributor

epage commented Apr 21, 2022

We talked about this in the cargo team meeting and we are leaning towards automating this, see #374, like with cargo-crate-api

@udoprog
Copy link

udoprog commented May 8, 2022

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

@RalfJung
Copy link
Member

RalfJung commented Sep 5, 2023

Compatibility of types of fields within a repr(packed) type. See rust-lang/rust#115305 (comment) and rust-lang/rust#115315.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation A-new-subcommand Area: new subcommand A-semver Area: semver specifications, version matching, etc. S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

No branches or pull requests

7 participants