Lazily normalize inside trait ref during orphan check & consider ty params in rigid alias types to be uncovered #117164
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
fmease
commented
Oct 25, 2023
| LL | impl<T> foreign::Trait<Local, T, ()> for <T as Identity>::Output {} | ||
| | ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`) | ||
| | | ||
| = note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local, and no uncovered type parameters appear before that first local type |
There was a problem hiding this comment.
While I could update the diagnostic to say something like
Suggested change
| = note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local, and no uncovered type parameters appear before that first local type | |
| = note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local, and no type parameters not covered by a nominal type appear before that first local type |
or
- error[E0210]: type parameter `T` must be covered by another type
+ error[E0210]: type parameter `T` must be covered by a nominal typewe could instead change the definition of "covered" (e.g., in the Rust Reference) to reflect that the covering type has to be nominal for the contained type to be considered covered.
fmease
added
the
T-types
This comment has been minimized.
This comment has been minimized.
fmease
commented
Oct 25, 2023
lcnr
reviewed
Oct 26, 2023
fmease
added
S-waiting-on-author
This comment was marked as resolved.
This comment was marked as resolved.
fmease
force-pushed
the
orphan-norm
branch
2 times, most recently
from
f22fadd
to
719c089
Compare
fmease
changed the title
This comment has been minimized.
This comment has been minimized.
fmease
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
fmease
force-pushed
the
orphan-norm
branch
2 times, most recently
from
878f995
to
f5df0d1
Compare
This comment has been minimized.
This comment has been minimized.
fmease
added
S-waiting-on-author
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 1, 2024
… r=lcnr Cleanup: Replace item names referencing GitHub issues or error codes with something more meaningful **lcnr** in rust-lang#117164 (review): > […] while I know that there's precendent to name things `Issue69420`, I really dislike this as it requires looking up the issue to figure out the purpose of such a variant. Actually referring to the underlying issue, e.g. `AliasMayNormToUncovered` or whatever and then linking to the issue in a doc comment feels a lot more desirable to me. We should ideally rename all the functions and enums which currently use issue numbers. I've grepped through `compiler/` like crazy and think that I've found all instances of this pattern. However, I haven't renamed `compute_2229_migrations_*`. Should I? The first commit introduces an abhorrent and super long name for an item because naming is hard but also scary looking / unwelcoming names are good for things related to temporary-ish backcompat hacks. I'll let you discover it by yourself. Contains a bit of drive-by cleanup and a diag migration bc that was the simplest option. r? lcnr or compiler
|
Finished benchmarking commit (f705de5): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 672.826s -> 673.884s (0.16%) |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 2, 2024
…er-errors stabilize `-Znext-solver=coherence` r? `@compiler-errors` --- This PR stabilizes the use of the next generation trait solver in coherence checking by enabling `-Znext-solver=coherence` by default. More specifically its use in the *implicit negative overlap check*. The tracking issue for this is rust-lang#114862. ## Background ### The next generation trait solver The new solver lives in [`rustc_trait_selection::solve`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_trait_selection/src/solve/mod.rs) and is intended to replace the existing *evaluate*, *fulfill*, and *project* implementation. It also has a wider impact on the rest of the type system, for example by changing our approach to handling associated types. For a more detailed explanation of the new trait solver, see the [rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/solve/trait-solving.html). This does not stabilize the current behavior of the new trait solver, only the behavior impacting the implicit negative overlap check. There are many areas in the new solver which are not yet finalized. We are confident that their final design will not conflict with the user-facing behavior observable via coherence. More on that further down. Please check out [the chapter](https://rustc-dev-guide.rust-lang.org/solve/significant-changes.html) summarizing the most significant changes between the existing and new implementations. ### Coherence and the implicit negative overlap check Coherence checking detects any overlapping impls. Overlapping trait impls always error while overlapping inherent impls result in an error if they have methods with the same name. Coherence also results in an error if any other impls could exist, even if they are currently unknown. This affects impls which may get added to upstream crates in a backwards compatible way and impls from downstream crates. Coherence failing to detect overlap is generally considered to be unsound, even if it is difficult to actually get runtime UB this way. It is quite easy to get ICEs due to bugs in coherence. It currently consists of two checks: The [orphan check] validates that impls do not overlap with other impls we do not know about: either because they may be defined in a sibling crate, or because an upstream crate is allowed to add it without being considered a breaking change. The [overlap check] validates that impls do not overlap with other impls we know about. This is done as follows: - Instantiate the generic parameters of both impls with inference variables - Equate the `TraitRef`s of both impls. If it fails there is no overlap. - [implicit negative]: Check whether any of the instantiated `where`-bounds of one of the impls definitely do not hold when using the constraints from the previous step. If a `where`-bound does not hold, there is no overlap. - *explicit negative (still unstable, ignored going forward)*: Check whether the any negated `where`-bounds can be proven, e.g. a `&mut u32: Clone` bound definitely does not hold as an explicit `impl<T> !Clone for &mut T` exists. The overlap check has to *prove that unifying the impls does not succeed*. This means that **incorrectly getting a type error during coherence is unsound** as it would allow impls to overlap: coherence has to be *complete*. Completeness means that we never incorrectly error. This means that during coherence we must only add inference constraints if they are definitely necessary. During ordinary type checking [this does not hold](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01d93b592bd9036ac96071cbf1d624a9), so the trait solver has to behave differently, depending on whether we're in coherence or not. The implicit negative check only considers goals to "definitely not hold" if they could not be implemented downstream, by a sibling, or upstream in a backwards compatible way. If the goal is is "unknowable" as it may get added in another crate, we add an ambiguous candidate: [source](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L858-L883). [orphan check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L566-L579 [overlap check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L92-L98 [implicit negative]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L223-L281 ## Motivation Replacing the existing solver in coherence fixes soundness bugs by removing sources of incompleteness in the type system. The new solver separately strengthens coherence, resulting in more impls being disjoint and passing the coherence check. The concrete changes will be elaborated further down. We believe the stabilization to reduce the likelihood of future bugs in coherence as the new implementation is easier to understand and reason about. It allows us to remove the support for coherence and implicit-negative reasoning in the old solver, allowing us to remove some code and simplifying the old trait solver. We will only remove the old solver support once this stabilization has reached stable to make sure we're able to quickly revert in case any unexpected issues are detected before then. Stabilizing the use of the next-generation trait solver expresses our confidence that its current behavior is intended and our work towards enabling its use everywhere will not require any breaking changes to the areas used by coherence checking. We are also confident that we will be able to replace the existing solver everywhere, as maintaining two separate systems adds a significant maintainance burden. ## User-facing impact and reasoning ### Breakage due to improved handling of associated types The new solver fixes multiple issues related to associated types. As these issues caused coherence to consider more types distinct, fixing them results in more overlap errors. This is therefore a breaking change. #### Structurally relating aliases containing bound vars Fixes rust-lang#102048. In the existing solver relating ambiguous projections containing bound variables is structural. This is *incomplete* and allows overlapping impls. These was mostly not exploitable as the same issue also caused impls to not apply when trying to use them. The new solver defers alias-relating to a nested goal, fixing this issue: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Trait {} trait Project { type Assoc<'a>; } impl Project for u32 { type Assoc<'a> = &'a u32; } // Eagerly normalizing `<?infer as Project>::Assoc<'a>` is ambiguous, // so the old solver ended up structurally relating // // (?infer, for<'a> fn(<?infer as Project>::Assoc<'a>)) // // with // // ((u32, fn(&'a u32))) // // Equating `&'a u32` with `<u32 as Project>::Assoc<'a>` failed, even // though these types are equal modulo normalization. impl<T: Project> Trait for (T, for<'a> fn(<T as Project>::Assoc<'a>)) {} impl<'a> Trait for (u32, fn(&'a u32)) {} //[next]~^ ERROR conflicting implementations of trait `Trait` for type `(u32, for<'a> fn(&'a u32))` ``` A crater run did not discover any breakage due to this change. #### Unknowable candidates for higher ranked trait goals This avoids an unsoundness by attempting to normalize in `trait_ref_is_knowable`, fixing rust-lang#114061. This is a side-effect of supporting lazy normalization, as that forces us to attempt to normalize when checking whether a `TraitRef` is knowable: [source](https://github.com/rust-lang/rust/blob/47dd709bedda8127e8daec33327e0a9d0cdae845/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L754-L764). ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait IsUnit {} impl IsUnit for () {} pub trait WithAssoc<'a> { type Assoc; } // We considered `for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit` // to be knowable, even though the projection is ambiguous. pub trait Trait {} impl<T> Trait for T where T: 'static, for<'a> T: WithAssoc<'a>, for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit, { } impl<T> Trait for Box<T> {} //[next]~^ ERROR conflicting implementations of trait `Trait` ``` The two impls of `Trait` overlap given the following downstream crate: ```rust use dep::*; struct Local; impl WithAssoc<'_> for Box<Local> { type Assoc = (); } ``` There a similar coherence unsoundness caused by our handling of aliases which is fixed separately in rust-lang#117164. This change breaks the [`derive-visitor`](https://crates.io/crates/derive-visitor) crate. I have opened an issue in that repo: nikis05/derive-visitor#16. ### Evaluating goals to a fixpoint and applying inference constraints In the old implementation of the implicit-negative check, each obligation is [checked separately without applying its inference constraints](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L323-L338). The new solver instead [uses a `FulfillmentCtxt`](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L315-L321) for this, which evaluates all obligations in a loop until there's no further inference progress. This is necessary for backwards compatibility as we do not eagerly normalize with the new solver, resulting in constraints from normalization to only get applied by evaluating a separate obligation. This also allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Mirror { type Assoc; } impl<T> Mirror for T { type Assoc = T; } trait Foo {} trait Bar {} // The self type starts out as `?0` but is constrained to `()` // due to the where-clause below. Because `(): Bar` is known to // not hold, we can prove the impls disjoint. impl<T> Foo for T where (): Mirror<Assoc = T> {} //[current]~^ ERROR conflicting implementations of trait `Foo` for type `()` impl<T> Foo for T where T: Bar {} fn main() {} ``` The old solver does not run nested goals to a fixpoint in evaluation. The new solver does do so, strengthening inference and improving the overlap check: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Foo {} impl<T> Foo for (u8, T, T) {} trait NotU8 {} trait Bar {} impl<T, U: NotU8> Bar for (T, T, U) {} trait NeedsFixpoint {} impl<T: Foo + Bar> NeedsFixpoint for T {} impl NeedsFixpoint for (u8, u8, u8) {} trait Overlap {} impl<T: NeedsFixpoint> Overlap for T {} impl<T, U: NotU8, V> Overlap for (T, U, V) {} //[current]~^ ERROR conflicting implementations of trait `Foo` ``` ### Breakage due to removal of incomplete candidate preference Fixes rust-lang#107887. In the old solver we incompletely prefer the builtin trait object impl over user defined impls. This can break inference guidance, inferring `?x` in `dyn Trait<u32>: Trait<?x>` to `u32`, even if an explicit impl of `Trait<u64>` also exists. This caused coherence to incorrectly allow overlapping impls, resulting in ICEs and a theoretical unsoundness. See rust-lang#107887 (comment). This compiles on stable but results in an overlap error with `-Znext-solver=coherence`: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence struct W<T: ?Sized>(*const T); trait Trait<T: ?Sized> { type Assoc; } // This would trigger the check for overlap between automatic and custom impl. // They actually don't overlap so an impl like this should remain possible // forever. // // impl Trait<u64> for dyn Trait<u32> {} trait Indirect {} impl Indirect for dyn Trait<u32, Assoc = ()> {} impl<T: Indirect + ?Sized> Trait<u64> for T { type Assoc = (); } // Incomplete impl where `dyn Trait<u32>: Trait<_>` does not hold, but // `dyn Trait<u32>: Trait<u64>` does. trait EvaluateHack<U: ?Sized> {} impl<T: ?Sized, U: ?Sized> EvaluateHack<W<U>> for T where T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` U: IsU64, T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` { } trait IsU64 {} impl IsU64 for u64 {} trait Overlap<U: ?Sized> { type Assoc: Default; } impl<T: ?Sized + EvaluateHack<W<U>>, U: ?Sized> Overlap<U> for T { type Assoc = Box<u32>; } impl<U: ?Sized> Overlap<U> for dyn Trait<u32, Assoc = ()> { //[next]~^ ERROR conflicting implementations of trait `Overlap<_>` type Assoc = usize; } ``` ### Considering region outlives bounds in the `leak_check` For details on the `leak_check`, see the FCP proposal in rust-lang#119820.[^leak_check] [^leak_check]: which should get moved to the dev-guide once that PR lands :3 In both coherence and during candidate selection, the `leak_check` relies on the region constraints added in `evaluate`. It therefore currently does not register outlives obligations: [source](https://github.com/rust-lang/rust/blob/ccb1415eac3289b5ebf64691c0190dc52e0e3d0e/compiler/rustc_trait_selection/src/traits/select/mod.rs#L792-L810). This was likely done as a performance optimization without considering its impact on the `leak_check`. This is the case as in the old solver, *evaluatation* and *fulfillment* are split, with evaluation being responsible for candidate selection and fulfillment actually registering all the constraints. This split does not exist with the new solver. The `leak_check` can therefore eagerly detect errors caused by region outlives obligations. This improves both coherence itself and candidate selection: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait LeakErr<'a, 'b> {} // Using this impl adds an `'b: 'a` bound which results // in a higher-ranked region error. This bound has been // previously ignored but is now considered. impl<'a, 'b: 'a> LeakErr<'a, 'b> for () {} trait NoOverlapDir<'a> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> NoOverlapDir<'a> for T {} impl<'a> NoOverlapDir<'a> for () {} //[current]~^ ERROR conflicting implementations of trait `NoOverlapDir<'_>` // -------------------------------------- // necessary to avoid coherence unknowable candidates struct W<T>(T); trait GuidesSelection<'a, U> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> GuidesSelection<'a, W<u32>> for T {} impl<'a, T> GuidesSelection<'a, W<u8>> for T {} trait NotImplementedByU8 {} trait NoOverlapInd<'a, U> {} impl<'a, T: GuidesSelection<'a, W<U>>, U> NoOverlapInd<'a, U> for T {} impl<'a, U: NotImplementedByU8> NoOverlapInd<'a, U> for () {} //[current]~^ conflicting implementations of trait `NoOverlapInd<'_, _>` ``` ### Removal of `fn match_fresh_trait_refs` The old solver tries to [eagerly detect unbounded recursion](https://github.com/rust-lang/rust/blob/b14fd2359f47fb9a14bbfe55359db4bb3af11861/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1196-L1211), forcing the affected goals to be ambiguous. This check is only an approximation and has not been added to the new solver. The check is not necessary in the new solver and it would be problematic for caching. As it depends on all goals currently on the stack, using a global cache entry would have to always make sure that doing so does not circumvent this check. This changes some goals to error - or succeed - instead of failing with ambiguity. This allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence // Need to use this local wrapper for the impls to be fully // knowable as unknowable candidate result in ambiguity. struct Local<T>(T); trait Trait<U> {} // This impl does not hold, but is ambiguous in the old // solver due to its overflow approximation. impl<U> Trait<U> for Local<u32> where Local<u16>: Trait<U> {} // This impl holds. impl Trait<Local<()>> for Local<u8> {} // In the old solver, `Local<?t>: Trait<Local<?u>>` is ambiguous, // resulting in `Local<?u>: NoImpl`, also being ambiguous. // // In the new solver the first impl does not apply, constraining // `?u` to `Local<()>`, causing `Local<()>: NoImpl` to error. trait Indirect<T> {} impl<T, U> Indirect<U> for T where T: Trait<U>, U: NoImpl {} // Not implemented for `Local<()>` trait NoImpl {} impl NoImpl for Local<u8> {} impl NoImpl for Local<u16> {} // `Local<?t>: Indirect<Local<?u>>` cannot hold, so // these impls do not overlap. trait NoOverlap<U> {} impl<T: Indirect<U>, U> NoOverlap<U> for T {} impl<T, U> NoOverlap<Local<U>> for Local<T> {} //~^ ERROR conflicting implementations of trait `NoOverlap<Local<_>>` ``` ### Non-fatal overflow The old solver immediately emits a fatal error when hitting the recursion limit. The new solver instead returns overflow. This both allows more code to compile and is results in performance and potential future compatability issues. Non-fatal overflow is generally desirable. With fatal overflow, changing the order in which we evaluate nested goals easily causes breakage if we have goal which errors and one which overflows. It is also required to prevent breakage due to the removal of `fn match_fresh_trait_refs`, e.g. [in `typenum`](rust-lang/trait-system-refactor-initiative#73). #### Enabling more code to compile In the below example, the old solver first tried to prove an overflowing goal, resulting in a fatal error. The new solver instead returns ambiguity due to overflow for that goal, causing the implicit negative overlap check to succeed as `Box<u32>: NotImplemented` does not hold. ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence //[current] ERROR overflow evaluating the requirement trait Indirect<T> {} impl<T: Overflow<()>> Indirect<T> for () {} trait Overflow<U> {} impl<T, U> Overflow<U> for Box<T> where U: Indirect<Box<Box<T>>>, {} trait NotImplemented {} trait Trait<U> {} impl<T, U> Trait<U> for T where // T: NotImplemented, // causes old solver to succeed U: Indirect<T>, T: NotImplemented, {} impl Trait<()> for Box<u32> {} ``` #### Avoiding hangs with non-fatal overflow Simply returning ambiguity when reaching the recursion limit can very easily result in hangs, e.g. ```rust trait Recur {} impl<T, U> Recur for ((T, U), (U, T)) where (T, U): Recur, (U, T): Recur, {} trait NotImplemented {} impl<T: NotImplemented> Recur for T {} ``` This can happen quite frequently as it's easy to have exponential blowup due to multiple nested goals at each step. As the trait solver is depth-first, this immediately caused a fatal overflow error in the old solver. In the new solver we have to handle the whole proof tree instead, which can very easily hang. To avoid this we restrict the recursion depth after hitting the recursion limit for the first time. We also **ignore all inference constraints from goals resulting in overflow**. This is mostly backwards compatible as any overflow in the old solver resulted in a fatal error. ### sidenote about normalization We return ambiguous nested goals of `NormalizesTo` goals to the caller and ignore their impact when computing the `Certainty` of the current goal. See the [normalization chapter](https://rustc-dev-guide.rust-lang.org/solve/normalization.html) for more details.This means we apply constraints resulting from other nested goals and from equating the impl header when normalizing, even if a nested goal results in overflow. This is necessary to avoid breaking the following example: ```rust trait Trait { type Assoc; } struct W<T: ?Sized>(*mut T); impl<T: ?Sized> Trait for W<W<T>> where W<T>: Trait, { type Assoc = (); } // `W<?t>: Trait<Assoc = u32>` does not hold as // `Assoc` gets normalized to `()`. However, proving // the where-bounds of the impl results in overflow. // // For this to continue to compile we must not discard // constraints from normalizing associated types. trait NoOverlap {} impl<T: Trait<Assoc = u32>> NoOverlap for T {} impl<T: ?Sized> NoOverlap for W<T> {} ``` #### Future compatability concerns Non-fatal overflow results in some unfortunate future compatability concerns. Changing the approach to avoid more hangs by more strongly penalizing overflow can cause breakage as we either drop constraints or ignore candidates necessary to successfully compile. Weakening the overflow penalities instead allows more code to compile and strengthens inference while potentially causing more code to hang. While the current approach is not perfect, we believe it to be good enough. We believe it to apply the necessary inference constraints to avoid breakage and expect there to not be any desirable patterns broken by our current penalities. Similarly we believe the current constraints to avoid most accidental hangs. Ignoring constraints of overflowing goals is especially useful, as it may allow major future optimizations to our overflow handling. See [this summary](https://hackmd.io/ATf4hN0NRY-w2LIVgeFsVg) and the linked documents in case you want to know more. ### changes to performance In general, trait solving during coherence checking is not significant for performance. Enabling the next-generation trait solver in coherence does not impact our compile time benchmarks. We are still unable to compile the benchmark suite when fully enabling the new trait solver. There are rare cases where the new solver has significantly worse performance due to non-fatal overflow, its reliance on fixpoint algorithms and the removal of the `fn match_fresh_trait_refs` approximation. We encountered such issues in [`typenum`](https://crates.io/crates/typenum) and believe it should be [pretty much as bad as it can get](rust-lang/trait-system-refactor-initiative#73). Due to an improved structure and far better caching, we believe that there is a lot of room for improvement and that the new solver will outperform the existing implementation in nearly all cases, sometimes significantly. We have not yet spent any time micro-optimizing the implementation and have many unimplemented major improvements, such as fast-paths for trivial goals. TODO: get some rough results here and put them in a table ### Unstable features #### Unsupported unstable features The new solver currently does not support all unstable features, most notably `#![feature(generic_const_exprs)]`, `#![feature(associated_const_equality)]` and `#![feature(adt_const_params)]` are not yet fully supported in the new solver. We are confident that supporting them is possible, but did not consider this to be a priority. This stabilization introduces new ICE when using these features in impl headers. #### fixes to `#![feature(specialization)]` - fixes rust-lang#105782 - fixes rust-lang#118987 #### fixes to `#![feature(type_alias_impl_trait)]` - fixes rust-lang#119272 - rust-lang#105787 (comment) - fixes rust-lang#124207 ## This does not stabilize the whole solver While this stabilizes the use of the new solver in coherence checking, there are many parts of the solver which will remain fully unstable. We may still adapt these areas while working towards stabilizing the new solver everywhere. We are confident that we are able to do so without negatively impacting coherence. ### goals with a non-empty `ParamEnv` Coherence always uses an empty environment. We therefore do not depend on the behavior of `AliasBound` and `ParamEnv` candidates. We only stabilizes the behavior of user-defined and builtin implementations of traits. There are still many open questions there. ### opaque types in the defining scope The handling of opaque types - `impl Trait` - in both the new and old solver is still not fully figured out. Luckily this can be ignored for now. While opaque types are reachable during coherence checking by using `impl_trait_in_associated_types`, the behavior during coherence is separate and self-contained. The old and new solver fully agree here. ### normalization is hard This stabilizes that we equate associated types involving bound variables using deferred-alias-equality. We also stop eagerly normalizing in coherence, which should not have any user-facing impact. We do not stabilize the normalization behavior outside of coherence, e.g. we currently deeply normalize all types during writeback with the new solver. This may change going forward ### how to replace `select` from the old solver We sometimes depend on getting a single `impl` for a given trait bound, e.g. when resolving a concrete method for codegen/CTFE. We do not depend on this during coherence, so the exact approach here can still be freely changed going forward. ## Acknowledgements This work would not have been possible without `@compiler-errors.` He implemented large chunks of the solver himself but also and did a lot of testing and experimentation, eagerly discovering multiple issues which had a significant impact on our approach. `@BoxyUwU` has also done some amazing work on the solver. Thank you for the endless hours of discussion resulting in the current approach. Especially the way aliases are handled has gone through multiple revisions to get to its current state. There were also many contributions from - and discussions with - other members of the community and the rest of `@rust-lang/types.` This solver builds upon previous improvements to the compiler, as well as lessons learned from `chalk` and `a-mir-formality`. Getting to this point would not have been possible without that and I am incredibly thankful to everyone involved. See the [list of relevant PRs](https://github.com/rust-lang/rust/pulls?q=is%3Apr+is%3Amerged+label%3AWG-trait-system-refactor+-label%3Arollup+closed%3A%3C2024-03-22+).
fmease
added a commit
to fmease/rust
that referenced
this pull request
Jun 4, 2024
Orphanck[old solver]: Consider opaque types to never cover type parameters This fixes an oversight of mine in rust-lang#117164. The change itself has already been FCP'ed. This only affects the old solver, the next solver already correctly rejects the added test since rust-lang#117164. r? `@lcnr`
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Jun 5, 2024
Orphanck[old solver]: Consider opaque types to never cover type parameters This fixes an oversight of mine in rust-lang#117164. The change itself has already been FCP'ed. This only affects the old solver, the next solver already correctly rejects the added test since rust-lang#117164. r? ``@lcnr``
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 5, 2024
Rollup merge of rust-lang#125871 - fmease:fix-orphanck-opaques, r=lcnr Orphanck[old solver]: Consider opaque types to never cover type parameters This fixes an oversight of mine in rust-lang#117164. The change itself has already been FCP'ed. This only affects the old solver, the next solver already correctly rejects the added test since rust-lang#117164. r? ``@lcnr``
|
we have some crater run regressions, maybe these changes need to be in the changelog @rustbot label +relnotes |
rustbot
added
the
relnotes
fmease
changed the title
fmease
changed the title
|
The changes belong into the changelog, we automatically go through all T-types FCP'd PRs for each release, so the explicit relnotes tag isn't necessary imo (and easy to forget, which is why I want us to go through all FCPs when writing the release notes). |
oh interesting I wasn't aware (or just forgot) about this flow, thanks for explaining! (I thought that the only workflow was T-release getting all |
|
yeah, it's fairly new and also not really formalized anywhere, see https://rust-lang.zulipchat.com/#narrow/stream/241545-t-release/topic/1.2E78/near/430856836 😊 |
6 tasks
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 15, 2024
stabilize `-Znext-solver=coherence` try-job: x86_64-fuchsia r? `@compiler-errors` --- This PR stabilizes the use of the next generation trait solver in coherence checking by enabling `-Znext-solver=coherence` by default. More specifically its use in the *implicit negative overlap check*. The tracking issue for this is rust-lang#114862. ## Background ### The next generation trait solver The new solver lives in [`rustc_trait_selection::solve`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_trait_selection/src/solve/mod.rs) and is intended to replace the existing *evaluate*, *fulfill*, and *project* implementation. It also has a wider impact on the rest of the type system, for example by changing our approach to handling associated types. For a more detailed explanation of the new trait solver, see the [rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/solve/trait-solving.html). This does not stabilize the current behavior of the new trait solver, only the behavior impacting the implicit negative overlap check. There are many areas in the new solver which are not yet finalized. We are confident that their final design will not conflict with the user-facing behavior observable via coherence. More on that further down. Please check out [the chapter](https://rustc-dev-guide.rust-lang.org/solve/significant-changes.html) summarizing the most significant changes between the existing and new implementations. ### Coherence and the implicit negative overlap check Coherence checking detects any overlapping impls. Overlapping trait impls always error while overlapping inherent impls result in an error if they have methods with the same name. Coherence also results in an error if any other impls could exist, even if they are currently unknown. This affects impls which may get added to upstream crates in a backwards compatible way and impls from downstream crates. Coherence failing to detect overlap is generally considered to be unsound, even if it is difficult to actually get runtime UB this way. It is quite easy to get ICEs due to bugs in coherence. It currently consists of two checks: The [orphan check] validates that impls do not overlap with other impls we do not know about: either because they may be defined in a sibling crate, or because an upstream crate is allowed to add it without being considered a breaking change. The [overlap check] validates that impls do not overlap with other impls we know about. This is done as follows: - Instantiate the generic parameters of both impls with inference variables - Equate the `TraitRef`s of both impls. If it fails there is no overlap. - [implicit negative]: Check whether any of the instantiated `where`-bounds of one of the impls definitely do not hold when using the constraints from the previous step. If a `where`-bound does not hold, there is no overlap. - *explicit negative (still unstable, ignored going forward)*: Check whether the any negated `where`-bounds can be proven, e.g. a `&mut u32: Clone` bound definitely does not hold as an explicit `impl<T> !Clone for &mut T` exists. The overlap check has to *prove that unifying the impls does not succeed*. This means that **incorrectly getting a type error during coherence is unsound** as it would allow impls to overlap: coherence has to be *complete*. Completeness means that we never incorrectly error. This means that during coherence we must only add inference constraints if they are definitely necessary. During ordinary type checking [this does not hold](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01d93b592bd9036ac96071cbf1d624a9), so the trait solver has to behave differently, depending on whether we're in coherence or not. The implicit negative check only considers goals to "definitely not hold" if they could not be implemented downstream, by a sibling, or upstream in a backwards compatible way. If the goal is is "unknowable" as it may get added in another crate, we add an ambiguous candidate: [source](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L858-L883). [orphan check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L566-L579 [overlap check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L92-L98 [implicit negative]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L223-L281 ## Motivation Replacing the existing solver in coherence fixes soundness bugs by removing sources of incompleteness in the type system. The new solver separately strengthens coherence, resulting in more impls being disjoint and passing the coherence check. The concrete changes will be elaborated further down. We believe the stabilization to reduce the likelihood of future bugs in coherence as the new implementation is easier to understand and reason about. It allows us to remove the support for coherence and implicit-negative reasoning in the old solver, allowing us to remove some code and simplifying the old trait solver. We will only remove the old solver support once this stabilization has reached stable to make sure we're able to quickly revert in case any unexpected issues are detected before then. Stabilizing the use of the next-generation trait solver expresses our confidence that its current behavior is intended and our work towards enabling its use everywhere will not require any breaking changes to the areas used by coherence checking. We are also confident that we will be able to replace the existing solver everywhere, as maintaining two separate systems adds a significant maintainance burden. ## User-facing impact and reasoning ### Breakage due to improved handling of associated types The new solver fixes multiple issues related to associated types. As these issues caused coherence to consider more types distinct, fixing them results in more overlap errors. This is therefore a breaking change. #### Structurally relating aliases containing bound vars Fixes rust-lang#102048. In the existing solver relating ambiguous projections containing bound variables is structural. This is *incomplete* and allows overlapping impls. These was mostly not exploitable as the same issue also caused impls to not apply when trying to use them. The new solver defers alias-relating to a nested goal, fixing this issue: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Trait {} trait Project { type Assoc<'a>; } impl Project for u32 { type Assoc<'a> = &'a u32; } // Eagerly normalizing `<?infer as Project>::Assoc<'a>` is ambiguous, // so the old solver ended up structurally relating // // (?infer, for<'a> fn(<?infer as Project>::Assoc<'a>)) // // with // // ((u32, fn(&'a u32))) // // Equating `&'a u32` with `<u32 as Project>::Assoc<'a>` failed, even // though these types are equal modulo normalization. impl<T: Project> Trait for (T, for<'a> fn(<T as Project>::Assoc<'a>)) {} impl<'a> Trait for (u32, fn(&'a u32)) {} //[next]~^ ERROR conflicting implementations of trait `Trait` for type `(u32, for<'a> fn(&'a u32))` ``` A crater run did not discover any breakage due to this change. #### Unknowable candidates for higher ranked trait goals This avoids an unsoundness by attempting to normalize in `trait_ref_is_knowable`, fixing rust-lang#114061. This is a side-effect of supporting lazy normalization, as that forces us to attempt to normalize when checking whether a `TraitRef` is knowable: [source](https://github.com/rust-lang/rust/blob/47dd709bedda8127e8daec33327e0a9d0cdae845/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L754-L764). ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait IsUnit {} impl IsUnit for () {} pub trait WithAssoc<'a> { type Assoc; } // We considered `for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit` // to be knowable, even though the projection is ambiguous. pub trait Trait {} impl<T> Trait for T where T: 'static, for<'a> T: WithAssoc<'a>, for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit, { } impl<T> Trait for Box<T> {} //[next]~^ ERROR conflicting implementations of trait `Trait` ``` The two impls of `Trait` overlap given the following downstream crate: ```rust use dep::*; struct Local; impl WithAssoc<'_> for Box<Local> { type Assoc = (); } ``` There a similar coherence unsoundness caused by our handling of aliases which is fixed separately in rust-lang#117164. This change breaks the [`derive-visitor`](https://crates.io/crates/derive-visitor) crate. I have opened an issue in that repo: nikis05/derive-visitor#16. ### Evaluating goals to a fixpoint and applying inference constraints In the old implementation of the implicit-negative check, each obligation is [checked separately without applying its inference constraints](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L323-L338). The new solver instead [uses a `FulfillmentCtxt`](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L315-L321) for this, which evaluates all obligations in a loop until there's no further inference progress. This is necessary for backwards compatibility as we do not eagerly normalize with the new solver, resulting in constraints from normalization to only get applied by evaluating a separate obligation. This also allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Mirror { type Assoc; } impl<T> Mirror for T { type Assoc = T; } trait Foo {} trait Bar {} // The self type starts out as `?0` but is constrained to `()` // due to the where-clause below. Because `(): Bar` is known to // not hold, we can prove the impls disjoint. impl<T> Foo for T where (): Mirror<Assoc = T> {} //[current]~^ ERROR conflicting implementations of trait `Foo` for type `()` impl<T> Foo for T where T: Bar {} fn main() {} ``` The old solver does not run nested goals to a fixpoint in evaluation. The new solver does do so, strengthening inference and improving the overlap check: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Foo {} impl<T> Foo for (u8, T, T) {} trait NotU8 {} trait Bar {} impl<T, U: NotU8> Bar for (T, T, U) {} trait NeedsFixpoint {} impl<T: Foo + Bar> NeedsFixpoint for T {} impl NeedsFixpoint for (u8, u8, u8) {} trait Overlap {} impl<T: NeedsFixpoint> Overlap for T {} impl<T, U: NotU8, V> Overlap for (T, U, V) {} //[current]~^ ERROR conflicting implementations of trait `Foo` ``` ### Breakage due to removal of incomplete candidate preference Fixes rust-lang#107887. In the old solver we incompletely prefer the builtin trait object impl over user defined impls. This can break inference guidance, inferring `?x` in `dyn Trait<u32>: Trait<?x>` to `u32`, even if an explicit impl of `Trait<u64>` also exists. This caused coherence to incorrectly allow overlapping impls, resulting in ICEs and a theoretical unsoundness. See rust-lang#107887 (comment). This compiles on stable but results in an overlap error with `-Znext-solver=coherence`: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence struct W<T: ?Sized>(*const T); trait Trait<T: ?Sized> { type Assoc; } // This would trigger the check for overlap between automatic and custom impl. // They actually don't overlap so an impl like this should remain possible // forever. // // impl Trait<u64> for dyn Trait<u32> {} trait Indirect {} impl Indirect for dyn Trait<u32, Assoc = ()> {} impl<T: Indirect + ?Sized> Trait<u64> for T { type Assoc = (); } // Incomplete impl where `dyn Trait<u32>: Trait<_>` does not hold, but // `dyn Trait<u32>: Trait<u64>` does. trait EvaluateHack<U: ?Sized> {} impl<T: ?Sized, U: ?Sized> EvaluateHack<W<U>> for T where T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` U: IsU64, T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` { } trait IsU64 {} impl IsU64 for u64 {} trait Overlap<U: ?Sized> { type Assoc: Default; } impl<T: ?Sized + EvaluateHack<W<U>>, U: ?Sized> Overlap<U> for T { type Assoc = Box<u32>; } impl<U: ?Sized> Overlap<U> for dyn Trait<u32, Assoc = ()> { //[next]~^ ERROR conflicting implementations of trait `Overlap<_>` type Assoc = usize; } ``` ### Considering region outlives bounds in the `leak_check` For details on the `leak_check`, see the FCP proposal in rust-lang#119820.[^leak_check] [^leak_check]: which should get moved to the dev-guide once that PR lands :3 In both coherence and during candidate selection, the `leak_check` relies on the region constraints added in `evaluate`. It therefore currently does not register outlives obligations: [source](https://github.com/rust-lang/rust/blob/ccb1415eac3289b5ebf64691c0190dc52e0e3d0e/compiler/rustc_trait_selection/src/traits/select/mod.rs#L792-L810). This was likely done as a performance optimization without considering its impact on the `leak_check`. This is the case as in the old solver, *evaluatation* and *fulfillment* are split, with evaluation being responsible for candidate selection and fulfillment actually registering all the constraints. This split does not exist with the new solver. The `leak_check` can therefore eagerly detect errors caused by region outlives obligations. This improves both coherence itself and candidate selection: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait LeakErr<'a, 'b> {} // Using this impl adds an `'b: 'a` bound which results // in a higher-ranked region error. This bound has been // previously ignored but is now considered. impl<'a, 'b: 'a> LeakErr<'a, 'b> for () {} trait NoOverlapDir<'a> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> NoOverlapDir<'a> for T {} impl<'a> NoOverlapDir<'a> for () {} //[current]~^ ERROR conflicting implementations of trait `NoOverlapDir<'_>` // -------------------------------------- // necessary to avoid coherence unknowable candidates struct W<T>(T); trait GuidesSelection<'a, U> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> GuidesSelection<'a, W<u32>> for T {} impl<'a, T> GuidesSelection<'a, W<u8>> for T {} trait NotImplementedByU8 {} trait NoOverlapInd<'a, U> {} impl<'a, T: GuidesSelection<'a, W<U>>, U> NoOverlapInd<'a, U> for T {} impl<'a, U: NotImplementedByU8> NoOverlapInd<'a, U> for () {} //[current]~^ conflicting implementations of trait `NoOverlapInd<'_, _>` ``` ### Removal of `fn match_fresh_trait_refs` The old solver tries to [eagerly detect unbounded recursion](https://github.com/rust-lang/rust/blob/b14fd2359f47fb9a14bbfe55359db4bb3af11861/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1196-L1211), forcing the affected goals to be ambiguous. This check is only an approximation and has not been added to the new solver. The check is not necessary in the new solver and it would be problematic for caching. As it depends on all goals currently on the stack, using a global cache entry would have to always make sure that doing so does not circumvent this check. This changes some goals to error - or succeed - instead of failing with ambiguity. This allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence // Need to use this local wrapper for the impls to be fully // knowable as unknowable candidate result in ambiguity. struct Local<T>(T); trait Trait<U> {} // This impl does not hold, but is ambiguous in the old // solver due to its overflow approximation. impl<U> Trait<U> for Local<u32> where Local<u16>: Trait<U> {} // This impl holds. impl Trait<Local<()>> for Local<u8> {} // In the old solver, `Local<?t>: Trait<Local<?u>>` is ambiguous, // resulting in `Local<?u>: NoImpl`, also being ambiguous. // // In the new solver the first impl does not apply, constraining // `?u` to `Local<()>`, causing `Local<()>: NoImpl` to error. trait Indirect<T> {} impl<T, U> Indirect<U> for T where T: Trait<U>, U: NoImpl {} // Not implemented for `Local<()>` trait NoImpl {} impl NoImpl for Local<u8> {} impl NoImpl for Local<u16> {} // `Local<?t>: Indirect<Local<?u>>` cannot hold, so // these impls do not overlap. trait NoOverlap<U> {} impl<T: Indirect<U>, U> NoOverlap<U> for T {} impl<T, U> NoOverlap<Local<U>> for Local<T> {} //~^ ERROR conflicting implementations of trait `NoOverlap<Local<_>>` ``` ### Non-fatal overflow The old solver immediately emits a fatal error when hitting the recursion limit. The new solver instead returns overflow. This both allows more code to compile and is results in performance and potential future compatability issues. Non-fatal overflow is generally desirable. With fatal overflow, changing the order in which we evaluate nested goals easily causes breakage if we have goal which errors and one which overflows. It is also required to prevent breakage due to the removal of `fn match_fresh_trait_refs`, e.g. [in `typenum`](rust-lang/trait-system-refactor-initiative#73). #### Enabling more code to compile In the below example, the old solver first tried to prove an overflowing goal, resulting in a fatal error. The new solver instead returns ambiguity due to overflow for that goal, causing the implicit negative overlap check to succeed as `Box<u32>: NotImplemented` does not hold. ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence //[current] ERROR overflow evaluating the requirement trait Indirect<T> {} impl<T: Overflow<()>> Indirect<T> for () {} trait Overflow<U> {} impl<T, U> Overflow<U> for Box<T> where U: Indirect<Box<Box<T>>>, {} trait NotImplemented {} trait Trait<U> {} impl<T, U> Trait<U> for T where // T: NotImplemented, // causes old solver to succeed U: Indirect<T>, T: NotImplemented, {} impl Trait<()> for Box<u32> {} ``` #### Avoiding hangs with non-fatal overflow Simply returning ambiguity when reaching the recursion limit can very easily result in hangs, e.g. ```rust trait Recur {} impl<T, U> Recur for ((T, U), (U, T)) where (T, U): Recur, (U, T): Recur, {} trait NotImplemented {} impl<T: NotImplemented> Recur for T {} ``` This can happen quite frequently as it's easy to have exponential blowup due to multiple nested goals at each step. As the trait solver is depth-first, this immediately caused a fatal overflow error in the old solver. In the new solver we have to handle the whole proof tree instead, which can very easily hang. To avoid this we restrict the recursion depth after hitting the recursion limit for the first time. We also **ignore all inference constraints from goals resulting in overflow**. This is mostly backwards compatible as any overflow in the old solver resulted in a fatal error. ### sidenote about normalization We return ambiguous nested goals of `NormalizesTo` goals to the caller and ignore their impact when computing the `Certainty` of the current goal. See the [normalization chapter](https://rustc-dev-guide.rust-lang.org/solve/normalization.html) for more details.This means we apply constraints resulting from other nested goals and from equating the impl header when normalizing, even if a nested goal results in overflow. This is necessary to avoid breaking the following example: ```rust trait Trait { type Assoc; } struct W<T: ?Sized>(*mut T); impl<T: ?Sized> Trait for W<W<T>> where W<T>: Trait, { type Assoc = (); } // `W<?t>: Trait<Assoc = u32>` does not hold as // `Assoc` gets normalized to `()`. However, proving // the where-bounds of the impl results in overflow. // // For this to continue to compile we must not discard // constraints from normalizing associated types. trait NoOverlap {} impl<T: Trait<Assoc = u32>> NoOverlap for T {} impl<T: ?Sized> NoOverlap for W<T> {} ``` #### Future compatability concerns Non-fatal overflow results in some unfortunate future compatability concerns. Changing the approach to avoid more hangs by more strongly penalizing overflow can cause breakage as we either drop constraints or ignore candidates necessary to successfully compile. Weakening the overflow penalities instead allows more code to compile and strengthens inference while potentially causing more code to hang. While the current approach is not perfect, we believe it to be good enough. We believe it to apply the necessary inference constraints to avoid breakage and expect there to not be any desirable patterns broken by our current penalities. Similarly we believe the current constraints to avoid most accidental hangs. Ignoring constraints of overflowing goals is especially useful, as it may allow major future optimizations to our overflow handling. See [this summary](https://hackmd.io/ATf4hN0NRY-w2LIVgeFsVg) and the linked documents in case you want to know more. ### changes to performance In general, trait solving during coherence checking is not significant for performance. Enabling the next-generation trait solver in coherence does not impact our compile time benchmarks. We are still unable to compile the benchmark suite when fully enabling the new trait solver. There are rare cases where the new solver has significantly worse performance due to non-fatal overflow, its reliance on fixpoint algorithms and the removal of the `fn match_fresh_trait_refs` approximation. We encountered such issues in [`typenum`](https://crates.io/crates/typenum) and believe it should be [pretty much as bad as it can get](rust-lang/trait-system-refactor-initiative#73). Due to an improved structure and far better caching, we believe that there is a lot of room for improvement and that the new solver will outperform the existing implementation in nearly all cases, sometimes significantly. We have not yet spent any time micro-optimizing the implementation and have many unimplemented major improvements, such as fast-paths for trivial goals. TODO: get some rough results here and put them in a table ### Unstable features #### Unsupported unstable features The new solver currently does not support all unstable features, most notably `#![feature(generic_const_exprs)]`, `#![feature(associated_const_equality)]` and `#![feature(adt_const_params)]` are not yet fully supported in the new solver. We are confident that supporting them is possible, but did not consider this to be a priority. This stabilization introduces new ICE when using these features in impl headers. #### fixes to `#![feature(specialization)]` - fixes rust-lang#105782 - fixes rust-lang#118987 #### fixes to `#![feature(type_alias_impl_trait)]` - fixes rust-lang#119272 - rust-lang#105787 (comment) - fixes rust-lang#124207 ## This does not stabilize the whole solver While this stabilizes the use of the new solver in coherence checking, there are many parts of the solver which will remain fully unstable. We may still adapt these areas while working towards stabilizing the new solver everywhere. We are confident that we are able to do so without negatively impacting coherence. ### goals with a non-empty `ParamEnv` Coherence always uses an empty environment. We therefore do not depend on the behavior of `AliasBound` and `ParamEnv` candidates. We only stabilizes the behavior of user-defined and builtin implementations of traits. There are still many open questions there. ### opaque types in the defining scope The handling of opaque types - `impl Trait` - in both the new and old solver is still not fully figured out. Luckily this can be ignored for now. While opaque types are reachable during coherence checking by using `impl_trait_in_associated_types`, the behavior during coherence is separate and self-contained. The old and new solver fully agree here. ### normalization is hard This stabilizes that we equate associated types involving bound variables using deferred-alias-equality. We also stop eagerly normalizing in coherence, which should not have any user-facing impact. We do not stabilize the normalization behavior outside of coherence, e.g. we currently deeply normalize all types during writeback with the new solver. This may change going forward ### how to replace `select` from the old solver We sometimes depend on getting a single `impl` for a given trait bound, e.g. when resolving a concrete method for codegen/CTFE. We do not depend on this during coherence, so the exact approach here can still be freely changed going forward. ## Acknowledgements This work would not have been possible without `@compiler-errors.` He implemented large chunks of the solver himself but also and did a lot of testing and experimentation, eagerly discovering multiple issues which had a significant impact on our approach. `@BoxyUwU` has also done some amazing work on the solver. Thank you for the endless hours of discussion resulting in the current approach. Especially the way aliases are handled has gone through multiple revisions to get to its current state. There were also many contributions from - and discussions with - other members of the community and the rest of `@rust-lang/types.` This solver builds upon previous improvements to the compiler, as well as lessons learned from `chalk` and `a-mir-formality`. Getting to this point would not have been possible without that and I am incredibly thankful to everyone involved. See the [list of relevant PRs](https://github.com/rust-lang/rust/pulls?q=is%3Apr+is%3Amerged+label%3AWG-trait-system-refactor+-label%3Arollup+closed%3A%3C2024-03-22+).
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 15, 2024
stabilize `-Znext-solver=coherence` r? `@compiler-errors` --- This PR stabilizes the use of the next generation trait solver in coherence checking by enabling `-Znext-solver=coherence` by default. More specifically its use in the *implicit negative overlap check*. The tracking issue for this is rust-lang#114862. ## Background ### The next generation trait solver The new solver lives in [`rustc_trait_selection::solve`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_trait_selection/src/solve/mod.rs) and is intended to replace the existing *evaluate*, *fulfill*, and *project* implementation. It also has a wider impact on the rest of the type system, for example by changing our approach to handling associated types. For a more detailed explanation of the new trait solver, see the [rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/solve/trait-solving.html). This does not stabilize the current behavior of the new trait solver, only the behavior impacting the implicit negative overlap check. There are many areas in the new solver which are not yet finalized. We are confident that their final design will not conflict with the user-facing behavior observable via coherence. More on that further down. Please check out [the chapter](https://rustc-dev-guide.rust-lang.org/solve/significant-changes.html) summarizing the most significant changes between the existing and new implementations. ### Coherence and the implicit negative overlap check Coherence checking detects any overlapping impls. Overlapping trait impls always error while overlapping inherent impls result in an error if they have methods with the same name. Coherence also results in an error if any other impls could exist, even if they are currently unknown. This affects impls which may get added to upstream crates in a backwards compatible way and impls from downstream crates. Coherence failing to detect overlap is generally considered to be unsound, even if it is difficult to actually get runtime UB this way. It is quite easy to get ICEs due to bugs in coherence. It currently consists of two checks: The [orphan check] validates that impls do not overlap with other impls we do not know about: either because they may be defined in a sibling crate, or because an upstream crate is allowed to add it without being considered a breaking change. The [overlap check] validates that impls do not overlap with other impls we know about. This is done as follows: - Instantiate the generic parameters of both impls with inference variables - Equate the `TraitRef`s of both impls. If it fails there is no overlap. - [implicit negative]: Check whether any of the instantiated `where`-bounds of one of the impls definitely do not hold when using the constraints from the previous step. If a `where`-bound does not hold, there is no overlap. - *explicit negative (still unstable, ignored going forward)*: Check whether the any negated `where`-bounds can be proven, e.g. a `&mut u32: Clone` bound definitely does not hold as an explicit `impl<T> !Clone for &mut T` exists. The overlap check has to *prove that unifying the impls does not succeed*. This means that **incorrectly getting a type error during coherence is unsound** as it would allow impls to overlap: coherence has to be *complete*. Completeness means that we never incorrectly error. This means that during coherence we must only add inference constraints if they are definitely necessary. During ordinary type checking [this does not hold](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01d93b592bd9036ac96071cbf1d624a9), so the trait solver has to behave differently, depending on whether we're in coherence or not. The implicit negative check only considers goals to "definitely not hold" if they could not be implemented downstream, by a sibling, or upstream in a backwards compatible way. If the goal is is "unknowable" as it may get added in another crate, we add an ambiguous candidate: [source](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L858-L883). [orphan check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L566-L579 [overlap check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L92-L98 [implicit negative]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L223-L281 ## Motivation Replacing the existing solver in coherence fixes soundness bugs by removing sources of incompleteness in the type system. The new solver separately strengthens coherence, resulting in more impls being disjoint and passing the coherence check. The concrete changes will be elaborated further down. We believe the stabilization to reduce the likelihood of future bugs in coherence as the new implementation is easier to understand and reason about. It allows us to remove the support for coherence and implicit-negative reasoning in the old solver, allowing us to remove some code and simplifying the old trait solver. We will only remove the old solver support once this stabilization has reached stable to make sure we're able to quickly revert in case any unexpected issues are detected before then. Stabilizing the use of the next-generation trait solver expresses our confidence that its current behavior is intended and our work towards enabling its use everywhere will not require any breaking changes to the areas used by coherence checking. We are also confident that we will be able to replace the existing solver everywhere, as maintaining two separate systems adds a significant maintainance burden. ## User-facing impact and reasoning ### Breakage due to improved handling of associated types The new solver fixes multiple issues related to associated types. As these issues caused coherence to consider more types distinct, fixing them results in more overlap errors. This is therefore a breaking change. #### Structurally relating aliases containing bound vars Fixes rust-lang#102048. In the existing solver relating ambiguous projections containing bound variables is structural. This is *incomplete* and allows overlapping impls. These was mostly not exploitable as the same issue also caused impls to not apply when trying to use them. The new solver defers alias-relating to a nested goal, fixing this issue: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Trait {} trait Project { type Assoc<'a>; } impl Project for u32 { type Assoc<'a> = &'a u32; } // Eagerly normalizing `<?infer as Project>::Assoc<'a>` is ambiguous, // so the old solver ended up structurally relating // // (?infer, for<'a> fn(<?infer as Project>::Assoc<'a>)) // // with // // ((u32, fn(&'a u32))) // // Equating `&'a u32` with `<u32 as Project>::Assoc<'a>` failed, even // though these types are equal modulo normalization. impl<T: Project> Trait for (T, for<'a> fn(<T as Project>::Assoc<'a>)) {} impl<'a> Trait for (u32, fn(&'a u32)) {} //[next]~^ ERROR conflicting implementations of trait `Trait` for type `(u32, for<'a> fn(&'a u32))` ``` A crater run did not discover any breakage due to this change. #### Unknowable candidates for higher ranked trait goals This avoids an unsoundness by attempting to normalize in `trait_ref_is_knowable`, fixing rust-lang#114061. This is a side-effect of supporting lazy normalization, as that forces us to attempt to normalize when checking whether a `TraitRef` is knowable: [source](https://github.com/rust-lang/rust/blob/47dd709bedda8127e8daec33327e0a9d0cdae845/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L754-L764). ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait IsUnit {} impl IsUnit for () {} pub trait WithAssoc<'a> { type Assoc; } // We considered `for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit` // to be knowable, even though the projection is ambiguous. pub trait Trait {} impl<T> Trait for T where T: 'static, for<'a> T: WithAssoc<'a>, for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit, { } impl<T> Trait for Box<T> {} //[next]~^ ERROR conflicting implementations of trait `Trait` ``` The two impls of `Trait` overlap given the following downstream crate: ```rust use dep::*; struct Local; impl WithAssoc<'_> for Box<Local> { type Assoc = (); } ``` There a similar coherence unsoundness caused by our handling of aliases which is fixed separately in rust-lang#117164. This change breaks the [`derive-visitor`](https://crates.io/crates/derive-visitor) crate. I have opened an issue in that repo: nikis05/derive-visitor#16. ### Evaluating goals to a fixpoint and applying inference constraints In the old implementation of the implicit-negative check, each obligation is [checked separately without applying its inference constraints](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L323-L338). The new solver instead [uses a `FulfillmentCtxt`](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L315-L321) for this, which evaluates all obligations in a loop until there's no further inference progress. This is necessary for backwards compatibility as we do not eagerly normalize with the new solver, resulting in constraints from normalization to only get applied by evaluating a separate obligation. This also allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Mirror { type Assoc; } impl<T> Mirror for T { type Assoc = T; } trait Foo {} trait Bar {} // The self type starts out as `?0` but is constrained to `()` // due to the where-clause below. Because `(): Bar` is known to // not hold, we can prove the impls disjoint. impl<T> Foo for T where (): Mirror<Assoc = T> {} //[current]~^ ERROR conflicting implementations of trait `Foo` for type `()` impl<T> Foo for T where T: Bar {} fn main() {} ``` The old solver does not run nested goals to a fixpoint in evaluation. The new solver does do so, strengthening inference and improving the overlap check: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Foo {} impl<T> Foo for (u8, T, T) {} trait NotU8 {} trait Bar {} impl<T, U: NotU8> Bar for (T, T, U) {} trait NeedsFixpoint {} impl<T: Foo + Bar> NeedsFixpoint for T {} impl NeedsFixpoint for (u8, u8, u8) {} trait Overlap {} impl<T: NeedsFixpoint> Overlap for T {} impl<T, U: NotU8, V> Overlap for (T, U, V) {} //[current]~^ ERROR conflicting implementations of trait `Foo` ``` ### Breakage due to removal of incomplete candidate preference Fixes rust-lang#107887. In the old solver we incompletely prefer the builtin trait object impl over user defined impls. This can break inference guidance, inferring `?x` in `dyn Trait<u32>: Trait<?x>` to `u32`, even if an explicit impl of `Trait<u64>` also exists. This caused coherence to incorrectly allow overlapping impls, resulting in ICEs and a theoretical unsoundness. See rust-lang#107887 (comment). This compiles on stable but results in an overlap error with `-Znext-solver=coherence`: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence struct W<T: ?Sized>(*const T); trait Trait<T: ?Sized> { type Assoc; } // This would trigger the check for overlap between automatic and custom impl. // They actually don't overlap so an impl like this should remain possible // forever. // // impl Trait<u64> for dyn Trait<u32> {} trait Indirect {} impl Indirect for dyn Trait<u32, Assoc = ()> {} impl<T: Indirect + ?Sized> Trait<u64> for T { type Assoc = (); } // Incomplete impl where `dyn Trait<u32>: Trait<_>` does not hold, but // `dyn Trait<u32>: Trait<u64>` does. trait EvaluateHack<U: ?Sized> {} impl<T: ?Sized, U: ?Sized> EvaluateHack<W<U>> for T where T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` U: IsU64, T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` { } trait IsU64 {} impl IsU64 for u64 {} trait Overlap<U: ?Sized> { type Assoc: Default; } impl<T: ?Sized + EvaluateHack<W<U>>, U: ?Sized> Overlap<U> for T { type Assoc = Box<u32>; } impl<U: ?Sized> Overlap<U> for dyn Trait<u32, Assoc = ()> { //[next]~^ ERROR conflicting implementations of trait `Overlap<_>` type Assoc = usize; } ``` ### Considering region outlives bounds in the `leak_check` For details on the `leak_check`, see the FCP proposal in rust-lang#119820.[^leak_check] [^leak_check]: which should get moved to the dev-guide once that PR lands :3 In both coherence and during candidate selection, the `leak_check` relies on the region constraints added in `evaluate`. It therefore currently does not register outlives obligations: [source](https://github.com/rust-lang/rust/blob/ccb1415eac3289b5ebf64691c0190dc52e0e3d0e/compiler/rustc_trait_selection/src/traits/select/mod.rs#L792-L810). This was likely done as a performance optimization without considering its impact on the `leak_check`. This is the case as in the old solver, *evaluatation* and *fulfillment* are split, with evaluation being responsible for candidate selection and fulfillment actually registering all the constraints. This split does not exist with the new solver. The `leak_check` can therefore eagerly detect errors caused by region outlives obligations. This improves both coherence itself and candidate selection: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait LeakErr<'a, 'b> {} // Using this impl adds an `'b: 'a` bound which results // in a higher-ranked region error. This bound has been // previously ignored but is now considered. impl<'a, 'b: 'a> LeakErr<'a, 'b> for () {} trait NoOverlapDir<'a> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> NoOverlapDir<'a> for T {} impl<'a> NoOverlapDir<'a> for () {} //[current]~^ ERROR conflicting implementations of trait `NoOverlapDir<'_>` // -------------------------------------- // necessary to avoid coherence unknowable candidates struct W<T>(T); trait GuidesSelection<'a, U> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> GuidesSelection<'a, W<u32>> for T {} impl<'a, T> GuidesSelection<'a, W<u8>> for T {} trait NotImplementedByU8 {} trait NoOverlapInd<'a, U> {} impl<'a, T: GuidesSelection<'a, W<U>>, U> NoOverlapInd<'a, U> for T {} impl<'a, U: NotImplementedByU8> NoOverlapInd<'a, U> for () {} //[current]~^ conflicting implementations of trait `NoOverlapInd<'_, _>` ``` ### Removal of `fn match_fresh_trait_refs` The old solver tries to [eagerly detect unbounded recursion](https://github.com/rust-lang/rust/blob/b14fd2359f47fb9a14bbfe55359db4bb3af11861/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1196-L1211), forcing the affected goals to be ambiguous. This check is only an approximation and has not been added to the new solver. The check is not necessary in the new solver and it would be problematic for caching. As it depends on all goals currently on the stack, using a global cache entry would have to always make sure that doing so does not circumvent this check. This changes some goals to error - or succeed - instead of failing with ambiguity. This allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence // Need to use this local wrapper for the impls to be fully // knowable as unknowable candidate result in ambiguity. struct Local<T>(T); trait Trait<U> {} // This impl does not hold, but is ambiguous in the old // solver due to its overflow approximation. impl<U> Trait<U> for Local<u32> where Local<u16>: Trait<U> {} // This impl holds. impl Trait<Local<()>> for Local<u8> {} // In the old solver, `Local<?t>: Trait<Local<?u>>` is ambiguous, // resulting in `Local<?u>: NoImpl`, also being ambiguous. // // In the new solver the first impl does not apply, constraining // `?u` to `Local<()>`, causing `Local<()>: NoImpl` to error. trait Indirect<T> {} impl<T, U> Indirect<U> for T where T: Trait<U>, U: NoImpl {} // Not implemented for `Local<()>` trait NoImpl {} impl NoImpl for Local<u8> {} impl NoImpl for Local<u16> {} // `Local<?t>: Indirect<Local<?u>>` cannot hold, so // these impls do not overlap. trait NoOverlap<U> {} impl<T: Indirect<U>, U> NoOverlap<U> for T {} impl<T, U> NoOverlap<Local<U>> for Local<T> {} //~^ ERROR conflicting implementations of trait `NoOverlap<Local<_>>` ``` ### Non-fatal overflow The old solver immediately emits a fatal error when hitting the recursion limit. The new solver instead returns overflow. This both allows more code to compile and is results in performance and potential future compatability issues. Non-fatal overflow is generally desirable. With fatal overflow, changing the order in which we evaluate nested goals easily causes breakage if we have goal which errors and one which overflows. It is also required to prevent breakage due to the removal of `fn match_fresh_trait_refs`, e.g. [in `typenum`](rust-lang/trait-system-refactor-initiative#73). #### Enabling more code to compile In the below example, the old solver first tried to prove an overflowing goal, resulting in a fatal error. The new solver instead returns ambiguity due to overflow for that goal, causing the implicit negative overlap check to succeed as `Box<u32>: NotImplemented` does not hold. ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence //[current] ERROR overflow evaluating the requirement trait Indirect<T> {} impl<T: Overflow<()>> Indirect<T> for () {} trait Overflow<U> {} impl<T, U> Overflow<U> for Box<T> where U: Indirect<Box<Box<T>>>, {} trait NotImplemented {} trait Trait<U> {} impl<T, U> Trait<U> for T where // T: NotImplemented, // causes old solver to succeed U: Indirect<T>, T: NotImplemented, {} impl Trait<()> for Box<u32> {} ``` #### Avoiding hangs with non-fatal overflow Simply returning ambiguity when reaching the recursion limit can very easily result in hangs, e.g. ```rust trait Recur {} impl<T, U> Recur for ((T, U), (U, T)) where (T, U): Recur, (U, T): Recur, {} trait NotImplemented {} impl<T: NotImplemented> Recur for T {} ``` This can happen quite frequently as it's easy to have exponential blowup due to multiple nested goals at each step. As the trait solver is depth-first, this immediately caused a fatal overflow error in the old solver. In the new solver we have to handle the whole proof tree instead, which can very easily hang. To avoid this we restrict the recursion depth after hitting the recursion limit for the first time. We also **ignore all inference constraints from goals resulting in overflow**. This is mostly backwards compatible as any overflow in the old solver resulted in a fatal error. ### sidenote about normalization We return ambiguous nested goals of `NormalizesTo` goals to the caller and ignore their impact when computing the `Certainty` of the current goal. See the [normalization chapter](https://rustc-dev-guide.rust-lang.org/solve/normalization.html) for more details.This means we apply constraints resulting from other nested goals and from equating the impl header when normalizing, even if a nested goal results in overflow. This is necessary to avoid breaking the following example: ```rust trait Trait { type Assoc; } struct W<T: ?Sized>(*mut T); impl<T: ?Sized> Trait for W<W<T>> where W<T>: Trait, { type Assoc = (); } // `W<?t>: Trait<Assoc = u32>` does not hold as // `Assoc` gets normalized to `()`. However, proving // the where-bounds of the impl results in overflow. // // For this to continue to compile we must not discard // constraints from normalizing associated types. trait NoOverlap {} impl<T: Trait<Assoc = u32>> NoOverlap for T {} impl<T: ?Sized> NoOverlap for W<T> {} ``` #### Future compatability concerns Non-fatal overflow results in some unfortunate future compatability concerns. Changing the approach to avoid more hangs by more strongly penalizing overflow can cause breakage as we either drop constraints or ignore candidates necessary to successfully compile. Weakening the overflow penalities instead allows more code to compile and strengthens inference while potentially causing more code to hang. While the current approach is not perfect, we believe it to be good enough. We believe it to apply the necessary inference constraints to avoid breakage and expect there to not be any desirable patterns broken by our current penalities. Similarly we believe the current constraints to avoid most accidental hangs. Ignoring constraints of overflowing goals is especially useful, as it may allow major future optimizations to our overflow handling. See [this summary](https://hackmd.io/ATf4hN0NRY-w2LIVgeFsVg) and the linked documents in case you want to know more. ### changes to performance In general, trait solving during coherence checking is not significant for performance. Enabling the next-generation trait solver in coherence does not impact our compile time benchmarks. We are still unable to compile the benchmark suite when fully enabling the new trait solver. There are rare cases where the new solver has significantly worse performance due to non-fatal overflow, its reliance on fixpoint algorithms and the removal of the `fn match_fresh_trait_refs` approximation. We encountered such issues in [`typenum`](https://crates.io/crates/typenum) and believe it should be [pretty much as bad as it can get](rust-lang/trait-system-refactor-initiative#73). Due to an improved structure and far better caching, we believe that there is a lot of room for improvement and that the new solver will outperform the existing implementation in nearly all cases, sometimes significantly. We have not yet spent any time micro-optimizing the implementation and have many unimplemented major improvements, such as fast-paths for trivial goals. TODO: get some rough results here and put them in a table ### Unstable features #### Unsupported unstable features The new solver currently does not support all unstable features, most notably `#![feature(generic_const_exprs)]`, `#![feature(associated_const_equality)]` and `#![feature(adt_const_params)]` are not yet fully supported in the new solver. We are confident that supporting them is possible, but did not consider this to be a priority. This stabilization introduces new ICE when using these features in impl headers. #### fixes to `#![feature(specialization)]` - fixes rust-lang#105782 - fixes rust-lang#118987 #### fixes to `#![feature(type_alias_impl_trait)]` - fixes rust-lang#119272 - rust-lang#105787 (comment) - fixes rust-lang#124207 ## This does not stabilize the whole solver While this stabilizes the use of the new solver in coherence checking, there are many parts of the solver which will remain fully unstable. We may still adapt these areas while working towards stabilizing the new solver everywhere. We are confident that we are able to do so without negatively impacting coherence. ### goals with a non-empty `ParamEnv` Coherence always uses an empty environment. We therefore do not depend on the behavior of `AliasBound` and `ParamEnv` candidates. We only stabilizes the behavior of user-defined and builtin implementations of traits. There are still many open questions there. ### opaque types in the defining scope The handling of opaque types - `impl Trait` - in both the new and old solver is still not fully figured out. Luckily this can be ignored for now. While opaque types are reachable during coherence checking by using `impl_trait_in_associated_types`, the behavior during coherence is separate and self-contained. The old and new solver fully agree here. ### normalization is hard This stabilizes that we equate associated types involving bound variables using deferred-alias-equality. We also stop eagerly normalizing in coherence, which should not have any user-facing impact. We do not stabilize the normalization behavior outside of coherence, e.g. we currently deeply normalize all types during writeback with the new solver. This may change going forward ### how to replace `select` from the old solver We sometimes depend on getting a single `impl` for a given trait bound, e.g. when resolving a concrete method for codegen/CTFE. We do not depend on this during coherence, so the exact approach here can still be freely changed going forward. ## Acknowledgements This work would not have been possible without `@compiler-errors.` He implemented large chunks of the solver himself but also and did a lot of testing and experimentation, eagerly discovering multiple issues which had a significant impact on our approach. `@BoxyUwU` has also done some amazing work on the solver. Thank you for the endless hours of discussion resulting in the current approach. Especially the way aliases are handled has gone through multiple revisions to get to its current state. There were also many contributions from - and discussions with - other members of the community and the rest of `@rust-lang/types.` This solver builds upon previous improvements to the compiler, as well as lessons learned from `chalk` and `a-mir-formality`. Getting to this point would not have been possible without that and I am incredibly thankful to everyone involved. See the [list of relevant PRs](https://github.com/rust-lang/rust/pulls?q=is%3Apr+is%3Amerged+label%3AWG-trait-system-refactor+-label%3Arollup+closed%3A%3C2024-03-22+).
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 16, 2024
stabilize `-Znext-solver=coherence` r? `@compiler-errors` --- This PR stabilizes the use of the next generation trait solver in coherence checking by enabling `-Znext-solver=coherence` by default. More specifically its use in the *implicit negative overlap check*. The tracking issue for this is rust-lang#114862. ## Background ### The next generation trait solver The new solver lives in [`rustc_trait_selection::solve`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_trait_selection/src/solve/mod.rs) and is intended to replace the existing *evaluate*, *fulfill*, and *project* implementation. It also has a wider impact on the rest of the type system, for example by changing our approach to handling associated types. For a more detailed explanation of the new trait solver, see the [rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/solve/trait-solving.html). This does not stabilize the current behavior of the new trait solver, only the behavior impacting the implicit negative overlap check. There are many areas in the new solver which are not yet finalized. We are confident that their final design will not conflict with the user-facing behavior observable via coherence. More on that further down. Please check out [the chapter](https://rustc-dev-guide.rust-lang.org/solve/significant-changes.html) summarizing the most significant changes between the existing and new implementations. ### Coherence and the implicit negative overlap check Coherence checking detects any overlapping impls. Overlapping trait impls always error while overlapping inherent impls result in an error if they have methods with the same name. Coherence also results in an error if any other impls could exist, even if they are currently unknown. This affects impls which may get added to upstream crates in a backwards compatible way and impls from downstream crates. Coherence failing to detect overlap is generally considered to be unsound, even if it is difficult to actually get runtime UB this way. It is quite easy to get ICEs due to bugs in coherence. It currently consists of two checks: The [orphan check] validates that impls do not overlap with other impls we do not know about: either because they may be defined in a sibling crate, or because an upstream crate is allowed to add it without being considered a breaking change. The [overlap check] validates that impls do not overlap with other impls we know about. This is done as follows: - Instantiate the generic parameters of both impls with inference variables - Equate the `TraitRef`s of both impls. If it fails there is no overlap. - [implicit negative]: Check whether any of the instantiated `where`-bounds of one of the impls definitely do not hold when using the constraints from the previous step. If a `where`-bound does not hold, there is no overlap. - *explicit negative (still unstable, ignored going forward)*: Check whether the any negated `where`-bounds can be proven, e.g. a `&mut u32: Clone` bound definitely does not hold as an explicit `impl<T> !Clone for &mut T` exists. The overlap check has to *prove that unifying the impls does not succeed*. This means that **incorrectly getting a type error during coherence is unsound** as it would allow impls to overlap: coherence has to be *complete*. Completeness means that we never incorrectly error. This means that during coherence we must only add inference constraints if they are definitely necessary. During ordinary type checking [this does not hold](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01d93b592bd9036ac96071cbf1d624a9), so the trait solver has to behave differently, depending on whether we're in coherence or not. The implicit negative check only considers goals to "definitely not hold" if they could not be implemented downstream, by a sibling, or upstream in a backwards compatible way. If the goal is is "unknowable" as it may get added in another crate, we add an ambiguous candidate: [source](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L858-L883). [orphan check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L566-L579 [overlap check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L92-L98 [implicit negative]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L223-L281 ## Motivation Replacing the existing solver in coherence fixes soundness bugs by removing sources of incompleteness in the type system. The new solver separately strengthens coherence, resulting in more impls being disjoint and passing the coherence check. The concrete changes will be elaborated further down. We believe the stabilization to reduce the likelihood of future bugs in coherence as the new implementation is easier to understand and reason about. It allows us to remove the support for coherence and implicit-negative reasoning in the old solver, allowing us to remove some code and simplifying the old trait solver. We will only remove the old solver support once this stabilization has reached stable to make sure we're able to quickly revert in case any unexpected issues are detected before then. Stabilizing the use of the next-generation trait solver expresses our confidence that its current behavior is intended and our work towards enabling its use everywhere will not require any breaking changes to the areas used by coherence checking. We are also confident that we will be able to replace the existing solver everywhere, as maintaining two separate systems adds a significant maintainance burden. ## User-facing impact and reasoning ### Breakage due to improved handling of associated types The new solver fixes multiple issues related to associated types. As these issues caused coherence to consider more types distinct, fixing them results in more overlap errors. This is therefore a breaking change. #### Structurally relating aliases containing bound vars Fixes rust-lang#102048. In the existing solver relating ambiguous projections containing bound variables is structural. This is *incomplete* and allows overlapping impls. These was mostly not exploitable as the same issue also caused impls to not apply when trying to use them. The new solver defers alias-relating to a nested goal, fixing this issue: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Trait {} trait Project { type Assoc<'a>; } impl Project for u32 { type Assoc<'a> = &'a u32; } // Eagerly normalizing `<?infer as Project>::Assoc<'a>` is ambiguous, // so the old solver ended up structurally relating // // (?infer, for<'a> fn(<?infer as Project>::Assoc<'a>)) // // with // // ((u32, fn(&'a u32))) // // Equating `&'a u32` with `<u32 as Project>::Assoc<'a>` failed, even // though these types are equal modulo normalization. impl<T: Project> Trait for (T, for<'a> fn(<T as Project>::Assoc<'a>)) {} impl<'a> Trait for (u32, fn(&'a u32)) {} //[next]~^ ERROR conflicting implementations of trait `Trait` for type `(u32, for<'a> fn(&'a u32))` ``` A crater run did not discover any breakage due to this change. #### Unknowable candidates for higher ranked trait goals This avoids an unsoundness by attempting to normalize in `trait_ref_is_knowable`, fixing rust-lang#114061. This is a side-effect of supporting lazy normalization, as that forces us to attempt to normalize when checking whether a `TraitRef` is knowable: [source](https://github.com/rust-lang/rust/blob/47dd709bedda8127e8daec33327e0a9d0cdae845/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L754-L764). ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait IsUnit {} impl IsUnit for () {} pub trait WithAssoc<'a> { type Assoc; } // We considered `for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit` // to be knowable, even though the projection is ambiguous. pub trait Trait {} impl<T> Trait for T where T: 'static, for<'a> T: WithAssoc<'a>, for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit, { } impl<T> Trait for Box<T> {} //[next]~^ ERROR conflicting implementations of trait `Trait` ``` The two impls of `Trait` overlap given the following downstream crate: ```rust use dep::*; struct Local; impl WithAssoc<'_> for Box<Local> { type Assoc = (); } ``` There a similar coherence unsoundness caused by our handling of aliases which is fixed separately in rust-lang#117164. This change breaks the [`derive-visitor`](https://crates.io/crates/derive-visitor) crate. I have opened an issue in that repo: nikis05/derive-visitor#16. ### Evaluating goals to a fixpoint and applying inference constraints In the old implementation of the implicit-negative check, each obligation is [checked separately without applying its inference constraints](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L323-L338). The new solver instead [uses a `FulfillmentCtxt`](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L315-L321) for this, which evaluates all obligations in a loop until there's no further inference progress. This is necessary for backwards compatibility as we do not eagerly normalize with the new solver, resulting in constraints from normalization to only get applied by evaluating a separate obligation. This also allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Mirror { type Assoc; } impl<T> Mirror for T { type Assoc = T; } trait Foo {} trait Bar {} // The self type starts out as `?0` but is constrained to `()` // due to the where-clause below. Because `(): Bar` is known to // not hold, we can prove the impls disjoint. impl<T> Foo for T where (): Mirror<Assoc = T> {} //[current]~^ ERROR conflicting implementations of trait `Foo` for type `()` impl<T> Foo for T where T: Bar {} fn main() {} ``` The old solver does not run nested goals to a fixpoint in evaluation. The new solver does do so, strengthening inference and improving the overlap check: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Foo {} impl<T> Foo for (u8, T, T) {} trait NotU8 {} trait Bar {} impl<T, U: NotU8> Bar for (T, T, U) {} trait NeedsFixpoint {} impl<T: Foo + Bar> NeedsFixpoint for T {} impl NeedsFixpoint for (u8, u8, u8) {} trait Overlap {} impl<T: NeedsFixpoint> Overlap for T {} impl<T, U: NotU8, V> Overlap for (T, U, V) {} //[current]~^ ERROR conflicting implementations of trait `Foo` ``` ### Breakage due to removal of incomplete candidate preference Fixes rust-lang#107887. In the old solver we incompletely prefer the builtin trait object impl over user defined impls. This can break inference guidance, inferring `?x` in `dyn Trait<u32>: Trait<?x>` to `u32`, even if an explicit impl of `Trait<u64>` also exists. This caused coherence to incorrectly allow overlapping impls, resulting in ICEs and a theoretical unsoundness. See rust-lang#107887 (comment). This compiles on stable but results in an overlap error with `-Znext-solver=coherence`: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence struct W<T: ?Sized>(*const T); trait Trait<T: ?Sized> { type Assoc; } // This would trigger the check for overlap between automatic and custom impl. // They actually don't overlap so an impl like this should remain possible // forever. // // impl Trait<u64> for dyn Trait<u32> {} trait Indirect {} impl Indirect for dyn Trait<u32, Assoc = ()> {} impl<T: Indirect + ?Sized> Trait<u64> for T { type Assoc = (); } // Incomplete impl where `dyn Trait<u32>: Trait<_>` does not hold, but // `dyn Trait<u32>: Trait<u64>` does. trait EvaluateHack<U: ?Sized> {} impl<T: ?Sized, U: ?Sized> EvaluateHack<W<U>> for T where T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` U: IsU64, T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` { } trait IsU64 {} impl IsU64 for u64 {} trait Overlap<U: ?Sized> { type Assoc: Default; } impl<T: ?Sized + EvaluateHack<W<U>>, U: ?Sized> Overlap<U> for T { type Assoc = Box<u32>; } impl<U: ?Sized> Overlap<U> for dyn Trait<u32, Assoc = ()> { //[next]~^ ERROR conflicting implementations of trait `Overlap<_>` type Assoc = usize; } ``` ### Considering region outlives bounds in the `leak_check` For details on the `leak_check`, see the FCP proposal in rust-lang#119820.[^leak_check] [^leak_check]: which should get moved to the dev-guide once that PR lands :3 In both coherence and during candidate selection, the `leak_check` relies on the region constraints added in `evaluate`. It therefore currently does not register outlives obligations: [source](https://github.com/rust-lang/rust/blob/ccb1415eac3289b5ebf64691c0190dc52e0e3d0e/compiler/rustc_trait_selection/src/traits/select/mod.rs#L792-L810). This was likely done as a performance optimization without considering its impact on the `leak_check`. This is the case as in the old solver, *evaluatation* and *fulfillment* are split, with evaluation being responsible for candidate selection and fulfillment actually registering all the constraints. This split does not exist with the new solver. The `leak_check` can therefore eagerly detect errors caused by region outlives obligations. This improves both coherence itself and candidate selection: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait LeakErr<'a, 'b> {} // Using this impl adds an `'b: 'a` bound which results // in a higher-ranked region error. This bound has been // previously ignored but is now considered. impl<'a, 'b: 'a> LeakErr<'a, 'b> for () {} trait NoOverlapDir<'a> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> NoOverlapDir<'a> for T {} impl<'a> NoOverlapDir<'a> for () {} //[current]~^ ERROR conflicting implementations of trait `NoOverlapDir<'_>` // -------------------------------------- // necessary to avoid coherence unknowable candidates struct W<T>(T); trait GuidesSelection<'a, U> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> GuidesSelection<'a, W<u32>> for T {} impl<'a, T> GuidesSelection<'a, W<u8>> for T {} trait NotImplementedByU8 {} trait NoOverlapInd<'a, U> {} impl<'a, T: GuidesSelection<'a, W<U>>, U> NoOverlapInd<'a, U> for T {} impl<'a, U: NotImplementedByU8> NoOverlapInd<'a, U> for () {} //[current]~^ conflicting implementations of trait `NoOverlapInd<'_, _>` ``` ### Removal of `fn match_fresh_trait_refs` The old solver tries to [eagerly detect unbounded recursion](https://github.com/rust-lang/rust/blob/b14fd2359f47fb9a14bbfe55359db4bb3af11861/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1196-L1211), forcing the affected goals to be ambiguous. This check is only an approximation and has not been added to the new solver. The check is not necessary in the new solver and it would be problematic for caching. As it depends on all goals currently on the stack, using a global cache entry would have to always make sure that doing so does not circumvent this check. This changes some goals to error - or succeed - instead of failing with ambiguity. This allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence // Need to use this local wrapper for the impls to be fully // knowable as unknowable candidate result in ambiguity. struct Local<T>(T); trait Trait<U> {} // This impl does not hold, but is ambiguous in the old // solver due to its overflow approximation. impl<U> Trait<U> for Local<u32> where Local<u16>: Trait<U> {} // This impl holds. impl Trait<Local<()>> for Local<u8> {} // In the old solver, `Local<?t>: Trait<Local<?u>>` is ambiguous, // resulting in `Local<?u>: NoImpl`, also being ambiguous. // // In the new solver the first impl does not apply, constraining // `?u` to `Local<()>`, causing `Local<()>: NoImpl` to error. trait Indirect<T> {} impl<T, U> Indirect<U> for T where T: Trait<U>, U: NoImpl {} // Not implemented for `Local<()>` trait NoImpl {} impl NoImpl for Local<u8> {} impl NoImpl for Local<u16> {} // `Local<?t>: Indirect<Local<?u>>` cannot hold, so // these impls do not overlap. trait NoOverlap<U> {} impl<T: Indirect<U>, U> NoOverlap<U> for T {} impl<T, U> NoOverlap<Local<U>> for Local<T> {} //~^ ERROR conflicting implementations of trait `NoOverlap<Local<_>>` ``` ### Non-fatal overflow The old solver immediately emits a fatal error when hitting the recursion limit. The new solver instead returns overflow. This both allows more code to compile and is results in performance and potential future compatability issues. Non-fatal overflow is generally desirable. With fatal overflow, changing the order in which we evaluate nested goals easily causes breakage if we have goal which errors and one which overflows. It is also required to prevent breakage due to the removal of `fn match_fresh_trait_refs`, e.g. [in `typenum`](rust-lang/trait-system-refactor-initiative#73). #### Enabling more code to compile In the below example, the old solver first tried to prove an overflowing goal, resulting in a fatal error. The new solver instead returns ambiguity due to overflow for that goal, causing the implicit negative overlap check to succeed as `Box<u32>: NotImplemented` does not hold. ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence //[current] ERROR overflow evaluating the requirement trait Indirect<T> {} impl<T: Overflow<()>> Indirect<T> for () {} trait Overflow<U> {} impl<T, U> Overflow<U> for Box<T> where U: Indirect<Box<Box<T>>>, {} trait NotImplemented {} trait Trait<U> {} impl<T, U> Trait<U> for T where // T: NotImplemented, // causes old solver to succeed U: Indirect<T>, T: NotImplemented, {} impl Trait<()> for Box<u32> {} ``` #### Avoiding hangs with non-fatal overflow Simply returning ambiguity when reaching the recursion limit can very easily result in hangs, e.g. ```rust trait Recur {} impl<T, U> Recur for ((T, U), (U, T)) where (T, U): Recur, (U, T): Recur, {} trait NotImplemented {} impl<T: NotImplemented> Recur for T {} ``` This can happen quite frequently as it's easy to have exponential blowup due to multiple nested goals at each step. As the trait solver is depth-first, this immediately caused a fatal overflow error in the old solver. In the new solver we have to handle the whole proof tree instead, which can very easily hang. To avoid this we restrict the recursion depth after hitting the recursion limit for the first time. We also **ignore all inference constraints from goals resulting in overflow**. This is mostly backwards compatible as any overflow in the old solver resulted in a fatal error. ### sidenote about normalization We return ambiguous nested goals of `NormalizesTo` goals to the caller and ignore their impact when computing the `Certainty` of the current goal. See the [normalization chapter](https://rustc-dev-guide.rust-lang.org/solve/normalization.html) for more details.This means we apply constraints resulting from other nested goals and from equating the impl header when normalizing, even if a nested goal results in overflow. This is necessary to avoid breaking the following example: ```rust trait Trait { type Assoc; } struct W<T: ?Sized>(*mut T); impl<T: ?Sized> Trait for W<W<T>> where W<T>: Trait, { type Assoc = (); } // `W<?t>: Trait<Assoc = u32>` does not hold as // `Assoc` gets normalized to `()`. However, proving // the where-bounds of the impl results in overflow. // // For this to continue to compile we must not discard // constraints from normalizing associated types. trait NoOverlap {} impl<T: Trait<Assoc = u32>> NoOverlap for T {} impl<T: ?Sized> NoOverlap for W<T> {} ``` #### Future compatability concerns Non-fatal overflow results in some unfortunate future compatability concerns. Changing the approach to avoid more hangs by more strongly penalizing overflow can cause breakage as we either drop constraints or ignore candidates necessary to successfully compile. Weakening the overflow penalities instead allows more code to compile and strengthens inference while potentially causing more code to hang. While the current approach is not perfect, we believe it to be good enough. We believe it to apply the necessary inference constraints to avoid breakage and expect there to not be any desirable patterns broken by our current penalities. Similarly we believe the current constraints to avoid most accidental hangs. Ignoring constraints of overflowing goals is especially useful, as it may allow major future optimizations to our overflow handling. See [this summary](https://hackmd.io/ATf4hN0NRY-w2LIVgeFsVg) and the linked documents in case you want to know more. ### changes to performance In general, trait solving during coherence checking is not significant for performance. Enabling the next-generation trait solver in coherence does not impact our compile time benchmarks. We are still unable to compile the benchmark suite when fully enabling the new trait solver. There are rare cases where the new solver has significantly worse performance due to non-fatal overflow, its reliance on fixpoint algorithms and the removal of the `fn match_fresh_trait_refs` approximation. We encountered such issues in [`typenum`](https://crates.io/crates/typenum) and believe it should be [pretty much as bad as it can get](rust-lang/trait-system-refactor-initiative#73). Due to an improved structure and far better caching, we believe that there is a lot of room for improvement and that the new solver will outperform the existing implementation in nearly all cases, sometimes significantly. We have not yet spent any time micro-optimizing the implementation and have many unimplemented major improvements, such as fast-paths for trivial goals. TODO: get some rough results here and put them in a table ### Unstable features #### Unsupported unstable features The new solver currently does not support all unstable features, most notably `#![feature(generic_const_exprs)]`, `#![feature(associated_const_equality)]` and `#![feature(adt_const_params)]` are not yet fully supported in the new solver. We are confident that supporting them is possible, but did not consider this to be a priority. This stabilization introduces new ICE when using these features in impl headers. #### fixes to `#![feature(specialization)]` - fixes rust-lang#105782 - fixes rust-lang#118987 #### fixes to `#![feature(type_alias_impl_trait)]` - fixes rust-lang#119272 - rust-lang#105787 (comment) - fixes rust-lang#124207 ## This does not stabilize the whole solver While this stabilizes the use of the new solver in coherence checking, there are many parts of the solver which will remain fully unstable. We may still adapt these areas while working towards stabilizing the new solver everywhere. We are confident that we are able to do so without negatively impacting coherence. ### goals with a non-empty `ParamEnv` Coherence always uses an empty environment. We therefore do not depend on the behavior of `AliasBound` and `ParamEnv` candidates. We only stabilizes the behavior of user-defined and builtin implementations of traits. There are still many open questions there. ### opaque types in the defining scope The handling of opaque types - `impl Trait` - in both the new and old solver is still not fully figured out. Luckily this can be ignored for now. While opaque types are reachable during coherence checking by using `impl_trait_in_associated_types`, the behavior during coherence is separate and self-contained. The old and new solver fully agree here. ### normalization is hard This stabilizes that we equate associated types involving bound variables using deferred-alias-equality. We also stop eagerly normalizing in coherence, which should not have any user-facing impact. We do not stabilize the normalization behavior outside of coherence, e.g. we currently deeply normalize all types during writeback with the new solver. This may change going forward ### how to replace `select` from the old solver We sometimes depend on getting a single `impl` for a given trait bound, e.g. when resolving a concrete method for codegen/CTFE. We do not depend on this during coherence, so the exact approach here can still be freely changed going forward. ## Acknowledgements This work would not have been possible without `@compiler-errors.` He implemented large chunks of the solver himself but also and did a lot of testing and experimentation, eagerly discovering multiple issues which had a significant impact on our approach. `@BoxyUwU` has also done some amazing work on the solver. Thank you for the endless hours of discussion resulting in the current approach. Especially the way aliases are handled has gone through multiple revisions to get to its current state. There were also many contributions from - and discussions with - other members of the community and the rest of `@rust-lang/types.` This solver builds upon previous improvements to the compiler, as well as lessons learned from `chalk` and `a-mir-formality`. Getting to this point would not have been possible without that and I am incredibly thankful to everyone involved. See the [list of relevant PRs](https://github.com/rust-lang/rust/pulls?q=is%3Apr+is%3Amerged+label%3AWG-trait-system-refactor+-label%3Arollup+closed%3A%3C2024-03-22+).
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 6, 2024
…er-errors stabilize `-Znext-solver=coherence` r? `@compiler-errors` --- This PR stabilizes the use of the next generation trait solver in coherence checking by enabling `-Znext-solver=coherence` by default. More specifically its use in the *implicit negative overlap check*. The tracking issue for this is rust-lang#114862. Closes rust-lang#114862. ## Background ### The next generation trait solver The new solver lives in [`rustc_trait_selection::solve`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_trait_selection/src/solve/mod.rs) and is intended to replace the existing *evaluate*, *fulfill*, and *project* implementation. It also has a wider impact on the rest of the type system, for example by changing our approach to handling associated types. For a more detailed explanation of the new trait solver, see the [rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/solve/trait-solving.html). This does not stabilize the current behavior of the new trait solver, only the behavior impacting the implicit negative overlap check. There are many areas in the new solver which are not yet finalized. We are confident that their final design will not conflict with the user-facing behavior observable via coherence. More on that further down. Please check out [the chapter](https://rustc-dev-guide.rust-lang.org/solve/significant-changes.html) summarizing the most significant changes between the existing and new implementations. ### Coherence and the implicit negative overlap check Coherence checking detects any overlapping impls. Overlapping trait impls always error while overlapping inherent impls result in an error if they have methods with the same name. Coherence also results in an error if any other impls could exist, even if they are currently unknown. This affects impls which may get added to upstream crates in a backwards compatible way and impls from downstream crates. Coherence failing to detect overlap is generally considered to be unsound, even if it is difficult to actually get runtime UB this way. It is quite easy to get ICEs due to bugs in coherence. It currently consists of two checks: The [orphan check] validates that impls do not overlap with other impls we do not know about: either because they may be defined in a sibling crate, or because an upstream crate is allowed to add it without being considered a breaking change. The [overlap check] validates that impls do not overlap with other impls we know about. This is done as follows: - Instantiate the generic parameters of both impls with inference variables - Equate the `TraitRef`s of both impls. If it fails there is no overlap. - [implicit negative]: Check whether any of the instantiated `where`-bounds of one of the impls definitely do not hold when using the constraints from the previous step. If a `where`-bound does not hold, there is no overlap. - *explicit negative (still unstable, ignored going forward)*: Check whether the any negated `where`-bounds can be proven, e.g. a `&mut u32: Clone` bound definitely does not hold as an explicit `impl<T> !Clone for &mut T` exists. The overlap check has to *prove that unifying the impls does not succeed*. This means that **incorrectly getting a type error during coherence is unsound** as it would allow impls to overlap: coherence has to be *complete*. Completeness means that we never incorrectly error. This means that during coherence we must only add inference constraints if they are definitely necessary. During ordinary type checking [this does not hold](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01d93b592bd9036ac96071cbf1d624a9), so the trait solver has to behave differently, depending on whether we're in coherence or not. The implicit negative check only considers goals to "definitely not hold" if they could not be implemented downstream, by a sibling, or upstream in a backwards compatible way. If the goal is is "unknowable" as it may get added in another crate, we add an ambiguous candidate: [source](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L858-L883). [orphan check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L566-L579 [overlap check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L92-L98 [implicit negative]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L223-L281 ## Motivation Replacing the existing solver in coherence fixes soundness bugs by removing sources of incompleteness in the type system. The new solver separately strengthens coherence, resulting in more impls being disjoint and passing the coherence check. The concrete changes will be elaborated further down. We believe the stabilization to reduce the likelihood of future bugs in coherence as the new implementation is easier to understand and reason about. It allows us to remove the support for coherence and implicit-negative reasoning in the old solver, allowing us to remove some code and simplifying the old trait solver. We will only remove the old solver support once this stabilization has reached stable to make sure we're able to quickly revert in case any unexpected issues are detected before then. Stabilizing the use of the next-generation trait solver expresses our confidence that its current behavior is intended and our work towards enabling its use everywhere will not require any breaking changes to the areas used by coherence checking. We are also confident that we will be able to replace the existing solver everywhere, as maintaining two separate systems adds a significant maintainance burden. ## User-facing impact and reasoning ### Breakage due to improved handling of associated types The new solver fixes multiple issues related to associated types. As these issues caused coherence to consider more types distinct, fixing them results in more overlap errors. This is therefore a breaking change. #### Structurally relating aliases containing bound vars Fixes rust-lang#102048. In the existing solver relating ambiguous projections containing bound variables is structural. This is *incomplete* and allows overlapping impls. These was mostly not exploitable as the same issue also caused impls to not apply when trying to use them. The new solver defers alias-relating to a nested goal, fixing this issue: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Trait {} trait Project { type Assoc<'a>; } impl Project for u32 { type Assoc<'a> = &'a u32; } // Eagerly normalizing `<?infer as Project>::Assoc<'a>` is ambiguous, // so the old solver ended up structurally relating // // (?infer, for<'a> fn(<?infer as Project>::Assoc<'a>)) // // with // // ((u32, fn(&'a u32))) // // Equating `&'a u32` with `<u32 as Project>::Assoc<'a>` failed, even // though these types are equal modulo normalization. impl<T: Project> Trait for (T, for<'a> fn(<T as Project>::Assoc<'a>)) {} impl<'a> Trait for (u32, fn(&'a u32)) {} //[next]~^ ERROR conflicting implementations of trait `Trait` for type `(u32, for<'a> fn(&'a u32))` ``` A crater run did not discover any breakage due to this change. #### Unknowable candidates for higher ranked trait goals This avoids an unsoundness by attempting to normalize in `trait_ref_is_knowable`, fixing rust-lang#114061. This is a side-effect of supporting lazy normalization, as that forces us to attempt to normalize when checking whether a `TraitRef` is knowable: [source](https://github.com/rust-lang/rust/blob/47dd709bedda8127e8daec33327e0a9d0cdae845/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L754-L764). ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait IsUnit {} impl IsUnit for () {} pub trait WithAssoc<'a> { type Assoc; } // We considered `for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit` // to be knowable, even though the projection is ambiguous. pub trait Trait {} impl<T> Trait for T where T: 'static, for<'a> T: WithAssoc<'a>, for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit, { } impl<T> Trait for Box<T> {} //[next]~^ ERROR conflicting implementations of trait `Trait` ``` The two impls of `Trait` overlap given the following downstream crate: ```rust use dep::*; struct Local; impl WithAssoc<'_> for Box<Local> { type Assoc = (); } ``` There a similar coherence unsoundness caused by our handling of aliases which is fixed separately in rust-lang#117164. This change breaks the [`derive-visitor`](https://crates.io/crates/derive-visitor) crate. I have opened an issue in that repo: nikis05/derive-visitor#16. ### Evaluating goals to a fixpoint and applying inference constraints In the old implementation of the implicit-negative check, each obligation is [checked separately without applying its inference constraints](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L323-L338). The new solver instead [uses a `FulfillmentCtxt`](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L315-L321) for this, which evaluates all obligations in a loop until there's no further inference progress. This is necessary for backwards compatibility as we do not eagerly normalize with the new solver, resulting in constraints from normalization to only get applied by evaluating a separate obligation. This also allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Mirror { type Assoc; } impl<T> Mirror for T { type Assoc = T; } trait Foo {} trait Bar {} // The self type starts out as `?0` but is constrained to `()` // due to the where-clause below. Because `(): Bar` is known to // not hold, we can prove the impls disjoint. impl<T> Foo for T where (): Mirror<Assoc = T> {} //[current]~^ ERROR conflicting implementations of trait `Foo` for type `()` impl<T> Foo for T where T: Bar {} fn main() {} ``` The old solver does not run nested goals to a fixpoint in evaluation. The new solver does do so, strengthening inference and improving the overlap check: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Foo {} impl<T> Foo for (u8, T, T) {} trait NotU8 {} trait Bar {} impl<T, U: NotU8> Bar for (T, T, U) {} trait NeedsFixpoint {} impl<T: Foo + Bar> NeedsFixpoint for T {} impl NeedsFixpoint for (u8, u8, u8) {} trait Overlap {} impl<T: NeedsFixpoint> Overlap for T {} impl<T, U: NotU8, V> Overlap for (T, U, V) {} //[current]~^ ERROR conflicting implementations of trait `Foo` ``` ### Breakage due to removal of incomplete candidate preference Fixes rust-lang#107887. In the old solver we incompletely prefer the builtin trait object impl over user defined impls. This can break inference guidance, inferring `?x` in `dyn Trait<u32>: Trait<?x>` to `u32`, even if an explicit impl of `Trait<u64>` also exists. This caused coherence to incorrectly allow overlapping impls, resulting in ICEs and a theoretical unsoundness. See rust-lang#107887 (comment). This compiles on stable but results in an overlap error with `-Znext-solver=coherence`: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence struct W<T: ?Sized>(*const T); trait Trait<T: ?Sized> { type Assoc; } // This would trigger the check for overlap between automatic and custom impl. // They actually don't overlap so an impl like this should remain possible // forever. // // impl Trait<u64> for dyn Trait<u32> {} trait Indirect {} impl Indirect for dyn Trait<u32, Assoc = ()> {} impl<T: Indirect + ?Sized> Trait<u64> for T { type Assoc = (); } // Incomplete impl where `dyn Trait<u32>: Trait<_>` does not hold, but // `dyn Trait<u32>: Trait<u64>` does. trait EvaluateHack<U: ?Sized> {} impl<T: ?Sized, U: ?Sized> EvaluateHack<W<U>> for T where T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` U: IsU64, T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` { } trait IsU64 {} impl IsU64 for u64 {} trait Overlap<U: ?Sized> { type Assoc: Default; } impl<T: ?Sized + EvaluateHack<W<U>>, U: ?Sized> Overlap<U> for T { type Assoc = Box<u32>; } impl<U: ?Sized> Overlap<U> for dyn Trait<u32, Assoc = ()> { //[next]~^ ERROR conflicting implementations of trait `Overlap<_>` type Assoc = usize; } ``` ### Considering region outlives bounds in the `leak_check` For details on the `leak_check`, see the FCP proposal in rust-lang#119820.[^leak_check] [^leak_check]: which should get moved to the dev-guide once that PR lands :3 In both coherence and during candidate selection, the `leak_check` relies on the region constraints added in `evaluate`. It therefore currently does not register outlives obligations: [source](https://github.com/rust-lang/rust/blob/ccb1415eac3289b5ebf64691c0190dc52e0e3d0e/compiler/rustc_trait_selection/src/traits/select/mod.rs#L792-L810). This was likely done as a performance optimization without considering its impact on the `leak_check`. This is the case as in the old solver, *evaluatation* and *fulfillment* are split, with evaluation being responsible for candidate selection and fulfillment actually registering all the constraints. This split does not exist with the new solver. The `leak_check` can therefore eagerly detect errors caused by region outlives obligations. This improves both coherence itself and candidate selection: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait LeakErr<'a, 'b> {} // Using this impl adds an `'b: 'a` bound which results // in a higher-ranked region error. This bound has been // previously ignored but is now considered. impl<'a, 'b: 'a> LeakErr<'a, 'b> for () {} trait NoOverlapDir<'a> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> NoOverlapDir<'a> for T {} impl<'a> NoOverlapDir<'a> for () {} //[current]~^ ERROR conflicting implementations of trait `NoOverlapDir<'_>` // -------------------------------------- // necessary to avoid coherence unknowable candidates struct W<T>(T); trait GuidesSelection<'a, U> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> GuidesSelection<'a, W<u32>> for T {} impl<'a, T> GuidesSelection<'a, W<u8>> for T {} trait NotImplementedByU8 {} trait NoOverlapInd<'a, U> {} impl<'a, T: GuidesSelection<'a, W<U>>, U> NoOverlapInd<'a, U> for T {} impl<'a, U: NotImplementedByU8> NoOverlapInd<'a, U> for () {} //[current]~^ conflicting implementations of trait `NoOverlapInd<'_, _>` ``` ### Removal of `fn match_fresh_trait_refs` The old solver tries to [eagerly detect unbounded recursion](https://github.com/rust-lang/rust/blob/b14fd2359f47fb9a14bbfe55359db4bb3af11861/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1196-L1211), forcing the affected goals to be ambiguous. This check is only an approximation and has not been added to the new solver. The check is not necessary in the new solver and it would be problematic for caching. As it depends on all goals currently on the stack, using a global cache entry would have to always make sure that doing so does not circumvent this check. This changes some goals to error - or succeed - instead of failing with ambiguity. This allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence // Need to use this local wrapper for the impls to be fully // knowable as unknowable candidate result in ambiguity. struct Local<T>(T); trait Trait<U> {} // This impl does not hold, but is ambiguous in the old // solver due to its overflow approximation. impl<U> Trait<U> for Local<u32> where Local<u16>: Trait<U> {} // This impl holds. impl Trait<Local<()>> for Local<u8> {} // In the old solver, `Local<?t>: Trait<Local<?u>>` is ambiguous, // resulting in `Local<?u>: NoImpl`, also being ambiguous. // // In the new solver the first impl does not apply, constraining // `?u` to `Local<()>`, causing `Local<()>: NoImpl` to error. trait Indirect<T> {} impl<T, U> Indirect<U> for T where T: Trait<U>, U: NoImpl {} // Not implemented for `Local<()>` trait NoImpl {} impl NoImpl for Local<u8> {} impl NoImpl for Local<u16> {} // `Local<?t>: Indirect<Local<?u>>` cannot hold, so // these impls do not overlap. trait NoOverlap<U> {} impl<T: Indirect<U>, U> NoOverlap<U> for T {} impl<T, U> NoOverlap<Local<U>> for Local<T> {} //~^ ERROR conflicting implementations of trait `NoOverlap<Local<_>>` ``` ### Non-fatal overflow The old solver immediately emits a fatal error when hitting the recursion limit. The new solver instead returns overflow. This both allows more code to compile and is results in performance and potential future compatability issues. Non-fatal overflow is generally desirable. With fatal overflow, changing the order in which we evaluate nested goals easily causes breakage if we have goal which errors and one which overflows. It is also required to prevent breakage due to the removal of `fn match_fresh_trait_refs`, e.g. [in `typenum`](rust-lang/trait-system-refactor-initiative#73). #### Enabling more code to compile In the below example, the old solver first tried to prove an overflowing goal, resulting in a fatal error. The new solver instead returns ambiguity due to overflow for that goal, causing the implicit negative overlap check to succeed as `Box<u32>: NotImplemented` does not hold. ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence //[current] ERROR overflow evaluating the requirement trait Indirect<T> {} impl<T: Overflow<()>> Indirect<T> for () {} trait Overflow<U> {} impl<T, U> Overflow<U> for Box<T> where U: Indirect<Box<Box<T>>>, {} trait NotImplemented {} trait Trait<U> {} impl<T, U> Trait<U> for T where // T: NotImplemented, // causes old solver to succeed U: Indirect<T>, T: NotImplemented, {} impl Trait<()> for Box<u32> {} ``` #### Avoiding hangs with non-fatal overflow Simply returning ambiguity when reaching the recursion limit can very easily result in hangs, e.g. ```rust trait Recur {} impl<T, U> Recur for ((T, U), (U, T)) where (T, U): Recur, (U, T): Recur, {} trait NotImplemented {} impl<T: NotImplemented> Recur for T {} ``` This can happen quite frequently as it's easy to have exponential blowup due to multiple nested goals at each step. As the trait solver is depth-first, this immediately caused a fatal overflow error in the old solver. In the new solver we have to handle the whole proof tree instead, which can very easily hang. To avoid this we restrict the recursion depth after hitting the recursion limit for the first time. We also **ignore all inference constraints from goals resulting in overflow**. This is mostly backwards compatible as any overflow in the old solver resulted in a fatal error. ### sidenote about normalization We return ambiguous nested goals of `NormalizesTo` goals to the caller and ignore their impact when computing the `Certainty` of the current goal. See the [normalization chapter](https://rustc-dev-guide.rust-lang.org/solve/normalization.html) for more details.This means we apply constraints resulting from other nested goals and from equating the impl header when normalizing, even if a nested goal results in overflow. This is necessary to avoid breaking the following example: ```rust trait Trait { type Assoc; } struct W<T: ?Sized>(*mut T); impl<T: ?Sized> Trait for W<W<T>> where W<T>: Trait, { type Assoc = (); } // `W<?t>: Trait<Assoc = u32>` does not hold as // `Assoc` gets normalized to `()`. However, proving // the where-bounds of the impl results in overflow. // // For this to continue to compile we must not discard // constraints from normalizing associated types. trait NoOverlap {} impl<T: Trait<Assoc = u32>> NoOverlap for T {} impl<T: ?Sized> NoOverlap for W<T> {} ``` #### Future compatability concerns Non-fatal overflow results in some unfortunate future compatability concerns. Changing the approach to avoid more hangs by more strongly penalizing overflow can cause breakage as we either drop constraints or ignore candidates necessary to successfully compile. Weakening the overflow penalities instead allows more code to compile and strengthens inference while potentially causing more code to hang. While the current approach is not perfect, we believe it to be good enough. We believe it to apply the necessary inference constraints to avoid breakage and expect there to not be any desirable patterns broken by our current penalities. Similarly we believe the current constraints to avoid most accidental hangs. Ignoring constraints of overflowing goals is especially useful, as it may allow major future optimizations to our overflow handling. See [this summary](https://hackmd.io/ATf4hN0NRY-w2LIVgeFsVg) and the linked documents in case you want to know more. ### changes to performance In general, trait solving during coherence checking is not significant for performance. Enabling the next-generation trait solver in coherence does not impact our compile time benchmarks. We are still unable to compile the benchmark suite when fully enabling the new trait solver. There are rare cases where the new solver has significantly worse performance due to non-fatal overflow, its reliance on fixpoint algorithms and the removal of the `fn match_fresh_trait_refs` approximation. We encountered such issues in [`typenum`](https://crates.io/crates/typenum) and believe it should be [pretty much as bad as it can get](rust-lang/trait-system-refactor-initiative#73). Due to an improved structure and far better caching, we believe that there is a lot of room for improvement and that the new solver will outperform the existing implementation in nearly all cases, sometimes significantly. We have not yet spent any time micro-optimizing the implementation and have many unimplemented major improvements, such as fast-paths for trivial goals. TODO: get some rough results here and put them in a table ### Unstable features #### Unsupported unstable features The new solver currently does not support all unstable features, most notably `#![feature(generic_const_exprs)]`, `#![feature(associated_const_equality)]` and `#![feature(adt_const_params)]` are not yet fully supported in the new solver. We are confident that supporting them is possible, but did not consider this to be a priority. This stabilization introduces new ICE when using these features in impl headers. #### fixes to `#![feature(specialization)]` - fixes rust-lang#105782 - fixes rust-lang#118987 #### fixes to `#![feature(type_alias_impl_trait)]` - fixes rust-lang#119272 - rust-lang#105787 (comment) - fixes rust-lang#124207 ## This does not stabilize the whole solver While this stabilizes the use of the new solver in coherence checking, there are many parts of the solver which will remain fully unstable. We may still adapt these areas while working towards stabilizing the new solver everywhere. We are confident that we are able to do so without negatively impacting coherence. ### goals with a non-empty `ParamEnv` Coherence always uses an empty environment. We therefore do not depend on the behavior of `AliasBound` and `ParamEnv` candidates. We only stabilizes the behavior of user-defined and builtin implementations of traits. There are still many open questions there. ### opaque types in the defining scope The handling of opaque types - `impl Trait` - in both the new and old solver is still not fully figured out. Luckily this can be ignored for now. While opaque types are reachable during coherence checking by using `impl_trait_in_associated_types`, the behavior during coherence is separate and self-contained. The old and new solver fully agree here. ### normalization is hard This stabilizes that we equate associated types involving bound variables using deferred-alias-equality. We also stop eagerly normalizing in coherence, which should not have any user-facing impact. We do not stabilize the normalization behavior outside of coherence, e.g. we currently deeply normalize all types during writeback with the new solver. This may change going forward ### how to replace `select` from the old solver We sometimes depend on getting a single `impl` for a given trait bound, e.g. when resolving a concrete method for codegen/CTFE. We do not depend on this during coherence, so the exact approach here can still be freely changed going forward. ## Acknowledgements This work would not have been possible without `@compiler-errors.` He implemented large chunks of the solver himself but also and did a lot of testing and experimentation, eagerly discovering multiple issues which had a significant impact on our approach. `@BoxyUwU` has also done some amazing work on the solver. Thank you for the endless hours of discussion resulting in the current approach. Especially the way aliases are handled has gone through multiple revisions to get to its current state. There were also many contributions from - and discussions with - other members of the community and the rest of `@rust-lang/types.` This solver builds upon previous improvements to the compiler, as well as lessons learned from `chalk` and `a-mir-formality`. Getting to this point would not have been possible without that and I am incredibly thankful to everyone involved. See the [list of relevant PRs](https://github.com/rust-lang/rust/pulls?q=is%3Apr+is%3Amerged+label%3AWG-trait-system-refactor+-label%3Arollup+closed%3A%3C2024-03-22+).
RalfJung
pushed a commit
to RalfJung/miri
that referenced
this pull request
Sep 10, 2024
stabilize `-Znext-solver=coherence` r? `@compiler-errors` --- This PR stabilizes the use of the next generation trait solver in coherence checking by enabling `-Znext-solver=coherence` by default. More specifically its use in the *implicit negative overlap check*. The tracking issue for this is rust-lang/rust#114862. Closes #114862. ## Background ### The next generation trait solver The new solver lives in [`rustc_trait_selection::solve`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_trait_selection/src/solve/mod.rs) and is intended to replace the existing *evaluate*, *fulfill*, and *project* implementation. It also has a wider impact on the rest of the type system, for example by changing our approach to handling associated types. For a more detailed explanation of the new trait solver, see the [rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/solve/trait-solving.html). This does not stabilize the current behavior of the new trait solver, only the behavior impacting the implicit negative overlap check. There are many areas in the new solver which are not yet finalized. We are confident that their final design will not conflict with the user-facing behavior observable via coherence. More on that further down. Please check out [the chapter](https://rustc-dev-guide.rust-lang.org/solve/significant-changes.html) summarizing the most significant changes between the existing and new implementations. ### Coherence and the implicit negative overlap check Coherence checking detects any overlapping impls. Overlapping trait impls always error while overlapping inherent impls result in an error if they have methods with the same name. Coherence also results in an error if any other impls could exist, even if they are currently unknown. This affects impls which may get added to upstream crates in a backwards compatible way and impls from downstream crates. Coherence failing to detect overlap is generally considered to be unsound, even if it is difficult to actually get runtime UB this way. It is quite easy to get ICEs due to bugs in coherence. It currently consists of two checks: The [orphan check] validates that impls do not overlap with other impls we do not know about: either because they may be defined in a sibling crate, or because an upstream crate is allowed to add it without being considered a breaking change. The [overlap check] validates that impls do not overlap with other impls we know about. This is done as follows: - Instantiate the generic parameters of both impls with inference variables - Equate the `TraitRef`s of both impls. If it fails there is no overlap. - [implicit negative]: Check whether any of the instantiated `where`-bounds of one of the impls definitely do not hold when using the constraints from the previous step. If a `where`-bound does not hold, there is no overlap. - *explicit negative (still unstable, ignored going forward)*: Check whether the any negated `where`-bounds can be proven, e.g. a `&mut u32: Clone` bound definitely does not hold as an explicit `impl<T> !Clone for &mut T` exists. The overlap check has to *prove that unifying the impls does not succeed*. This means that **incorrectly getting a type error during coherence is unsound** as it would allow impls to overlap: coherence has to be *complete*. Completeness means that we never incorrectly error. This means that during coherence we must only add inference constraints if they are definitely necessary. During ordinary type checking [this does not hold](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01d93b592bd9036ac96071cbf1d624a9), so the trait solver has to behave differently, depending on whether we're in coherence or not. The implicit negative check only considers goals to "definitely not hold" if they could not be implemented downstream, by a sibling, or upstream in a backwards compatible way. If the goal is is "unknowable" as it may get added in another crate, we add an ambiguous candidate: [source](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L858-L883). [orphan check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L566-L579 [overlap check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L92-L98 [implicit negative]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L223-L281 ## Motivation Replacing the existing solver in coherence fixes soundness bugs by removing sources of incompleteness in the type system. The new solver separately strengthens coherence, resulting in more impls being disjoint and passing the coherence check. The concrete changes will be elaborated further down. We believe the stabilization to reduce the likelihood of future bugs in coherence as the new implementation is easier to understand and reason about. It allows us to remove the support for coherence and implicit-negative reasoning in the old solver, allowing us to remove some code and simplifying the old trait solver. We will only remove the old solver support once this stabilization has reached stable to make sure we're able to quickly revert in case any unexpected issues are detected before then. Stabilizing the use of the next-generation trait solver expresses our confidence that its current behavior is intended and our work towards enabling its use everywhere will not require any breaking changes to the areas used by coherence checking. We are also confident that we will be able to replace the existing solver everywhere, as maintaining two separate systems adds a significant maintainance burden. ## User-facing impact and reasoning ### Breakage due to improved handling of associated types The new solver fixes multiple issues related to associated types. As these issues caused coherence to consider more types distinct, fixing them results in more overlap errors. This is therefore a breaking change. #### Structurally relating aliases containing bound vars Fixes rust-lang/rust#102048. In the existing solver relating ambiguous projections containing bound variables is structural. This is *incomplete* and allows overlapping impls. These was mostly not exploitable as the same issue also caused impls to not apply when trying to use them. The new solver defers alias-relating to a nested goal, fixing this issue: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Trait {} trait Project { type Assoc<'a>; } impl Project for u32 { type Assoc<'a> = &'a u32; } // Eagerly normalizing `<?infer as Project>::Assoc<'a>` is ambiguous, // so the old solver ended up structurally relating // // (?infer, for<'a> fn(<?infer as Project>::Assoc<'a>)) // // with // // ((u32, fn(&'a u32))) // // Equating `&'a u32` with `<u32 as Project>::Assoc<'a>` failed, even // though these types are equal modulo normalization. impl<T: Project> Trait for (T, for<'a> fn(<T as Project>::Assoc<'a>)) {} impl<'a> Trait for (u32, fn(&'a u32)) {} //[next]~^ ERROR conflicting implementations of trait `Trait` for type `(u32, for<'a> fn(&'a u32))` ``` A crater run did not discover any breakage due to this change. #### Unknowable candidates for higher ranked trait goals This avoids an unsoundness by attempting to normalize in `trait_ref_is_knowable`, fixing rust-lang/rust#114061. This is a side-effect of supporting lazy normalization, as that forces us to attempt to normalize when checking whether a `TraitRef` is knowable: [source](https://github.com/rust-lang/rust/blob/47dd709bedda8127e8daec33327e0a9d0cdae845/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L754-L764). ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait IsUnit {} impl IsUnit for () {} pub trait WithAssoc<'a> { type Assoc; } // We considered `for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit` // to be knowable, even though the projection is ambiguous. pub trait Trait {} impl<T> Trait for T where T: 'static, for<'a> T: WithAssoc<'a>, for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit, { } impl<T> Trait for Box<T> {} //[next]~^ ERROR conflicting implementations of trait `Trait` ``` The two impls of `Trait` overlap given the following downstream crate: ```rust use dep::*; struct Local; impl WithAssoc<'_> for Box<Local> { type Assoc = (); } ``` There a similar coherence unsoundness caused by our handling of aliases which is fixed separately in rust-lang/rust#117164. This change breaks the [`derive-visitor`](https://crates.io/crates/derive-visitor) crate. I have opened an issue in that repo: nikis05/derive-visitor#16. ### Evaluating goals to a fixpoint and applying inference constraints In the old implementation of the implicit-negative check, each obligation is [checked separately without applying its inference constraints](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L323-L338). The new solver instead [uses a `FulfillmentCtxt`](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L315-L321) for this, which evaluates all obligations in a loop until there's no further inference progress. This is necessary for backwards compatibility as we do not eagerly normalize with the new solver, resulting in constraints from normalization to only get applied by evaluating a separate obligation. This also allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Mirror { type Assoc; } impl<T> Mirror for T { type Assoc = T; } trait Foo {} trait Bar {} // The self type starts out as `?0` but is constrained to `()` // due to the where-clause below. Because `(): Bar` is known to // not hold, we can prove the impls disjoint. impl<T> Foo for T where (): Mirror<Assoc = T> {} //[current]~^ ERROR conflicting implementations of trait `Foo` for type `()` impl<T> Foo for T where T: Bar {} fn main() {} ``` The old solver does not run nested goals to a fixpoint in evaluation. The new solver does do so, strengthening inference and improving the overlap check: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Foo {} impl<T> Foo for (u8, T, T) {} trait NotU8 {} trait Bar {} impl<T, U: NotU8> Bar for (T, T, U) {} trait NeedsFixpoint {} impl<T: Foo + Bar> NeedsFixpoint for T {} impl NeedsFixpoint for (u8, u8, u8) {} trait Overlap {} impl<T: NeedsFixpoint> Overlap for T {} impl<T, U: NotU8, V> Overlap for (T, U, V) {} //[current]~^ ERROR conflicting implementations of trait `Foo` ``` ### Breakage due to removal of incomplete candidate preference Fixes #107887. In the old solver we incompletely prefer the builtin trait object impl over user defined impls. This can break inference guidance, inferring `?x` in `dyn Trait<u32>: Trait<?x>` to `u32`, even if an explicit impl of `Trait<u64>` also exists. This caused coherence to incorrectly allow overlapping impls, resulting in ICEs and a theoretical unsoundness. See rust-lang/rust#107887 (comment). This compiles on stable but results in an overlap error with `-Znext-solver=coherence`: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence struct W<T: ?Sized>(*const T); trait Trait<T: ?Sized> { type Assoc; } // This would trigger the check for overlap between automatic and custom impl. // They actually don't overlap so an impl like this should remain possible // forever. // // impl Trait<u64> for dyn Trait<u32> {} trait Indirect {} impl Indirect for dyn Trait<u32, Assoc = ()> {} impl<T: Indirect + ?Sized> Trait<u64> for T { type Assoc = (); } // Incomplete impl where `dyn Trait<u32>: Trait<_>` does not hold, but // `dyn Trait<u32>: Trait<u64>` does. trait EvaluateHack<U: ?Sized> {} impl<T: ?Sized, U: ?Sized> EvaluateHack<W<U>> for T where T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` U: IsU64, T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` { } trait IsU64 {} impl IsU64 for u64 {} trait Overlap<U: ?Sized> { type Assoc: Default; } impl<T: ?Sized + EvaluateHack<W<U>>, U: ?Sized> Overlap<U> for T { type Assoc = Box<u32>; } impl<U: ?Sized> Overlap<U> for dyn Trait<u32, Assoc = ()> { //[next]~^ ERROR conflicting implementations of trait `Overlap<_>` type Assoc = usize; } ``` ### Considering region outlives bounds in the `leak_check` For details on the `leak_check`, see the FCP proposal in #119820.[^leak_check] [^leak_check]: which should get moved to the dev-guide once that PR lands :3 In both coherence and during candidate selection, the `leak_check` relies on the region constraints added in `evaluate`. It therefore currently does not register outlives obligations: [source](https://github.com/rust-lang/rust/blob/ccb1415eac3289b5ebf64691c0190dc52e0e3d0e/compiler/rustc_trait_selection/src/traits/select/mod.rs#L792-L810). This was likely done as a performance optimization without considering its impact on the `leak_check`. This is the case as in the old solver, *evaluatation* and *fulfillment* are split, with evaluation being responsible for candidate selection and fulfillment actually registering all the constraints. This split does not exist with the new solver. The `leak_check` can therefore eagerly detect errors caused by region outlives obligations. This improves both coherence itself and candidate selection: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait LeakErr<'a, 'b> {} // Using this impl adds an `'b: 'a` bound which results // in a higher-ranked region error. This bound has been // previously ignored but is now considered. impl<'a, 'b: 'a> LeakErr<'a, 'b> for () {} trait NoOverlapDir<'a> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> NoOverlapDir<'a> for T {} impl<'a> NoOverlapDir<'a> for () {} //[current]~^ ERROR conflicting implementations of trait `NoOverlapDir<'_>` // -------------------------------------- // necessary to avoid coherence unknowable candidates struct W<T>(T); trait GuidesSelection<'a, U> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> GuidesSelection<'a, W<u32>> for T {} impl<'a, T> GuidesSelection<'a, W<u8>> for T {} trait NotImplementedByU8 {} trait NoOverlapInd<'a, U> {} impl<'a, T: GuidesSelection<'a, W<U>>, U> NoOverlapInd<'a, U> for T {} impl<'a, U: NotImplementedByU8> NoOverlapInd<'a, U> for () {} //[current]~^ conflicting implementations of trait `NoOverlapInd<'_, _>` ``` ### Removal of `fn match_fresh_trait_refs` The old solver tries to [eagerly detect unbounded recursion](https://github.com/rust-lang/rust/blob/b14fd2359f47fb9a14bbfe55359db4bb3af11861/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1196-L1211), forcing the affected goals to be ambiguous. This check is only an approximation and has not been added to the new solver. The check is not necessary in the new solver and it would be problematic for caching. As it depends on all goals currently on the stack, using a global cache entry would have to always make sure that doing so does not circumvent this check. This changes some goals to error - or succeed - instead of failing with ambiguity. This allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence // Need to use this local wrapper for the impls to be fully // knowable as unknowable candidate result in ambiguity. struct Local<T>(T); trait Trait<U> {} // This impl does not hold, but is ambiguous in the old // solver due to its overflow approximation. impl<U> Trait<U> for Local<u32> where Local<u16>: Trait<U> {} // This impl holds. impl Trait<Local<()>> for Local<u8> {} // In the old solver, `Local<?t>: Trait<Local<?u>>` is ambiguous, // resulting in `Local<?u>: NoImpl`, also being ambiguous. // // In the new solver the first impl does not apply, constraining // `?u` to `Local<()>`, causing `Local<()>: NoImpl` to error. trait Indirect<T> {} impl<T, U> Indirect<U> for T where T: Trait<U>, U: NoImpl {} // Not implemented for `Local<()>` trait NoImpl {} impl NoImpl for Local<u8> {} impl NoImpl for Local<u16> {} // `Local<?t>: Indirect<Local<?u>>` cannot hold, so // these impls do not overlap. trait NoOverlap<U> {} impl<T: Indirect<U>, U> NoOverlap<U> for T {} impl<T, U> NoOverlap<Local<U>> for Local<T> {} //~^ ERROR conflicting implementations of trait `NoOverlap<Local<_>>` ``` ### Non-fatal overflow The old solver immediately emits a fatal error when hitting the recursion limit. The new solver instead returns overflow. This both allows more code to compile and is results in performance and potential future compatability issues. Non-fatal overflow is generally desirable. With fatal overflow, changing the order in which we evaluate nested goals easily causes breakage if we have goal which errors and one which overflows. It is also required to prevent breakage due to the removal of `fn match_fresh_trait_refs`, e.g. [in `typenum`](rust-lang/trait-system-refactor-initiative#73). #### Enabling more code to compile In the below example, the old solver first tried to prove an overflowing goal, resulting in a fatal error. The new solver instead returns ambiguity due to overflow for that goal, causing the implicit negative overlap check to succeed as `Box<u32>: NotImplemented` does not hold. ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence //[current] ERROR overflow evaluating the requirement trait Indirect<T> {} impl<T: Overflow<()>> Indirect<T> for () {} trait Overflow<U> {} impl<T, U> Overflow<U> for Box<T> where U: Indirect<Box<Box<T>>>, {} trait NotImplemented {} trait Trait<U> {} impl<T, U> Trait<U> for T where // T: NotImplemented, // causes old solver to succeed U: Indirect<T>, T: NotImplemented, {} impl Trait<()> for Box<u32> {} ``` #### Avoiding hangs with non-fatal overflow Simply returning ambiguity when reaching the recursion limit can very easily result in hangs, e.g. ```rust trait Recur {} impl<T, U> Recur for ((T, U), (U, T)) where (T, U): Recur, (U, T): Recur, {} trait NotImplemented {} impl<T: NotImplemented> Recur for T {} ``` This can happen quite frequently as it's easy to have exponential blowup due to multiple nested goals at each step. As the trait solver is depth-first, this immediately caused a fatal overflow error in the old solver. In the new solver we have to handle the whole proof tree instead, which can very easily hang. To avoid this we restrict the recursion depth after hitting the recursion limit for the first time. We also **ignore all inference constraints from goals resulting in overflow**. This is mostly backwards compatible as any overflow in the old solver resulted in a fatal error. ### sidenote about normalization We return ambiguous nested goals of `NormalizesTo` goals to the caller and ignore their impact when computing the `Certainty` of the current goal. See the [normalization chapter](https://rustc-dev-guide.rust-lang.org/solve/normalization.html) for more details.This means we apply constraints resulting from other nested goals and from equating the impl header when normalizing, even if a nested goal results in overflow. This is necessary to avoid breaking the following example: ```rust trait Trait { type Assoc; } struct W<T: ?Sized>(*mut T); impl<T: ?Sized> Trait for W<W<T>> where W<T>: Trait, { type Assoc = (); } // `W<?t>: Trait<Assoc = u32>` does not hold as // `Assoc` gets normalized to `()`. However, proving // the where-bounds of the impl results in overflow. // // For this to continue to compile we must not discard // constraints from normalizing associated types. trait NoOverlap {} impl<T: Trait<Assoc = u32>> NoOverlap for T {} impl<T: ?Sized> NoOverlap for W<T> {} ``` #### Future compatability concerns Non-fatal overflow results in some unfortunate future compatability concerns. Changing the approach to avoid more hangs by more strongly penalizing overflow can cause breakage as we either drop constraints or ignore candidates necessary to successfully compile. Weakening the overflow penalities instead allows more code to compile and strengthens inference while potentially causing more code to hang. While the current approach is not perfect, we believe it to be good enough. We believe it to apply the necessary inference constraints to avoid breakage and expect there to not be any desirable patterns broken by our current penalities. Similarly we believe the current constraints to avoid most accidental hangs. Ignoring constraints of overflowing goals is especially useful, as it may allow major future optimizations to our overflow handling. See [this summary](https://hackmd.io/ATf4hN0NRY-w2LIVgeFsVg) and the linked documents in case you want to know more. ### changes to performance In general, trait solving during coherence checking is not significant for performance. Enabling the next-generation trait solver in coherence does not impact our compile time benchmarks. We are still unable to compile the benchmark suite when fully enabling the new trait solver. There are rare cases where the new solver has significantly worse performance due to non-fatal overflow, its reliance on fixpoint algorithms and the removal of the `fn match_fresh_trait_refs` approximation. We encountered such issues in [`typenum`](https://crates.io/crates/typenum) and believe it should be [pretty much as bad as it can get](rust-lang/trait-system-refactor-initiative#73). Due to an improved structure and far better caching, we believe that there is a lot of room for improvement and that the new solver will outperform the existing implementation in nearly all cases, sometimes significantly. We have not yet spent any time micro-optimizing the implementation and have many unimplemented major improvements, such as fast-paths for trivial goals. TODO: get some rough results here and put them in a table ### Unstable features #### Unsupported unstable features The new solver currently does not support all unstable features, most notably `#![feature(generic_const_exprs)]`, `#![feature(associated_const_equality)]` and `#![feature(adt_const_params)]` are not yet fully supported in the new solver. We are confident that supporting them is possible, but did not consider this to be a priority. This stabilization introduces new ICE when using these features in impl headers. #### fixes to `#![feature(specialization)]` - fixes #105782 - fixes #118987 #### fixes to `#![feature(type_alias_impl_trait)]` - fixes #119272 - rust-lang/rust#105787 (comment) - fixes #124207 ## This does not stabilize the whole solver While this stabilizes the use of the new solver in coherence checking, there are many parts of the solver which will remain fully unstable. We may still adapt these areas while working towards stabilizing the new solver everywhere. We are confident that we are able to do so without negatively impacting coherence. ### goals with a non-empty `ParamEnv` Coherence always uses an empty environment. We therefore do not depend on the behavior of `AliasBound` and `ParamEnv` candidates. We only stabilizes the behavior of user-defined and builtin implementations of traits. There are still many open questions there. ### opaque types in the defining scope The handling of opaque types - `impl Trait` - in both the new and old solver is still not fully figured out. Luckily this can be ignored for now. While opaque types are reachable during coherence checking by using `impl_trait_in_associated_types`, the behavior during coherence is separate and self-contained. The old and new solver fully agree here. ### normalization is hard This stabilizes that we equate associated types involving bound variables using deferred-alias-equality. We also stop eagerly normalizing in coherence, which should not have any user-facing impact. We do not stabilize the normalization behavior outside of coherence, e.g. we currently deeply normalize all types during writeback with the new solver. This may change going forward ### how to replace `select` from the old solver We sometimes depend on getting a single `impl` for a given trait bound, e.g. when resolving a concrete method for codegen/CTFE. We do not depend on this during coherence, so the exact approach here can still be freely changed going forward. ## Acknowledgements This work would not have been possible without `@compiler-errors.` He implemented large chunks of the solver himself but also and did a lot of testing and experimentation, eagerly discovering multiple issues which had a significant impact on our approach. `@BoxyUwU` has also done some amazing work on the solver. Thank you for the endless hours of discussion resulting in the current approach. Especially the way aliases are handled has gone through multiple revisions to get to its current state. There were also many contributions from - and discussions with - other members of the community and the rest of `@rust-lang/types.` This solver builds upon previous improvements to the compiler, as well as lessons learned from `chalk` and `a-mir-formality`. Getting to this point would not have been possible without that and I am incredibly thankful to everyone involved. See the [list of relevant PRs](https://github.com/rust-lang/rust/pulls?q=is%3Apr+is%3Amerged+label%3AWG-trait-system-refactor+-label%3Arollup+closed%3A%3C2024-03-22+).
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 21, 2024
stabilize `-Znext-solver=coherence` again r? `@compiler-errors` --- This PR stabilizes the use of the next generation trait solver in coherence checking by enabling `-Znext-solver=coherence` by default. More specifically its use in the *implicit negative overlap check*. The tracking issue for this is rust-lang#114862. Closes rust-lang#114862. This is a direct copy of rust-lang#121848 which has been reverted due to a hang in `nalgebra`: rust-lang#130056. This hang should have been fixed by rust-lang#130617. See the added section in the stabilization report containing user facing changes merged since the original FCP. ## Background ### The next generation trait solver The new solver lives in [`rustc_trait_selection::solve`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_trait_selection/src/solve/mod.rs) and is intended to replace the existing *evaluate*, *fulfill*, and *project* implementation. It also has a wider impact on the rest of the type system, for example by changing our approach to handling associated types. For a more detailed explanation of the new trait solver, see the [rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/solve/trait-solving.html). This does not stabilize the current behavior of the new trait solver, only the behavior impacting the implicit negative overlap check. There are many areas in the new solver which are not yet finalized. We are confident that their final design will not conflict with the user-facing behavior observable via coherence. More on that further down. Please check out [the chapter](https://rustc-dev-guide.rust-lang.org/solve/significant-changes.html) summarizing the most significant changes between the existing and new implementations. ### Coherence and the implicit negative overlap check Coherence checking detects any overlapping impls. Overlapping trait impls always error while overlapping inherent impls result in an error if they have methods with the same name. Coherence also results in an error if any other impls could exist, even if they are currently unknown. This affects impls which may get added to upstream crates in a backwards compatible way and impls from downstream crates. Coherence failing to detect overlap is generally considered to be unsound, even if it is difficult to actually get runtime UB this way. It is quite easy to get ICEs due to bugs in coherence. It currently consists of two checks: The [orphan check] validates that impls do not overlap with other impls we do not know about: either because they may be defined in a sibling crate, or because an upstream crate is allowed to add it without being considered a breaking change. The [overlap check] validates that impls do not overlap with other impls we know about. This is done as follows: - Instantiate the generic parameters of both impls with inference variables - Equate the `TraitRef`s of both impls. If it fails there is no overlap. - [implicit negative]: Check whether any of the instantiated `where`-bounds of one of the impls definitely do not hold when using the constraints from the previous step. If a `where`-bound does not hold, there is no overlap. - *explicit negative (still unstable, ignored going forward)*: Check whether the any negated `where`-bounds can be proven, e.g. a `&mut u32: Clone` bound definitely does not hold as an explicit `impl<T> !Clone for &mut T` exists. The overlap check has to *prove that unifying the impls does not succeed*. This means that **incorrectly getting a type error during coherence is unsound** as it would allow impls to overlap: coherence has to be *complete*. Completeness means that we never incorrectly error. This means that during coherence we must only add inference constraints if they are definitely necessary. During ordinary type checking [this does not hold](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01d93b592bd9036ac96071cbf1d624a9), so the trait solver has to behave differently, depending on whether we're in coherence or not. The implicit negative check only considers goals to "definitely not hold" if they could not be implemented downstream, by a sibling, or upstream in a backwards compatible way. If the goal is is "unknowable" as it may get added in another crate, we add an ambiguous candidate: [source](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L858-L883). [orphan check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L566-L579 [overlap check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L92-L98 [implicit negative]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L223-L281 ## Motivation Replacing the existing solver in coherence fixes soundness bugs by removing sources of incompleteness in the type system. The new solver separately strengthens coherence, resulting in more impls being disjoint and passing the coherence check. The concrete changes will be elaborated further down. We believe the stabilization to reduce the likelihood of future bugs in coherence as the new implementation is easier to understand and reason about. It allows us to remove the support for coherence and implicit-negative reasoning in the old solver, allowing us to remove some code and simplifying the old trait solver. We will only remove the old solver support once this stabilization has reached stable to make sure we're able to quickly revert in case any unexpected issues are detected before then. Stabilizing the use of the next-generation trait solver expresses our confidence that its current behavior is intended and our work towards enabling its use everywhere will not require any breaking changes to the areas used by coherence checking. We are also confident that we will be able to replace the existing solver everywhere, as maintaining two separate systems adds a significant maintainance burden. ## User-facing impact and reasoning ### Breakage due to improved handling of associated types The new solver fixes multiple issues related to associated types. As these issues caused coherence to consider more types distinct, fixing them results in more overlap errors. This is therefore a breaking change. #### Structurally relating aliases containing bound vars Fixes rust-lang#102048. In the existing solver relating ambiguous projections containing bound variables is structural. This is *incomplete* and allows overlapping impls. These was mostly not exploitable as the same issue also caused impls to not apply when trying to use them. The new solver defers alias-relating to a nested goal, fixing this issue: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Trait {} trait Project { type Assoc<'a>; } impl Project for u32 { type Assoc<'a> = &'a u32; } // Eagerly normalizing `<?infer as Project>::Assoc<'a>` is ambiguous, // so the old solver ended up structurally relating // // (?infer, for<'a> fn(<?infer as Project>::Assoc<'a>)) // // with // // ((u32, fn(&'a u32))) // // Equating `&'a u32` with `<u32 as Project>::Assoc<'a>` failed, even // though these types are equal modulo normalization. impl<T: Project> Trait for (T, for<'a> fn(<T as Project>::Assoc<'a>)) {} impl<'a> Trait for (u32, fn(&'a u32)) {} //[next]~^ ERROR conflicting implementations of trait `Trait` for type `(u32, for<'a> fn(&'a u32))` ``` A crater run did not discover any breakage due to this change. #### Unknowable candidates for higher ranked trait goals This avoids an unsoundness by attempting to normalize in `trait_ref_is_knowable`, fixing rust-lang#114061. This is a side-effect of supporting lazy normalization, as that forces us to attempt to normalize when checking whether a `TraitRef` is knowable: [source](https://github.com/rust-lang/rust/blob/47dd709bedda8127e8daec33327e0a9d0cdae845/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L754-L764). ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait IsUnit {} impl IsUnit for () {} pub trait WithAssoc<'a> { type Assoc; } // We considered `for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit` // to be knowable, even though the projection is ambiguous. pub trait Trait {} impl<T> Trait for T where T: 'static, for<'a> T: WithAssoc<'a>, for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit, { } impl<T> Trait for Box<T> {} //[next]~^ ERROR conflicting implementations of trait `Trait` ``` The two impls of `Trait` overlap given the following downstream crate: ```rust use dep::*; struct Local; impl WithAssoc<'_> for Box<Local> { type Assoc = (); } ``` There a similar coherence unsoundness caused by our handling of aliases which is fixed separately in rust-lang#117164. This change breaks the [`derive-visitor`](https://crates.io/crates/derive-visitor) crate. I have opened an issue in that repo: nikis05/derive-visitor#16. ### Evaluating goals to a fixpoint and applying inference constraints In the old implementation of the implicit-negative check, each obligation is [checked separately without applying its inference constraints](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L323-L338). The new solver instead [uses a `FulfillmentCtxt`](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L315-L321) for this, which evaluates all obligations in a loop until there's no further inference progress. This is necessary for backwards compatibility as we do not eagerly normalize with the new solver, resulting in constraints from normalization to only get applied by evaluating a separate obligation. This also allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Mirror { type Assoc; } impl<T> Mirror for T { type Assoc = T; } trait Foo {} trait Bar {} // The self type starts out as `?0` but is constrained to `()` // due to the where-clause below. Because `(): Bar` is known to // not hold, we can prove the impls disjoint. impl<T> Foo for T where (): Mirror<Assoc = T> {} //[current]~^ ERROR conflicting implementations of trait `Foo` for type `()` impl<T> Foo for T where T: Bar {} fn main() {} ``` The old solver does not run nested goals to a fixpoint in evaluation. The new solver does do so, strengthening inference and improving the overlap check: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Foo {} impl<T> Foo for (u8, T, T) {} trait NotU8 {} trait Bar {} impl<T, U: NotU8> Bar for (T, T, U) {} trait NeedsFixpoint {} impl<T: Foo + Bar> NeedsFixpoint for T {} impl NeedsFixpoint for (u8, u8, u8) {} trait Overlap {} impl<T: NeedsFixpoint> Overlap for T {} impl<T, U: NotU8, V> Overlap for (T, U, V) {} //[current]~^ ERROR conflicting implementations of trait `Foo` ``` ### Breakage due to removal of incomplete candidate preference Fixes rust-lang#107887. In the old solver we incompletely prefer the builtin trait object impl over user defined impls. This can break inference guidance, inferring `?x` in `dyn Trait<u32>: Trait<?x>` to `u32`, even if an explicit impl of `Trait<u64>` also exists. This caused coherence to incorrectly allow overlapping impls, resulting in ICEs and a theoretical unsoundness. See rust-lang#107887 (comment). This compiles on stable but results in an overlap error with `-Znext-solver=coherence`: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence struct W<T: ?Sized>(*const T); trait Trait<T: ?Sized> { type Assoc; } // This would trigger the check for overlap between automatic and custom impl. // They actually don't overlap so an impl like this should remain possible // forever. // // impl Trait<u64> for dyn Trait<u32> {} trait Indirect {} impl Indirect for dyn Trait<u32, Assoc = ()> {} impl<T: Indirect + ?Sized> Trait<u64> for T { type Assoc = (); } // Incomplete impl where `dyn Trait<u32>: Trait<_>` does not hold, but // `dyn Trait<u32>: Trait<u64>` does. trait EvaluateHack<U: ?Sized> {} impl<T: ?Sized, U: ?Sized> EvaluateHack<W<U>> for T where T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` U: IsU64, T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` { } trait IsU64 {} impl IsU64 for u64 {} trait Overlap<U: ?Sized> { type Assoc: Default; } impl<T: ?Sized + EvaluateHack<W<U>>, U: ?Sized> Overlap<U> for T { type Assoc = Box<u32>; } impl<U: ?Sized> Overlap<U> for dyn Trait<u32, Assoc = ()> { //[next]~^ ERROR conflicting implementations of trait `Overlap<_>` type Assoc = usize; } ``` ### Considering region outlives bounds in the `leak_check` For details on the `leak_check`, see the FCP proposal rust-lang#119820.[^leak_check] [^leak_check]: which should get moved to the dev-guide :3 In both coherence and during candidate selection, the `leak_check` relies on the region constraints added in `evaluate`. It therefore currently does not register outlives obligations: [source](https://github.com/rust-lang/rust/blob/ccb1415eac3289b5ebf64691c0190dc52e0e3d0e/compiler/rustc_trait_selection/src/traits/select/mod.rs#L792-L810). This was likely done as a performance optimization without considering its impact on the `leak_check`. This is the case as in the old solver, *evaluatation* and *fulfillment* are split, with evaluation being responsible for candidate selection and fulfillment actually registering all the constraints. This split does not exist with the new solver. The `leak_check` can therefore eagerly detect errors caused by region outlives obligations. This improves both coherence itself and candidate selection: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait LeakErr<'a, 'b> {} // Using this impl adds an `'b: 'a` bound which results // in a higher-ranked region error. This bound has been // previously ignored but is now considered. impl<'a, 'b: 'a> LeakErr<'a, 'b> for () {} trait NoOverlapDir<'a> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> NoOverlapDir<'a> for T {} impl<'a> NoOverlapDir<'a> for () {} //[current]~^ ERROR conflicting implementations of trait `NoOverlapDir<'_>` // -------------------------------------- // necessary to avoid coherence unknowable candidates struct W<T>(T); trait GuidesSelection<'a, U> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> GuidesSelection<'a, W<u32>> for T {} impl<'a, T> GuidesSelection<'a, W<u8>> for T {} trait NotImplementedByU8 {} trait NoOverlapInd<'a, U> {} impl<'a, T: GuidesSelection<'a, W<U>>, U> NoOverlapInd<'a, U> for T {} impl<'a, U: NotImplementedByU8> NoOverlapInd<'a, U> for () {} //[current]~^ conflicting implementations of trait `NoOverlapInd<'_, _>` ``` ### Removal of `fn match_fresh_trait_refs` The old solver tries to [eagerly detect unbounded recursion](https://github.com/rust-lang/rust/blob/b14fd2359f47fb9a14bbfe55359db4bb3af11861/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1196-L1211), forcing the affected goals to be ambiguous. This check is only an approximation and has not been added to the new solver. The check is not necessary in the new solver and it would be problematic for caching. As it depends on all goals currently on the stack, using a global cache entry would have to always make sure that doing so does not circumvent this check. This changes some goals to error - or succeed - instead of failing with ambiguity. This allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence // Need to use this local wrapper for the impls to be fully // knowable as unknowable candidate result in ambiguity. struct Local<T>(T); trait Trait<U> {} // This impl does not hold, but is ambiguous in the old // solver due to its overflow approximation. impl<U> Trait<U> for Local<u32> where Local<u16>: Trait<U> {} // This impl holds. impl Trait<Local<()>> for Local<u8> {} // In the old solver, `Local<?t>: Trait<Local<?u>>` is ambiguous, // resulting in `Local<?u>: NoImpl`, also being ambiguous. // // In the new solver the first impl does not apply, constraining // `?u` to `Local<()>`, causing `Local<()>: NoImpl` to error. trait Indirect<T> {} impl<T, U> Indirect<U> for T where T: Trait<U>, U: NoImpl {} // Not implemented for `Local<()>` trait NoImpl {} impl NoImpl for Local<u8> {} impl NoImpl for Local<u16> {} // `Local<?t>: Indirect<Local<?u>>` cannot hold, so // these impls do not overlap. trait NoOverlap<U> {} impl<T: Indirect<U>, U> NoOverlap<U> for T {} impl<T, U> NoOverlap<Local<U>> for Local<T> {} //~^ ERROR conflicting implementations of trait `NoOverlap<Local<_>>` ``` ### Non-fatal overflow The old solver immediately emits a fatal error when hitting the recursion limit. The new solver instead returns overflow. This both allows more code to compile and is results in performance and potential future compatability issues. Non-fatal overflow is generally desirable. With fatal overflow, changing the order in which we evaluate nested goals easily causes breakage if we have goal which errors and one which overflows. It is also required to prevent breakage due to the removal of `fn match_fresh_trait_refs`, e.g. [in `typenum`](rust-lang/trait-system-refactor-initiative#73). #### Enabling more code to compile In the below example, the old solver first tried to prove an overflowing goal, resulting in a fatal error. The new solver instead returns ambiguity due to overflow for that goal, causing the implicit negative overlap check to succeed as `Box<u32>: NotImplemented` does not hold. ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence //[current] ERROR overflow evaluating the requirement trait Indirect<T> {} impl<T: Overflow<()>> Indirect<T> for () {} trait Overflow<U> {} impl<T, U> Overflow<U> for Box<T> where U: Indirect<Box<Box<T>>>, {} trait NotImplemented {} trait Trait<U> {} impl<T, U> Trait<U> for T where // T: NotImplemented, // causes old solver to succeed U: Indirect<T>, T: NotImplemented, {} impl Trait<()> for Box<u32> {} ``` #### Avoiding hangs with non-fatal overflow Simply returning ambiguity when reaching the recursion limit can very easily result in hangs, e.g. ```rust trait Recur {} impl<T, U> Recur for ((T, U), (U, T)) where (T, U): Recur, (U, T): Recur, {} trait NotImplemented {} impl<T: NotImplemented> Recur for T {} ``` This can happen quite frequently as it's easy to have exponential blowup due to multiple nested goals at each step. As the trait solver is depth-first, this immediately caused a fatal overflow error in the old solver. In the new solver we have to handle the whole proof tree instead, which can very easily hang. To avoid this we restrict the recursion depth after hitting the recursion limit for the first time. We also **ignore all inference constraints from goals resulting in overflow**. This is mostly backwards compatible as any overflow in the old solver resulted in a fatal error. ### sidenote about normalization We return ambiguous nested goals of `NormalizesTo` goals to the caller and ignore their impact when computing the `Certainty` of the current goal. See the [normalization chapter](https://rustc-dev-guide.rust-lang.org/solve/normalization.html) for more details.This means we apply constraints resulting from other nested goals and from equating the impl header when normalizing, even if a nested goal results in overflow. This is necessary to avoid breaking the following example: ```rust trait Trait { type Assoc; } struct W<T: ?Sized>(*mut T); impl<T: ?Sized> Trait for W<W<T>> where W<T>: Trait, { type Assoc = (); } // `W<?t>: Trait<Assoc = u32>` does not hold as // `Assoc` gets normalized to `()`. However, proving // the where-bounds of the impl results in overflow. // // For this to continue to compile we must not discard // constraints from normalizing associated types. trait NoOverlap {} impl<T: Trait<Assoc = u32>> NoOverlap for T {} impl<T: ?Sized> NoOverlap for W<T> {} ``` #### Future compatability concerns Non-fatal overflow results in some unfortunate future compatability concerns. Changing the approach to avoid more hangs by more strongly penalizing overflow can cause breakage as we either drop constraints or ignore candidates necessary to successfully compile. Weakening the overflow penalities instead allows more code to compile and strengthens inference while potentially causing more code to hang. While the current approach is not perfect, we believe it to be good enough. We believe it to apply the necessary inference constraints to avoid breakage and expect there to not be any desirable patterns broken by our current penalities. Similarly we believe the current constraints to avoid most accidental hangs. Ignoring constraints of overflowing goals is especially useful, as it may allow major future optimizations to our overflow handling. See [this summary](https://hackmd.io/ATf4hN0NRY-w2LIVgeFsVg) and the linked documents in case you want to know more. ### changes to performance In general, trait solving during coherence checking is not significant for performance. Enabling the next-generation trait solver in coherence does not impact our compile time benchmarks. We are still unable to compile the benchmark suite when fully enabling the new trait solver. There are rare cases where the new solver has significantly worse performance due to non-fatal overflow, its reliance on fixpoint algorithms and the removal of the `fn match_fresh_trait_refs` approximation. We encountered such issues in [`typenum`](https://crates.io/crates/typenum) and believe it should be [pretty much as bad as it can get](rust-lang/trait-system-refactor-initiative#73). Due to an improved structure and far better caching, we believe that there is a lot of room for improvement and that the new solver will outperform the existing implementation in nearly all cases, sometimes significantly. We have not yet spent any time micro-optimizing the implementation and have many unimplemented major improvements, such as fast-paths for trivial goals. TODO: get some rough results here and put them in a table ### Unstable features #### Unsupported unstable features The new solver currently does not support all unstable features, most notably `#![feature(generic_const_exprs)]`, `#![feature(associated_const_equality)]` and `#![feature(adt_const_params)]` are not yet fully supported in the new solver. We are confident that supporting them is possible, but did not consider this to be a priority. This stabilization introduces new ICE when using these features in impl headers. #### fixes to `#![feature(specialization)]` - fixes rust-lang#105782 - fixes rust-lang#118987 #### fixes to `#![feature(type_alias_impl_trait)]` - fixes rust-lang#119272 - rust-lang#105787 (comment) - fixes rust-lang#124207 ### Important changes since the original FCP rust-lang#127574 changes the coherence unknowable candidate to only apply if all the super trait bounds may hold. This allows more code to compile and fixes a regression in `pyella` rust-lang#130617 bails with ambiguity if the query response would contain too many non-region inference variables. This should only be triggered in case the result contains a lot of ambiguous aliases in which case further constraining the goal should resolve this. This PR prevents the hang in `nalgebra`. ## This does not stabilize the whole solver While this stabilizes the use of the new solver in coherence checking, there are many parts of the solver which will remain fully unstable. We may still adapt these areas while working towards stabilizing the new solver everywhere. We are confident that we are able to do so without negatively impacting coherence. ### goals with a non-empty `ParamEnv` Coherence always uses an empty environment. We therefore do not depend on the behavior of `AliasBound` and `ParamEnv` candidates. We only stabilizes the behavior of user-defined and builtin implementations of traits. There are still many open questions there. ### opaque types in the defining scope The handling of opaque types - `impl Trait` - in both the new and old solver is still not fully figured out. Luckily this can be ignored for now. While opaque types are reachable during coherence checking by using `impl_trait_in_associated_types`, the behavior during coherence is separate and self-contained. The old and new solver fully agree here. ### normalization is hard This stabilizes that we equate associated types involving bound variables using deferred-alias-equality. We also stop eagerly normalizing in coherence, which should not have any user-facing impact. We do not stabilize the normalization behavior outside of coherence, e.g. we currently deeply normalize all types during writeback with the new solver. This may change going forward ### how to replace `select` from the old solver We sometimes depend on getting a single `impl` for a given trait bound, e.g. when resolving a concrete method for codegen/CTFE. We do not depend on this during coherence, so the exact approach here can still be freely changed going forward. ## Acknowledgements This work would not have been possible without `@compiler-errors.` He implemented large chunks of the solver himself but also and did a lot of testing and experimentation, eagerly discovering multiple issues which had a significant impact on our approach. `@BoxyUwU` has also done some amazing work on the solver. Thank you for the endless hours of discussion resulting in the current approach. Especially the way aliases are handled has gone through multiple revisions to get to its current state. There were also many contributions from - and discussions with - other members of the community and the rest of `@rust-lang/types.` This solver builds upon previous improvements to the compiler, as well as lessons learned from `chalk` and `a-mir-formality`. Getting to this point would not have been possible without that and I am incredibly thankful to everyone involved. See the [list of relevant PRs](https://github.com/rust-lang/rust/pulls?q=is%3Apr+is%3Amerged+label%3AWG-trait-system-refactor+-label%3Arollup+closed%3A%3C2024-03-22+).
lnicola
pushed a commit
to lnicola/rust-analyzer
that referenced
this pull request
Sep 25, 2024
stabilize `-Znext-solver=coherence` r? `@compiler-errors` --- This PR stabilizes the use of the next generation trait solver in coherence checking by enabling `-Znext-solver=coherence` by default. More specifically its use in the *implicit negative overlap check*. The tracking issue for this is rust-lang/rust#114862. Closes #114862. ## Background ### The next generation trait solver The new solver lives in [`rustc_trait_selection::solve`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_trait_selection/src/solve/mod.rs) and is intended to replace the existing *evaluate*, *fulfill*, and *project* implementation. It also has a wider impact on the rest of the type system, for example by changing our approach to handling associated types. For a more detailed explanation of the new trait solver, see the [rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/solve/trait-solving.html). This does not stabilize the current behavior of the new trait solver, only the behavior impacting the implicit negative overlap check. There are many areas in the new solver which are not yet finalized. We are confident that their final design will not conflict with the user-facing behavior observable via coherence. More on that further down. Please check out [the chapter](https://rustc-dev-guide.rust-lang.org/solve/significant-changes.html) summarizing the most significant changes between the existing and new implementations. ### Coherence and the implicit negative overlap check Coherence checking detects any overlapping impls. Overlapping trait impls always error while overlapping inherent impls result in an error if they have methods with the same name. Coherence also results in an error if any other impls could exist, even if they are currently unknown. This affects impls which may get added to upstream crates in a backwards compatible way and impls from downstream crates. Coherence failing to detect overlap is generally considered to be unsound, even if it is difficult to actually get runtime UB this way. It is quite easy to get ICEs due to bugs in coherence. It currently consists of two checks: The [orphan check] validates that impls do not overlap with other impls we do not know about: either because they may be defined in a sibling crate, or because an upstream crate is allowed to add it without being considered a breaking change. The [overlap check] validates that impls do not overlap with other impls we know about. This is done as follows: - Instantiate the generic parameters of both impls with inference variables - Equate the `TraitRef`s of both impls. If it fails there is no overlap. - [implicit negative]: Check whether any of the instantiated `where`-bounds of one of the impls definitely do not hold when using the constraints from the previous step. If a `where`-bound does not hold, there is no overlap. - *explicit negative (still unstable, ignored going forward)*: Check whether the any negated `where`-bounds can be proven, e.g. a `&mut u32: Clone` bound definitely does not hold as an explicit `impl<T> !Clone for &mut T` exists. The overlap check has to *prove that unifying the impls does not succeed*. This means that **incorrectly getting a type error during coherence is unsound** as it would allow impls to overlap: coherence has to be *complete*. Completeness means that we never incorrectly error. This means that during coherence we must only add inference constraints if they are definitely necessary. During ordinary type checking [this does not hold](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01d93b592bd9036ac96071cbf1d624a9), so the trait solver has to behave differently, depending on whether we're in coherence or not. The implicit negative check only considers goals to "definitely not hold" if they could not be implemented downstream, by a sibling, or upstream in a backwards compatible way. If the goal is is "unknowable" as it may get added in another crate, we add an ambiguous candidate: [source](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L858-L883). [orphan check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L566-L579 [overlap check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L92-L98 [implicit negative]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L223-L281 ## Motivation Replacing the existing solver in coherence fixes soundness bugs by removing sources of incompleteness in the type system. The new solver separately strengthens coherence, resulting in more impls being disjoint and passing the coherence check. The concrete changes will be elaborated further down. We believe the stabilization to reduce the likelihood of future bugs in coherence as the new implementation is easier to understand and reason about. It allows us to remove the support for coherence and implicit-negative reasoning in the old solver, allowing us to remove some code and simplifying the old trait solver. We will only remove the old solver support once this stabilization has reached stable to make sure we're able to quickly revert in case any unexpected issues are detected before then. Stabilizing the use of the next-generation trait solver expresses our confidence that its current behavior is intended and our work towards enabling its use everywhere will not require any breaking changes to the areas used by coherence checking. We are also confident that we will be able to replace the existing solver everywhere, as maintaining two separate systems adds a significant maintainance burden. ## User-facing impact and reasoning ### Breakage due to improved handling of associated types The new solver fixes multiple issues related to associated types. As these issues caused coherence to consider more types distinct, fixing them results in more overlap errors. This is therefore a breaking change. #### Structurally relating aliases containing bound vars Fixes rust-lang/rust#102048. In the existing solver relating ambiguous projections containing bound variables is structural. This is *incomplete* and allows overlapping impls. These was mostly not exploitable as the same issue also caused impls to not apply when trying to use them. The new solver defers alias-relating to a nested goal, fixing this issue: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Trait {} trait Project { type Assoc<'a>; } impl Project for u32 { type Assoc<'a> = &'a u32; } // Eagerly normalizing `<?infer as Project>::Assoc<'a>` is ambiguous, // so the old solver ended up structurally relating // // (?infer, for<'a> fn(<?infer as Project>::Assoc<'a>)) // // with // // ((u32, fn(&'a u32))) // // Equating `&'a u32` with `<u32 as Project>::Assoc<'a>` failed, even // though these types are equal modulo normalization. impl<T: Project> Trait for (T, for<'a> fn(<T as Project>::Assoc<'a>)) {} impl<'a> Trait for (u32, fn(&'a u32)) {} //[next]~^ ERROR conflicting implementations of trait `Trait` for type `(u32, for<'a> fn(&'a u32))` ``` A crater run did not discover any breakage due to this change. #### Unknowable candidates for higher ranked trait goals This avoids an unsoundness by attempting to normalize in `trait_ref_is_knowable`, fixing rust-lang/rust#114061. This is a side-effect of supporting lazy normalization, as that forces us to attempt to normalize when checking whether a `TraitRef` is knowable: [source](https://github.com/rust-lang/rust/blob/47dd709bedda8127e8daec33327e0a9d0cdae845/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L754-L764). ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait IsUnit {} impl IsUnit for () {} pub trait WithAssoc<'a> { type Assoc; } // We considered `for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit` // to be knowable, even though the projection is ambiguous. pub trait Trait {} impl<T> Trait for T where T: 'static, for<'a> T: WithAssoc<'a>, for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit, { } impl<T> Trait for Box<T> {} //[next]~^ ERROR conflicting implementations of trait `Trait` ``` The two impls of `Trait` overlap given the following downstream crate: ```rust use dep::*; struct Local; impl WithAssoc<'_> for Box<Local> { type Assoc = (); } ``` There a similar coherence unsoundness caused by our handling of aliases which is fixed separately in rust-lang/rust#117164. This change breaks the [`derive-visitor`](https://crates.io/crates/derive-visitor) crate. I have opened an issue in that repo: nikis05/derive-visitor#16. ### Evaluating goals to a fixpoint and applying inference constraints In the old implementation of the implicit-negative check, each obligation is [checked separately without applying its inference constraints](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L323-L338). The new solver instead [uses a `FulfillmentCtxt`](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L315-L321) for this, which evaluates all obligations in a loop until there's no further inference progress. This is necessary for backwards compatibility as we do not eagerly normalize with the new solver, resulting in constraints from normalization to only get applied by evaluating a separate obligation. This also allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Mirror { type Assoc; } impl<T> Mirror for T { type Assoc = T; } trait Foo {} trait Bar {} // The self type starts out as `?0` but is constrained to `()` // due to the where-clause below. Because `(): Bar` is known to // not hold, we can prove the impls disjoint. impl<T> Foo for T where (): Mirror<Assoc = T> {} //[current]~^ ERROR conflicting implementations of trait `Foo` for type `()` impl<T> Foo for T where T: Bar {} fn main() {} ``` The old solver does not run nested goals to a fixpoint in evaluation. The new solver does do so, strengthening inference and improving the overlap check: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Foo {} impl<T> Foo for (u8, T, T) {} trait NotU8 {} trait Bar {} impl<T, U: NotU8> Bar for (T, T, U) {} trait NeedsFixpoint {} impl<T: Foo + Bar> NeedsFixpoint for T {} impl NeedsFixpoint for (u8, u8, u8) {} trait Overlap {} impl<T: NeedsFixpoint> Overlap for T {} impl<T, U: NotU8, V> Overlap for (T, U, V) {} //[current]~^ ERROR conflicting implementations of trait `Foo` ``` ### Breakage due to removal of incomplete candidate preference Fixes #107887. In the old solver we incompletely prefer the builtin trait object impl over user defined impls. This can break inference guidance, inferring `?x` in `dyn Trait<u32>: Trait<?x>` to `u32`, even if an explicit impl of `Trait<u64>` also exists. This caused coherence to incorrectly allow overlapping impls, resulting in ICEs and a theoretical unsoundness. See rust-lang/rust#107887 (comment). This compiles on stable but results in an overlap error with `-Znext-solver=coherence`: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence struct W<T: ?Sized>(*const T); trait Trait<T: ?Sized> { type Assoc; } // This would trigger the check for overlap between automatic and custom impl. // They actually don't overlap so an impl like this should remain possible // forever. // // impl Trait<u64> for dyn Trait<u32> {} trait Indirect {} impl Indirect for dyn Trait<u32, Assoc = ()> {} impl<T: Indirect + ?Sized> Trait<u64> for T { type Assoc = (); } // Incomplete impl where `dyn Trait<u32>: Trait<_>` does not hold, but // `dyn Trait<u32>: Trait<u64>` does. trait EvaluateHack<U: ?Sized> {} impl<T: ?Sized, U: ?Sized> EvaluateHack<W<U>> for T where T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` U: IsU64, T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` { } trait IsU64 {} impl IsU64 for u64 {} trait Overlap<U: ?Sized> { type Assoc: Default; } impl<T: ?Sized + EvaluateHack<W<U>>, U: ?Sized> Overlap<U> for T { type Assoc = Box<u32>; } impl<U: ?Sized> Overlap<U> for dyn Trait<u32, Assoc = ()> { //[next]~^ ERROR conflicting implementations of trait `Overlap<_>` type Assoc = usize; } ``` ### Considering region outlives bounds in the `leak_check` For details on the `leak_check`, see the FCP proposal in #119820.[^leak_check] [^leak_check]: which should get moved to the dev-guide once that PR lands :3 In both coherence and during candidate selection, the `leak_check` relies on the region constraints added in `evaluate`. It therefore currently does not register outlives obligations: [source](https://github.com/rust-lang/rust/blob/ccb1415eac3289b5ebf64691c0190dc52e0e3d0e/compiler/rustc_trait_selection/src/traits/select/mod.rs#L792-L810). This was likely done as a performance optimization without considering its impact on the `leak_check`. This is the case as in the old solver, *evaluatation* and *fulfillment* are split, with evaluation being responsible for candidate selection and fulfillment actually registering all the constraints. This split does not exist with the new solver. The `leak_check` can therefore eagerly detect errors caused by region outlives obligations. This improves both coherence itself and candidate selection: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait LeakErr<'a, 'b> {} // Using this impl adds an `'b: 'a` bound which results // in a higher-ranked region error. This bound has been // previously ignored but is now considered. impl<'a, 'b: 'a> LeakErr<'a, 'b> for () {} trait NoOverlapDir<'a> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> NoOverlapDir<'a> for T {} impl<'a> NoOverlapDir<'a> for () {} //[current]~^ ERROR conflicting implementations of trait `NoOverlapDir<'_>` // -------------------------------------- // necessary to avoid coherence unknowable candidates struct W<T>(T); trait GuidesSelection<'a, U> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> GuidesSelection<'a, W<u32>> for T {} impl<'a, T> GuidesSelection<'a, W<u8>> for T {} trait NotImplementedByU8 {} trait NoOverlapInd<'a, U> {} impl<'a, T: GuidesSelection<'a, W<U>>, U> NoOverlapInd<'a, U> for T {} impl<'a, U: NotImplementedByU8> NoOverlapInd<'a, U> for () {} //[current]~^ conflicting implementations of trait `NoOverlapInd<'_, _>` ``` ### Removal of `fn match_fresh_trait_refs` The old solver tries to [eagerly detect unbounded recursion](https://github.com/rust-lang/rust/blob/b14fd2359f47fb9a14bbfe55359db4bb3af11861/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1196-L1211), forcing the affected goals to be ambiguous. This check is only an approximation and has not been added to the new solver. The check is not necessary in the new solver and it would be problematic for caching. As it depends on all goals currently on the stack, using a global cache entry would have to always make sure that doing so does not circumvent this check. This changes some goals to error - or succeed - instead of failing with ambiguity. This allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence // Need to use this local wrapper for the impls to be fully // knowable as unknowable candidate result in ambiguity. struct Local<T>(T); trait Trait<U> {} // This impl does not hold, but is ambiguous in the old // solver due to its overflow approximation. impl<U> Trait<U> for Local<u32> where Local<u16>: Trait<U> {} // This impl holds. impl Trait<Local<()>> for Local<u8> {} // In the old solver, `Local<?t>: Trait<Local<?u>>` is ambiguous, // resulting in `Local<?u>: NoImpl`, also being ambiguous. // // In the new solver the first impl does not apply, constraining // `?u` to `Local<()>`, causing `Local<()>: NoImpl` to error. trait Indirect<T> {} impl<T, U> Indirect<U> for T where T: Trait<U>, U: NoImpl {} // Not implemented for `Local<()>` trait NoImpl {} impl NoImpl for Local<u8> {} impl NoImpl for Local<u16> {} // `Local<?t>: Indirect<Local<?u>>` cannot hold, so // these impls do not overlap. trait NoOverlap<U> {} impl<T: Indirect<U>, U> NoOverlap<U> for T {} impl<T, U> NoOverlap<Local<U>> for Local<T> {} //~^ ERROR conflicting implementations of trait `NoOverlap<Local<_>>` ``` ### Non-fatal overflow The old solver immediately emits a fatal error when hitting the recursion limit. The new solver instead returns overflow. This both allows more code to compile and is results in performance and potential future compatability issues. Non-fatal overflow is generally desirable. With fatal overflow, changing the order in which we evaluate nested goals easily causes breakage if we have goal which errors and one which overflows. It is also required to prevent breakage due to the removal of `fn match_fresh_trait_refs`, e.g. [in `typenum`](rust-lang/trait-system-refactor-initiative#73). #### Enabling more code to compile In the below example, the old solver first tried to prove an overflowing goal, resulting in a fatal error. The new solver instead returns ambiguity due to overflow for that goal, causing the implicit negative overlap check to succeed as `Box<u32>: NotImplemented` does not hold. ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence //[current] ERROR overflow evaluating the requirement trait Indirect<T> {} impl<T: Overflow<()>> Indirect<T> for () {} trait Overflow<U> {} impl<T, U> Overflow<U> for Box<T> where U: Indirect<Box<Box<T>>>, {} trait NotImplemented {} trait Trait<U> {} impl<T, U> Trait<U> for T where // T: NotImplemented, // causes old solver to succeed U: Indirect<T>, T: NotImplemented, {} impl Trait<()> for Box<u32> {} ``` #### Avoiding hangs with non-fatal overflow Simply returning ambiguity when reaching the recursion limit can very easily result in hangs, e.g. ```rust trait Recur {} impl<T, U> Recur for ((T, U), (U, T)) where (T, U): Recur, (U, T): Recur, {} trait NotImplemented {} impl<T: NotImplemented> Recur for T {} ``` This can happen quite frequently as it's easy to have exponential blowup due to multiple nested goals at each step. As the trait solver is depth-first, this immediately caused a fatal overflow error in the old solver. In the new solver we have to handle the whole proof tree instead, which can very easily hang. To avoid this we restrict the recursion depth after hitting the recursion limit for the first time. We also **ignore all inference constraints from goals resulting in overflow**. This is mostly backwards compatible as any overflow in the old solver resulted in a fatal error. ### sidenote about normalization We return ambiguous nested goals of `NormalizesTo` goals to the caller and ignore their impact when computing the `Certainty` of the current goal. See the [normalization chapter](https://rustc-dev-guide.rust-lang.org/solve/normalization.html) for more details.This means we apply constraints resulting from other nested goals and from equating the impl header when normalizing, even if a nested goal results in overflow. This is necessary to avoid breaking the following example: ```rust trait Trait { type Assoc; } struct W<T: ?Sized>(*mut T); impl<T: ?Sized> Trait for W<W<T>> where W<T>: Trait, { type Assoc = (); } // `W<?t>: Trait<Assoc = u32>` does not hold as // `Assoc` gets normalized to `()`. However, proving // the where-bounds of the impl results in overflow. // // For this to continue to compile we must not discard // constraints from normalizing associated types. trait NoOverlap {} impl<T: Trait<Assoc = u32>> NoOverlap for T {} impl<T: ?Sized> NoOverlap for W<T> {} ``` #### Future compatability concerns Non-fatal overflow results in some unfortunate future compatability concerns. Changing the approach to avoid more hangs by more strongly penalizing overflow can cause breakage as we either drop constraints or ignore candidates necessary to successfully compile. Weakening the overflow penalities instead allows more code to compile and strengthens inference while potentially causing more code to hang. While the current approach is not perfect, we believe it to be good enough. We believe it to apply the necessary inference constraints to avoid breakage and expect there to not be any desirable patterns broken by our current penalities. Similarly we believe the current constraints to avoid most accidental hangs. Ignoring constraints of overflowing goals is especially useful, as it may allow major future optimizations to our overflow handling. See [this summary](https://hackmd.io/ATf4hN0NRY-w2LIVgeFsVg) and the linked documents in case you want to know more. ### changes to performance In general, trait solving during coherence checking is not significant for performance. Enabling the next-generation trait solver in coherence does not impact our compile time benchmarks. We are still unable to compile the benchmark suite when fully enabling the new trait solver. There are rare cases where the new solver has significantly worse performance due to non-fatal overflow, its reliance on fixpoint algorithms and the removal of the `fn match_fresh_trait_refs` approximation. We encountered such issues in [`typenum`](https://crates.io/crates/typenum) and believe it should be [pretty much as bad as it can get](rust-lang/trait-system-refactor-initiative#73). Due to an improved structure and far better caching, we believe that there is a lot of room for improvement and that the new solver will outperform the existing implementation in nearly all cases, sometimes significantly. We have not yet spent any time micro-optimizing the implementation and have many unimplemented major improvements, such as fast-paths for trivial goals. TODO: get some rough results here and put them in a table ### Unstable features #### Unsupported unstable features The new solver currently does not support all unstable features, most notably `#![feature(generic_const_exprs)]`, `#![feature(associated_const_equality)]` and `#![feature(adt_const_params)]` are not yet fully supported in the new solver. We are confident that supporting them is possible, but did not consider this to be a priority. This stabilization introduces new ICE when using these features in impl headers. #### fixes to `#![feature(specialization)]` - fixes #105782 - fixes #118987 #### fixes to `#![feature(type_alias_impl_trait)]` - fixes #119272 - rust-lang/rust#105787 (comment) - fixes #124207 ## This does not stabilize the whole solver While this stabilizes the use of the new solver in coherence checking, there are many parts of the solver which will remain fully unstable. We may still adapt these areas while working towards stabilizing the new solver everywhere. We are confident that we are able to do so without negatively impacting coherence. ### goals with a non-empty `ParamEnv` Coherence always uses an empty environment. We therefore do not depend on the behavior of `AliasBound` and `ParamEnv` candidates. We only stabilizes the behavior of user-defined and builtin implementations of traits. There are still many open questions there. ### opaque types in the defining scope The handling of opaque types - `impl Trait` - in both the new and old solver is still not fully figured out. Luckily this can be ignored for now. While opaque types are reachable during coherence checking by using `impl_trait_in_associated_types`, the behavior during coherence is separate and self-contained. The old and new solver fully agree here. ### normalization is hard This stabilizes that we equate associated types involving bound variables using deferred-alias-equality. We also stop eagerly normalizing in coherence, which should not have any user-facing impact. We do not stabilize the normalization behavior outside of coherence, e.g. we currently deeply normalize all types during writeback with the new solver. This may change going forward ### how to replace `select` from the old solver We sometimes depend on getting a single `impl` for a given trait bound, e.g. when resolving a concrete method for codegen/CTFE. We do not depend on this during coherence, so the exact approach here can still be freely changed going forward. ## Acknowledgements This work would not have been possible without `@compiler-errors.` He implemented large chunks of the solver himself but also and did a lot of testing and experimentation, eagerly discovering multiple issues which had a significant impact on our approach. `@BoxyUwU` has also done some amazing work on the solver. Thank you for the endless hours of discussion resulting in the current approach. Especially the way aliases are handled has gone through multiple revisions to get to its current state. There were also many contributions from - and discussions with - other members of the community and the rest of `@rust-lang/types.` This solver builds upon previous improvements to the compiler, as well as lessons learned from `chalk` and `a-mir-formality`. Getting to this point would not have been possible without that and I am incredibly thankful to everyone involved. See the [list of relevant PRs](https://github.com/rust-lang/rust/pulls?q=is%3Apr+is%3Amerged+label%3AWG-trait-system-refactor+-label%3Arollup+closed%3A%3C2024-03-22+).
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 3, 2024
stabilize `-Znext-solver=coherence` again r? `@compiler-errors` --- This PR stabilizes the use of the next generation trait solver in coherence checking by enabling `-Znext-solver=coherence` by default. More specifically its use in the *implicit negative overlap check*. The tracking issue for this is rust-lang#114862. Closes rust-lang#114862. This is a direct copy of rust-lang#121848 which has been reverted due to a hang in `nalgebra`: rust-lang#130056. This hang should have been fixed by rust-lang#130617 and rust-lang#130821. See the added section in the stabilization report containing user facing changes merged since the original FCP. ## Background ### The next generation trait solver The new solver lives in [`rustc_trait_selection::solve`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_trait_selection/src/solve/mod.rs) and is intended to replace the existing *evaluate*, *fulfill*, and *project* implementation. It also has a wider impact on the rest of the type system, for example by changing our approach to handling associated types. For a more detailed explanation of the new trait solver, see the [rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/solve/trait-solving.html). This does not stabilize the current behavior of the new trait solver, only the behavior impacting the implicit negative overlap check. There are many areas in the new solver which are not yet finalized. We are confident that their final design will not conflict with the user-facing behavior observable via coherence. More on that further down. Please check out [the chapter](https://rustc-dev-guide.rust-lang.org/solve/significant-changes.html) summarizing the most significant changes between the existing and new implementations. ### Coherence and the implicit negative overlap check Coherence checking detects any overlapping impls. Overlapping trait impls always error while overlapping inherent impls result in an error if they have methods with the same name. Coherence also results in an error if any other impls could exist, even if they are currently unknown. This affects impls which may get added to upstream crates in a backwards compatible way and impls from downstream crates. Coherence failing to detect overlap is generally considered to be unsound, even if it is difficult to actually get runtime UB this way. It is quite easy to get ICEs due to bugs in coherence. It currently consists of two checks: The [orphan check] validates that impls do not overlap with other impls we do not know about: either because they may be defined in a sibling crate, or because an upstream crate is allowed to add it without being considered a breaking change. The [overlap check] validates that impls do not overlap with other impls we know about. This is done as follows: - Instantiate the generic parameters of both impls with inference variables - Equate the `TraitRef`s of both impls. If it fails there is no overlap. - [implicit negative]: Check whether any of the instantiated `where`-bounds of one of the impls definitely do not hold when using the constraints from the previous step. If a `where`-bound does not hold, there is no overlap. - *explicit negative (still unstable, ignored going forward)*: Check whether the any negated `where`-bounds can be proven, e.g. a `&mut u32: Clone` bound definitely does not hold as an explicit `impl<T> !Clone for &mut T` exists. The overlap check has to *prove that unifying the impls does not succeed*. This means that **incorrectly getting a type error during coherence is unsound** as it would allow impls to overlap: coherence has to be *complete*. Completeness means that we never incorrectly error. This means that during coherence we must only add inference constraints if they are definitely necessary. During ordinary type checking [this does not hold](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01d93b592bd9036ac96071cbf1d624a9), so the trait solver has to behave differently, depending on whether we're in coherence or not. The implicit negative check only considers goals to "definitely not hold" if they could not be implemented downstream, by a sibling, or upstream in a backwards compatible way. If the goal is is "unknowable" as it may get added in another crate, we add an ambiguous candidate: [source](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L858-L883). [orphan check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L566-L579 [overlap check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L92-L98 [implicit negative]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L223-L281 ## Motivation Replacing the existing solver in coherence fixes soundness bugs by removing sources of incompleteness in the type system. The new solver separately strengthens coherence, resulting in more impls being disjoint and passing the coherence check. The concrete changes will be elaborated further down. We believe the stabilization to reduce the likelihood of future bugs in coherence as the new implementation is easier to understand and reason about. It allows us to remove the support for coherence and implicit-negative reasoning in the old solver, allowing us to remove some code and simplifying the old trait solver. We will only remove the old solver support once this stabilization has reached stable to make sure we're able to quickly revert in case any unexpected issues are detected before then. Stabilizing the use of the next-generation trait solver expresses our confidence that its current behavior is intended and our work towards enabling its use everywhere will not require any breaking changes to the areas used by coherence checking. We are also confident that we will be able to replace the existing solver everywhere, as maintaining two separate systems adds a significant maintainance burden. ## User-facing impact and reasoning ### Breakage due to improved handling of associated types The new solver fixes multiple issues related to associated types. As these issues caused coherence to consider more types distinct, fixing them results in more overlap errors. This is therefore a breaking change. #### Structurally relating aliases containing bound vars Fixes rust-lang#102048. In the existing solver relating ambiguous projections containing bound variables is structural. This is *incomplete* and allows overlapping impls. These was mostly not exploitable as the same issue also caused impls to not apply when trying to use them. The new solver defers alias-relating to a nested goal, fixing this issue: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Trait {} trait Project { type Assoc<'a>; } impl Project for u32 { type Assoc<'a> = &'a u32; } // Eagerly normalizing `<?infer as Project>::Assoc<'a>` is ambiguous, // so the old solver ended up structurally relating // // (?infer, for<'a> fn(<?infer as Project>::Assoc<'a>)) // // with // // ((u32, fn(&'a u32))) // // Equating `&'a u32` with `<u32 as Project>::Assoc<'a>` failed, even // though these types are equal modulo normalization. impl<T: Project> Trait for (T, for<'a> fn(<T as Project>::Assoc<'a>)) {} impl<'a> Trait for (u32, fn(&'a u32)) {} //[next]~^ ERROR conflicting implementations of trait `Trait` for type `(u32, for<'a> fn(&'a u32))` ``` A crater run did not discover any breakage due to this change. #### Unknowable candidates for higher ranked trait goals This avoids an unsoundness by attempting to normalize in `trait_ref_is_knowable`, fixing rust-lang#114061. This is a side-effect of supporting lazy normalization, as that forces us to attempt to normalize when checking whether a `TraitRef` is knowable: [source](https://github.com/rust-lang/rust/blob/47dd709bedda8127e8daec33327e0a9d0cdae845/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L754-L764). ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait IsUnit {} impl IsUnit for () {} pub trait WithAssoc<'a> { type Assoc; } // We considered `for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit` // to be knowable, even though the projection is ambiguous. pub trait Trait {} impl<T> Trait for T where T: 'static, for<'a> T: WithAssoc<'a>, for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit, { } impl<T> Trait for Box<T> {} //[next]~^ ERROR conflicting implementations of trait `Trait` ``` The two impls of `Trait` overlap given the following downstream crate: ```rust use dep::*; struct Local; impl WithAssoc<'_> for Box<Local> { type Assoc = (); } ``` There a similar coherence unsoundness caused by our handling of aliases which is fixed separately in rust-lang#117164. This change breaks the [`derive-visitor`](https://crates.io/crates/derive-visitor) crate. I have opened an issue in that repo: nikis05/derive-visitor#16. ### Evaluating goals to a fixpoint and applying inference constraints In the old implementation of the implicit-negative check, each obligation is [checked separately without applying its inference constraints](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L323-L338). The new solver instead [uses a `FulfillmentCtxt`](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L315-L321) for this, which evaluates all obligations in a loop until there's no further inference progress. This is necessary for backwards compatibility as we do not eagerly normalize with the new solver, resulting in constraints from normalization to only get applied by evaluating a separate obligation. This also allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Mirror { type Assoc; } impl<T> Mirror for T { type Assoc = T; } trait Foo {} trait Bar {} // The self type starts out as `?0` but is constrained to `()` // due to the where-clause below. Because `(): Bar` is known to // not hold, we can prove the impls disjoint. impl<T> Foo for T where (): Mirror<Assoc = T> {} //[current]~^ ERROR conflicting implementations of trait `Foo` for type `()` impl<T> Foo for T where T: Bar {} fn main() {} ``` The old solver does not run nested goals to a fixpoint in evaluation. The new solver does do so, strengthening inference and improving the overlap check: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Foo {} impl<T> Foo for (u8, T, T) {} trait NotU8 {} trait Bar {} impl<T, U: NotU8> Bar for (T, T, U) {} trait NeedsFixpoint {} impl<T: Foo + Bar> NeedsFixpoint for T {} impl NeedsFixpoint for (u8, u8, u8) {} trait Overlap {} impl<T: NeedsFixpoint> Overlap for T {} impl<T, U: NotU8, V> Overlap for (T, U, V) {} //[current]~^ ERROR conflicting implementations of trait `Foo` ``` ### Breakage due to removal of incomplete candidate preference Fixes rust-lang#107887. In the old solver we incompletely prefer the builtin trait object impl over user defined impls. This can break inference guidance, inferring `?x` in `dyn Trait<u32>: Trait<?x>` to `u32`, even if an explicit impl of `Trait<u64>` also exists. This caused coherence to incorrectly allow overlapping impls, resulting in ICEs and a theoretical unsoundness. See rust-lang#107887 (comment). This compiles on stable but results in an overlap error with `-Znext-solver=coherence`: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence struct W<T: ?Sized>(*const T); trait Trait<T: ?Sized> { type Assoc; } // This would trigger the check for overlap between automatic and custom impl. // They actually don't overlap so an impl like this should remain possible // forever. // // impl Trait<u64> for dyn Trait<u32> {} trait Indirect {} impl Indirect for dyn Trait<u32, Assoc = ()> {} impl<T: Indirect + ?Sized> Trait<u64> for T { type Assoc = (); } // Incomplete impl where `dyn Trait<u32>: Trait<_>` does not hold, but // `dyn Trait<u32>: Trait<u64>` does. trait EvaluateHack<U: ?Sized> {} impl<T: ?Sized, U: ?Sized> EvaluateHack<W<U>> for T where T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` U: IsU64, T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` { } trait IsU64 {} impl IsU64 for u64 {} trait Overlap<U: ?Sized> { type Assoc: Default; } impl<T: ?Sized + EvaluateHack<W<U>>, U: ?Sized> Overlap<U> for T { type Assoc = Box<u32>; } impl<U: ?Sized> Overlap<U> for dyn Trait<u32, Assoc = ()> { //[next]~^ ERROR conflicting implementations of trait `Overlap<_>` type Assoc = usize; } ``` ### Considering region outlives bounds in the `leak_check` For details on the `leak_check`, see the FCP proposal rust-lang#119820.[^leak_check] [^leak_check]: which should get moved to the dev-guide :3 In both coherence and during candidate selection, the `leak_check` relies on the region constraints added in `evaluate`. It therefore currently does not register outlives obligations: [source](https://github.com/rust-lang/rust/blob/ccb1415eac3289b5ebf64691c0190dc52e0e3d0e/compiler/rustc_trait_selection/src/traits/select/mod.rs#L792-L810). This was likely done as a performance optimization without considering its impact on the `leak_check`. This is the case as in the old solver, *evaluatation* and *fulfillment* are split, with evaluation being responsible for candidate selection and fulfillment actually registering all the constraints. This split does not exist with the new solver. The `leak_check` can therefore eagerly detect errors caused by region outlives obligations. This improves both coherence itself and candidate selection: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait LeakErr<'a, 'b> {} // Using this impl adds an `'b: 'a` bound which results // in a higher-ranked region error. This bound has been // previously ignored but is now considered. impl<'a, 'b: 'a> LeakErr<'a, 'b> for () {} trait NoOverlapDir<'a> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> NoOverlapDir<'a> for T {} impl<'a> NoOverlapDir<'a> for () {} //[current]~^ ERROR conflicting implementations of trait `NoOverlapDir<'_>` // -------------------------------------- // necessary to avoid coherence unknowable candidates struct W<T>(T); trait GuidesSelection<'a, U> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> GuidesSelection<'a, W<u32>> for T {} impl<'a, T> GuidesSelection<'a, W<u8>> for T {} trait NotImplementedByU8 {} trait NoOverlapInd<'a, U> {} impl<'a, T: GuidesSelection<'a, W<U>>, U> NoOverlapInd<'a, U> for T {} impl<'a, U: NotImplementedByU8> NoOverlapInd<'a, U> for () {} //[current]~^ conflicting implementations of trait `NoOverlapInd<'_, _>` ``` ### Removal of `fn match_fresh_trait_refs` The old solver tries to [eagerly detect unbounded recursion](https://github.com/rust-lang/rust/blob/b14fd2359f47fb9a14bbfe55359db4bb3af11861/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1196-L1211), forcing the affected goals to be ambiguous. This check is only an approximation and has not been added to the new solver. The check is not necessary in the new solver and it would be problematic for caching. As it depends on all goals currently on the stack, using a global cache entry would have to always make sure that doing so does not circumvent this check. This changes some goals to error - or succeed - instead of failing with ambiguity. This allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence // Need to use this local wrapper for the impls to be fully // knowable as unknowable candidate result in ambiguity. struct Local<T>(T); trait Trait<U> {} // This impl does not hold, but is ambiguous in the old // solver due to its overflow approximation. impl<U> Trait<U> for Local<u32> where Local<u16>: Trait<U> {} // This impl holds. impl Trait<Local<()>> for Local<u8> {} // In the old solver, `Local<?t>: Trait<Local<?u>>` is ambiguous, // resulting in `Local<?u>: NoImpl`, also being ambiguous. // // In the new solver the first impl does not apply, constraining // `?u` to `Local<()>`, causing `Local<()>: NoImpl` to error. trait Indirect<T> {} impl<T, U> Indirect<U> for T where T: Trait<U>, U: NoImpl {} // Not implemented for `Local<()>` trait NoImpl {} impl NoImpl for Local<u8> {} impl NoImpl for Local<u16> {} // `Local<?t>: Indirect<Local<?u>>` cannot hold, so // these impls do not overlap. trait NoOverlap<U> {} impl<T: Indirect<U>, U> NoOverlap<U> for T {} impl<T, U> NoOverlap<Local<U>> for Local<T> {} //~^ ERROR conflicting implementations of trait `NoOverlap<Local<_>>` ``` ### Non-fatal overflow The old solver immediately emits a fatal error when hitting the recursion limit. The new solver instead returns overflow. This both allows more code to compile and is results in performance and potential future compatability issues. Non-fatal overflow is generally desirable. With fatal overflow, changing the order in which we evaluate nested goals easily causes breakage if we have goal which errors and one which overflows. It is also required to prevent breakage due to the removal of `fn match_fresh_trait_refs`, e.g. [in `typenum`](rust-lang/trait-system-refactor-initiative#73). #### Enabling more code to compile In the below example, the old solver first tried to prove an overflowing goal, resulting in a fatal error. The new solver instead returns ambiguity due to overflow for that goal, causing the implicit negative overlap check to succeed as `Box<u32>: NotImplemented` does not hold. ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence //[current] ERROR overflow evaluating the requirement trait Indirect<T> {} impl<T: Overflow<()>> Indirect<T> for () {} trait Overflow<U> {} impl<T, U> Overflow<U> for Box<T> where U: Indirect<Box<Box<T>>>, {} trait NotImplemented {} trait Trait<U> {} impl<T, U> Trait<U> for T where // T: NotImplemented, // causes old solver to succeed U: Indirect<T>, T: NotImplemented, {} impl Trait<()> for Box<u32> {} ``` #### Avoiding hangs with non-fatal overflow Simply returning ambiguity when reaching the recursion limit can very easily result in hangs, e.g. ```rust trait Recur {} impl<T, U> Recur for ((T, U), (U, T)) where (T, U): Recur, (U, T): Recur, {} trait NotImplemented {} impl<T: NotImplemented> Recur for T {} ``` This can happen quite frequently as it's easy to have exponential blowup due to multiple nested goals at each step. As the trait solver is depth-first, this immediately caused a fatal overflow error in the old solver. In the new solver we have to handle the whole proof tree instead, which can very easily hang. To avoid this we restrict the recursion depth after hitting the recursion limit for the first time. We also **ignore all inference constraints from goals resulting in overflow**. This is mostly backwards compatible as any overflow in the old solver resulted in a fatal error. ### sidenote about normalization We return ambiguous nested goals of `NormalizesTo` goals to the caller and ignore their impact when computing the `Certainty` of the current goal. See the [normalization chapter](https://rustc-dev-guide.rust-lang.org/solve/normalization.html) for more details.This means we apply constraints resulting from other nested goals and from equating the impl header when normalizing, even if a nested goal results in overflow. This is necessary to avoid breaking the following example: ```rust trait Trait { type Assoc; } struct W<T: ?Sized>(*mut T); impl<T: ?Sized> Trait for W<W<T>> where W<T>: Trait, { type Assoc = (); } // `W<?t>: Trait<Assoc = u32>` does not hold as // `Assoc` gets normalized to `()`. However, proving // the where-bounds of the impl results in overflow. // // For this to continue to compile we must not discard // constraints from normalizing associated types. trait NoOverlap {} impl<T: Trait<Assoc = u32>> NoOverlap for T {} impl<T: ?Sized> NoOverlap for W<T> {} ``` #### Future compatability concerns Non-fatal overflow results in some unfortunate future compatability concerns. Changing the approach to avoid more hangs by more strongly penalizing overflow can cause breakage as we either drop constraints or ignore candidates necessary to successfully compile. Weakening the overflow penalities instead allows more code to compile and strengthens inference while potentially causing more code to hang. While the current approach is not perfect, we believe it to be good enough. We believe it to apply the necessary inference constraints to avoid breakage and expect there to not be any desirable patterns broken by our current penalities. Similarly we believe the current constraints to avoid most accidental hangs. Ignoring constraints of overflowing goals is especially useful, as it may allow major future optimizations to our overflow handling. See [this summary](https://hackmd.io/ATf4hN0NRY-w2LIVgeFsVg) and the linked documents in case you want to know more. ### changes to performance In general, trait solving during coherence checking is not significant for performance. Enabling the next-generation trait solver in coherence does not impact our compile time benchmarks. We are still unable to compile the benchmark suite when fully enabling the new trait solver. There are rare cases where the new solver has significantly worse performance due to non-fatal overflow, its reliance on fixpoint algorithms and the removal of the `fn match_fresh_trait_refs` approximation. We encountered such issues in [`typenum`](https://crates.io/crates/typenum) and believe it should be [pretty much as bad as it can get](rust-lang/trait-system-refactor-initiative#73). Due to an improved structure and far better caching, we believe that there is a lot of room for improvement and that the new solver will outperform the existing implementation in nearly all cases, sometimes significantly. We have not yet spent any time micro-optimizing the implementation and have many unimplemented major improvements, such as fast-paths for trivial goals. ### Unstable features #### Unsupported unstable features The new solver currently does not support all unstable features, most notably `#![feature(generic_const_exprs)]`, `#![feature(associated_const_equality)]` and `#![feature(adt_const_params)]` are not yet fully supported in the new solver. We are confident that supporting them is possible, but did not consider this to be a priority. This stabilization introduces new ICE when using these features in impl headers. #### fixes to `#![feature(specialization)]` - fixes rust-lang#105782 - fixes rust-lang#118987 #### fixes to `#![feature(type_alias_impl_trait)]` - fixes rust-lang#119272 - rust-lang#105787 (comment) - fixes rust-lang#124207 ### Important changes since the original FCP rust-lang#127574 changes the coherence unknowable candidate to only apply if all the super trait bounds may hold. This allows more code to compile and fixes a regression in `pyella` rust-lang#130617 bails with ambiguity if the query response would contain too many non-region inference variables. This should only be triggered in case the result contains a lot of ambiguous aliases in which case further constraining the goal should resolve this. rust-lang#130821 adds caching to a lot of type folders, which is necessary to handle exponentially large types and handles the hang in `nalgebra` together with rust-lang#130617. ## This does not stabilize the whole solver While this stabilizes the use of the new solver in coherence checking, there are many parts of the solver which will remain fully unstable. We may still adapt these areas while working towards stabilizing the new solver everywhere. We are confident that we are able to do so without negatively impacting coherence. ### goals with a non-empty `ParamEnv` Coherence always uses an empty environment. We therefore do not depend on the behavior of `AliasBound` and `ParamEnv` candidates. We only stabilizes the behavior of user-defined and builtin implementations of traits. There are still many open questions there. ### opaque types in the defining scope The handling of opaque types - `impl Trait` - in both the new and old solver is still not fully figured out. Luckily this can be ignored for now. While opaque types are reachable during coherence checking by using `impl_trait_in_associated_types`, the behavior during coherence is separate and self-contained. The old and new solver fully agree here. ### normalization is hard This stabilizes that we equate associated types involving bound variables using deferred-alias-equality. We also stop eagerly normalizing in coherence, which should not have any user-facing impact. We do not stabilize the normalization behavior outside of coherence, e.g. we currently deeply normalize all types during writeback with the new solver. This may change going forward ### how to replace `select` from the old solver We sometimes depend on getting a single `impl` for a given trait bound, e.g. when resolving a concrete method for codegen/CTFE. We do not depend on this during coherence, so the exact approach here can still be freely changed going forward. ## Acknowledgements This work would not have been possible without `@compiler-errors.` He implemented large chunks of the solver himself but also and did a lot of testing and experimentation, eagerly discovering multiple issues which had a significant impact on our approach. `@BoxyUwU` has also done some amazing work on the solver. Thank you for the endless hours of discussion resulting in the current approach. Especially the way aliases are handled has gone through multiple revisions to get to its current state. There were also many contributions from - and discussions with - other members of the community and the rest of `@rust-lang/types.` This solver builds upon previous improvements to the compiler, as well as lessons learned from `chalk` and `a-mir-formality`. Getting to this point would not have been possible without that and I am incredibly thankful to everyone involved. See the [list of relevant PRs](https://github.com/rust-lang/rust/pulls?q=is%3Apr+is%3Amerged+label%3AWG-trait-system-refactor+-label%3Arollup+closed%3A%3C2024-03-22+).
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 9, 2024
stabilize `-Znext-solver=coherence` again r? `@compiler-errors` --- This PR stabilizes the use of the next generation trait solver in coherence checking by enabling `-Znext-solver=coherence` by default. More specifically its use in the *implicit negative overlap check*. The tracking issue for this is rust-lang#114862. Closes rust-lang#114862. This is a direct copy of rust-lang#121848 which has been reverted due to a hang in `nalgebra`: rust-lang#130056. This hang should have been fixed by rust-lang#130617 and rust-lang#130821. See the added section in the stabilization report containing user facing changes merged since the original FCP. ## Background ### The next generation trait solver The new solver lives in [`rustc_trait_selection::solve`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_trait_selection/src/solve/mod.rs) and is intended to replace the existing *evaluate*, *fulfill*, and *project* implementation. It also has a wider impact on the rest of the type system, for example by changing our approach to handling associated types. For a more detailed explanation of the new trait solver, see the [rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/solve/trait-solving.html). This does not stabilize the current behavior of the new trait solver, only the behavior impacting the implicit negative overlap check. There are many areas in the new solver which are not yet finalized. We are confident that their final design will not conflict with the user-facing behavior observable via coherence. More on that further down. Please check out [the chapter](https://rustc-dev-guide.rust-lang.org/solve/significant-changes.html) summarizing the most significant changes between the existing and new implementations. ### Coherence and the implicit negative overlap check Coherence checking detects any overlapping impls. Overlapping trait impls always error while overlapping inherent impls result in an error if they have methods with the same name. Coherence also results in an error if any other impls could exist, even if they are currently unknown. This affects impls which may get added to upstream crates in a backwards compatible way and impls from downstream crates. Coherence failing to detect overlap is generally considered to be unsound, even if it is difficult to actually get runtime UB this way. It is quite easy to get ICEs due to bugs in coherence. It currently consists of two checks: The [orphan check] validates that impls do not overlap with other impls we do not know about: either because they may be defined in a sibling crate, or because an upstream crate is allowed to add it without being considered a breaking change. The [overlap check] validates that impls do not overlap with other impls we know about. This is done as follows: - Instantiate the generic parameters of both impls with inference variables - Equate the `TraitRef`s of both impls. If it fails there is no overlap. - [implicit negative]: Check whether any of the instantiated `where`-bounds of one of the impls definitely do not hold when using the constraints from the previous step. If a `where`-bound does not hold, there is no overlap. - *explicit negative (still unstable, ignored going forward)*: Check whether the any negated `where`-bounds can be proven, e.g. a `&mut u32: Clone` bound definitely does not hold as an explicit `impl<T> !Clone for &mut T` exists. The overlap check has to *prove that unifying the impls does not succeed*. This means that **incorrectly getting a type error during coherence is unsound** as it would allow impls to overlap: coherence has to be *complete*. Completeness means that we never incorrectly error. This means that during coherence we must only add inference constraints if they are definitely necessary. During ordinary type checking [this does not hold](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01d93b592bd9036ac96071cbf1d624a9), so the trait solver has to behave differently, depending on whether we're in coherence or not. The implicit negative check only considers goals to "definitely not hold" if they could not be implemented downstream, by a sibling, or upstream in a backwards compatible way. If the goal is is "unknowable" as it may get added in another crate, we add an ambiguous candidate: [source](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L858-L883). [orphan check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L566-L579 [overlap check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L92-L98 [implicit negative]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L223-L281 ## Motivation Replacing the existing solver in coherence fixes soundness bugs by removing sources of incompleteness in the type system. The new solver separately strengthens coherence, resulting in more impls being disjoint and passing the coherence check. The concrete changes will be elaborated further down. We believe the stabilization to reduce the likelihood of future bugs in coherence as the new implementation is easier to understand and reason about. It allows us to remove the support for coherence and implicit-negative reasoning in the old solver, allowing us to remove some code and simplifying the old trait solver. We will only remove the old solver support once this stabilization has reached stable to make sure we're able to quickly revert in case any unexpected issues are detected before then. Stabilizing the use of the next-generation trait solver expresses our confidence that its current behavior is intended and our work towards enabling its use everywhere will not require any breaking changes to the areas used by coherence checking. We are also confident that we will be able to replace the existing solver everywhere, as maintaining two separate systems adds a significant maintainance burden. ## User-facing impact and reasoning ### Breakage due to improved handling of associated types The new solver fixes multiple issues related to associated types. As these issues caused coherence to consider more types distinct, fixing them results in more overlap errors. This is therefore a breaking change. #### Structurally relating aliases containing bound vars Fixes rust-lang#102048. In the existing solver relating ambiguous projections containing bound variables is structural. This is *incomplete* and allows overlapping impls. These was mostly not exploitable as the same issue also caused impls to not apply when trying to use them. The new solver defers alias-relating to a nested goal, fixing this issue: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Trait {} trait Project { type Assoc<'a>; } impl Project for u32 { type Assoc<'a> = &'a u32; } // Eagerly normalizing `<?infer as Project>::Assoc<'a>` is ambiguous, // so the old solver ended up structurally relating // // (?infer, for<'a> fn(<?infer as Project>::Assoc<'a>)) // // with // // ((u32, fn(&'a u32))) // // Equating `&'a u32` with `<u32 as Project>::Assoc<'a>` failed, even // though these types are equal modulo normalization. impl<T: Project> Trait for (T, for<'a> fn(<T as Project>::Assoc<'a>)) {} impl<'a> Trait for (u32, fn(&'a u32)) {} //[next]~^ ERROR conflicting implementations of trait `Trait` for type `(u32, for<'a> fn(&'a u32))` ``` A crater run did not discover any breakage due to this change. #### Unknowable candidates for higher ranked trait goals This avoids an unsoundness by attempting to normalize in `trait_ref_is_knowable`, fixing rust-lang#114061. This is a side-effect of supporting lazy normalization, as that forces us to attempt to normalize when checking whether a `TraitRef` is knowable: [source](https://github.com/rust-lang/rust/blob/47dd709bedda8127e8daec33327e0a9d0cdae845/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L754-L764). ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait IsUnit {} impl IsUnit for () {} pub trait WithAssoc<'a> { type Assoc; } // We considered `for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit` // to be knowable, even though the projection is ambiguous. pub trait Trait {} impl<T> Trait for T where T: 'static, for<'a> T: WithAssoc<'a>, for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit, { } impl<T> Trait for Box<T> {} //[next]~^ ERROR conflicting implementations of trait `Trait` ``` The two impls of `Trait` overlap given the following downstream crate: ```rust use dep::*; struct Local; impl WithAssoc<'_> for Box<Local> { type Assoc = (); } ``` There a similar coherence unsoundness caused by our handling of aliases which is fixed separately in rust-lang#117164. This change breaks the [`derive-visitor`](https://crates.io/crates/derive-visitor) crate. I have opened an issue in that repo: nikis05/derive-visitor#16. ### Evaluating goals to a fixpoint and applying inference constraints In the old implementation of the implicit-negative check, each obligation is [checked separately without applying its inference constraints](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L323-L338). The new solver instead [uses a `FulfillmentCtxt`](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L315-L321) for this, which evaluates all obligations in a loop until there's no further inference progress. This is necessary for backwards compatibility as we do not eagerly normalize with the new solver, resulting in constraints from normalization to only get applied by evaluating a separate obligation. This also allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Mirror { type Assoc; } impl<T> Mirror for T { type Assoc = T; } trait Foo {} trait Bar {} // The self type starts out as `?0` but is constrained to `()` // due to the where-clause below. Because `(): Bar` is known to // not hold, we can prove the impls disjoint. impl<T> Foo for T where (): Mirror<Assoc = T> {} //[current]~^ ERROR conflicting implementations of trait `Foo` for type `()` impl<T> Foo for T where T: Bar {} fn main() {} ``` The old solver does not run nested goals to a fixpoint in evaluation. The new solver does do so, strengthening inference and improving the overlap check: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Foo {} impl<T> Foo for (u8, T, T) {} trait NotU8 {} trait Bar {} impl<T, U: NotU8> Bar for (T, T, U) {} trait NeedsFixpoint {} impl<T: Foo + Bar> NeedsFixpoint for T {} impl NeedsFixpoint for (u8, u8, u8) {} trait Overlap {} impl<T: NeedsFixpoint> Overlap for T {} impl<T, U: NotU8, V> Overlap for (T, U, V) {} //[current]~^ ERROR conflicting implementations of trait `Foo` ``` ### Breakage due to removal of incomplete candidate preference Fixes rust-lang#107887. In the old solver we incompletely prefer the builtin trait object impl over user defined impls. This can break inference guidance, inferring `?x` in `dyn Trait<u32>: Trait<?x>` to `u32`, even if an explicit impl of `Trait<u64>` also exists. This caused coherence to incorrectly allow overlapping impls, resulting in ICEs and a theoretical unsoundness. See rust-lang#107887 (comment). This compiles on stable but results in an overlap error with `-Znext-solver=coherence`: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence struct W<T: ?Sized>(*const T); trait Trait<T: ?Sized> { type Assoc; } // This would trigger the check for overlap between automatic and custom impl. // They actually don't overlap so an impl like this should remain possible // forever. // // impl Trait<u64> for dyn Trait<u32> {} trait Indirect {} impl Indirect for dyn Trait<u32, Assoc = ()> {} impl<T: Indirect + ?Sized> Trait<u64> for T { type Assoc = (); } // Incomplete impl where `dyn Trait<u32>: Trait<_>` does not hold, but // `dyn Trait<u32>: Trait<u64>` does. trait EvaluateHack<U: ?Sized> {} impl<T: ?Sized, U: ?Sized> EvaluateHack<W<U>> for T where T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` U: IsU64, T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` { } trait IsU64 {} impl IsU64 for u64 {} trait Overlap<U: ?Sized> { type Assoc: Default; } impl<T: ?Sized + EvaluateHack<W<U>>, U: ?Sized> Overlap<U> for T { type Assoc = Box<u32>; } impl<U: ?Sized> Overlap<U> for dyn Trait<u32, Assoc = ()> { //[next]~^ ERROR conflicting implementations of trait `Overlap<_>` type Assoc = usize; } ``` ### Considering region outlives bounds in the `leak_check` For details on the `leak_check`, see the FCP proposal rust-lang#119820.[^leak_check] [^leak_check]: which should get moved to the dev-guide :3 In both coherence and during candidate selection, the `leak_check` relies on the region constraints added in `evaluate`. It therefore currently does not register outlives obligations: [source](https://github.com/rust-lang/rust/blob/ccb1415eac3289b5ebf64691c0190dc52e0e3d0e/compiler/rustc_trait_selection/src/traits/select/mod.rs#L792-L810). This was likely done as a performance optimization without considering its impact on the `leak_check`. This is the case as in the old solver, *evaluatation* and *fulfillment* are split, with evaluation being responsible for candidate selection and fulfillment actually registering all the constraints. This split does not exist with the new solver. The `leak_check` can therefore eagerly detect errors caused by region outlives obligations. This improves both coherence itself and candidate selection: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait LeakErr<'a, 'b> {} // Using this impl adds an `'b: 'a` bound which results // in a higher-ranked region error. This bound has been // previously ignored but is now considered. impl<'a, 'b: 'a> LeakErr<'a, 'b> for () {} trait NoOverlapDir<'a> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> NoOverlapDir<'a> for T {} impl<'a> NoOverlapDir<'a> for () {} //[current]~^ ERROR conflicting implementations of trait `NoOverlapDir<'_>` // -------------------------------------- // necessary to avoid coherence unknowable candidates struct W<T>(T); trait GuidesSelection<'a, U> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> GuidesSelection<'a, W<u32>> for T {} impl<'a, T> GuidesSelection<'a, W<u8>> for T {} trait NotImplementedByU8 {} trait NoOverlapInd<'a, U> {} impl<'a, T: GuidesSelection<'a, W<U>>, U> NoOverlapInd<'a, U> for T {} impl<'a, U: NotImplementedByU8> NoOverlapInd<'a, U> for () {} //[current]~^ conflicting implementations of trait `NoOverlapInd<'_, _>` ``` ### Removal of `fn match_fresh_trait_refs` The old solver tries to [eagerly detect unbounded recursion](https://github.com/rust-lang/rust/blob/b14fd2359f47fb9a14bbfe55359db4bb3af11861/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1196-L1211), forcing the affected goals to be ambiguous. This check is only an approximation and has not been added to the new solver. The check is not necessary in the new solver and it would be problematic for caching. As it depends on all goals currently on the stack, using a global cache entry would have to always make sure that doing so does not circumvent this check. This changes some goals to error - or succeed - instead of failing with ambiguity. This allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence // Need to use this local wrapper for the impls to be fully // knowable as unknowable candidate result in ambiguity. struct Local<T>(T); trait Trait<U> {} // This impl does not hold, but is ambiguous in the old // solver due to its overflow approximation. impl<U> Trait<U> for Local<u32> where Local<u16>: Trait<U> {} // This impl holds. impl Trait<Local<()>> for Local<u8> {} // In the old solver, `Local<?t>: Trait<Local<?u>>` is ambiguous, // resulting in `Local<?u>: NoImpl`, also being ambiguous. // // In the new solver the first impl does not apply, constraining // `?u` to `Local<()>`, causing `Local<()>: NoImpl` to error. trait Indirect<T> {} impl<T, U> Indirect<U> for T where T: Trait<U>, U: NoImpl {} // Not implemented for `Local<()>` trait NoImpl {} impl NoImpl for Local<u8> {} impl NoImpl for Local<u16> {} // `Local<?t>: Indirect<Local<?u>>` cannot hold, so // these impls do not overlap. trait NoOverlap<U> {} impl<T: Indirect<U>, U> NoOverlap<U> for T {} impl<T, U> NoOverlap<Local<U>> for Local<T> {} //~^ ERROR conflicting implementations of trait `NoOverlap<Local<_>>` ``` ### Non-fatal overflow The old solver immediately emits a fatal error when hitting the recursion limit. The new solver instead returns overflow. This both allows more code to compile and is results in performance and potential future compatability issues. Non-fatal overflow is generally desirable. With fatal overflow, changing the order in which we evaluate nested goals easily causes breakage if we have goal which errors and one which overflows. It is also required to prevent breakage due to the removal of `fn match_fresh_trait_refs`, e.g. [in `typenum`](rust-lang/trait-system-refactor-initiative#73). #### Enabling more code to compile In the below example, the old solver first tried to prove an overflowing goal, resulting in a fatal error. The new solver instead returns ambiguity due to overflow for that goal, causing the implicit negative overlap check to succeed as `Box<u32>: NotImplemented` does not hold. ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence //[current] ERROR overflow evaluating the requirement trait Indirect<T> {} impl<T: Overflow<()>> Indirect<T> for () {} trait Overflow<U> {} impl<T, U> Overflow<U> for Box<T> where U: Indirect<Box<Box<T>>>, {} trait NotImplemented {} trait Trait<U> {} impl<T, U> Trait<U> for T where // T: NotImplemented, // causes old solver to succeed U: Indirect<T>, T: NotImplemented, {} impl Trait<()> for Box<u32> {} ``` #### Avoiding hangs with non-fatal overflow Simply returning ambiguity when reaching the recursion limit can very easily result in hangs, e.g. ```rust trait Recur {} impl<T, U> Recur for ((T, U), (U, T)) where (T, U): Recur, (U, T): Recur, {} trait NotImplemented {} impl<T: NotImplemented> Recur for T {} ``` This can happen quite frequently as it's easy to have exponential blowup due to multiple nested goals at each step. As the trait solver is depth-first, this immediately caused a fatal overflow error in the old solver. In the new solver we have to handle the whole proof tree instead, which can very easily hang. To avoid this we restrict the recursion depth after hitting the recursion limit for the first time. We also **ignore all inference constraints from goals resulting in overflow**. This is mostly backwards compatible as any overflow in the old solver resulted in a fatal error. ### sidenote about normalization We return ambiguous nested goals of `NormalizesTo` goals to the caller and ignore their impact when computing the `Certainty` of the current goal. See the [normalization chapter](https://rustc-dev-guide.rust-lang.org/solve/normalization.html) for more details.This means we apply constraints resulting from other nested goals and from equating the impl header when normalizing, even if a nested goal results in overflow. This is necessary to avoid breaking the following example: ```rust trait Trait { type Assoc; } struct W<T: ?Sized>(*mut T); impl<T: ?Sized> Trait for W<W<T>> where W<T>: Trait, { type Assoc = (); } // `W<?t>: Trait<Assoc = u32>` does not hold as // `Assoc` gets normalized to `()`. However, proving // the where-bounds of the impl results in overflow. // // For this to continue to compile we must not discard // constraints from normalizing associated types. trait NoOverlap {} impl<T: Trait<Assoc = u32>> NoOverlap for T {} impl<T: ?Sized> NoOverlap for W<T> {} ``` #### Future compatability concerns Non-fatal overflow results in some unfortunate future compatability concerns. Changing the approach to avoid more hangs by more strongly penalizing overflow can cause breakage as we either drop constraints or ignore candidates necessary to successfully compile. Weakening the overflow penalities instead allows more code to compile and strengthens inference while potentially causing more code to hang. While the current approach is not perfect, we believe it to be good enough. We believe it to apply the necessary inference constraints to avoid breakage and expect there to not be any desirable patterns broken by our current penalities. Similarly we believe the current constraints to avoid most accidental hangs. Ignoring constraints of overflowing goals is especially useful, as it may allow major future optimizations to our overflow handling. See [this summary](https://hackmd.io/ATf4hN0NRY-w2LIVgeFsVg) and the linked documents in case you want to know more. ### changes to performance In general, trait solving during coherence checking is not significant for performance. Enabling the next-generation trait solver in coherence does not impact our compile time benchmarks. We are still unable to compile the benchmark suite when fully enabling the new trait solver. There are rare cases where the new solver has significantly worse performance due to non-fatal overflow, its reliance on fixpoint algorithms and the removal of the `fn match_fresh_trait_refs` approximation. We encountered such issues in [`typenum`](https://crates.io/crates/typenum) and believe it should be [pretty much as bad as it can get](rust-lang/trait-system-refactor-initiative#73). Due to an improved structure and far better caching, we believe that there is a lot of room for improvement and that the new solver will outperform the existing implementation in nearly all cases, sometimes significantly. We have not yet spent any time micro-optimizing the implementation and have many unimplemented major improvements, such as fast-paths for trivial goals. ### Unstable features #### Unsupported unstable features The new solver currently does not support all unstable features, most notably `#![feature(generic_const_exprs)]`, `#![feature(associated_const_equality)]` and `#![feature(adt_const_params)]` are not yet fully supported in the new solver. We are confident that supporting them is possible, but did not consider this to be a priority. This stabilization introduces new ICE when using these features in impl headers. #### fixes to `#![feature(specialization)]` - fixes rust-lang#105782 - fixes rust-lang#118987 #### fixes to `#![feature(type_alias_impl_trait)]` - fixes rust-lang#119272 - rust-lang#105787 (comment) - fixes rust-lang#124207 ### Important changes since the original FCP rust-lang#127574 changes the coherence unknowable candidate to only apply if all the super trait bounds may hold. This allows more code to compile and fixes a regression in `pyella` rust-lang#130617 bails with ambiguity if the query response would contain too many non-region inference variables. This should only be triggered in case the result contains a lot of ambiguous aliases in which case further constraining the goal should resolve this. rust-lang#130821 adds caching to a lot of type folders, which is necessary to handle exponentially large types and handles the hang in `nalgebra` together with rust-lang#130617. ## This does not stabilize the whole solver While this stabilizes the use of the new solver in coherence checking, there are many parts of the solver which will remain fully unstable. We may still adapt these areas while working towards stabilizing the new solver everywhere. We are confident that we are able to do so without negatively impacting coherence. ### goals with a non-empty `ParamEnv` Coherence always uses an empty environment. We therefore do not depend on the behavior of `AliasBound` and `ParamEnv` candidates. We only stabilizes the behavior of user-defined and builtin implementations of traits. There are still many open questions there. ### opaque types in the defining scope The handling of opaque types - `impl Trait` - in both the new and old solver is still not fully figured out. Luckily this can be ignored for now. While opaque types are reachable during coherence checking by using `impl_trait_in_associated_types`, the behavior during coherence is separate and self-contained. The old and new solver fully agree here. ### normalization is hard This stabilizes that we equate associated types involving bound variables using deferred-alias-equality. We also stop eagerly normalizing in coherence, which should not have any user-facing impact. We do not stabilize the normalization behavior outside of coherence, e.g. we currently deeply normalize all types during writeback with the new solver. This may change going forward ### how to replace `select` from the old solver We sometimes depend on getting a single `impl` for a given trait bound, e.g. when resolving a concrete method for codegen/CTFE. We do not depend on this during coherence, so the exact approach here can still be freely changed going forward. ## Acknowledgements This work would not have been possible without `@compiler-errors.` He implemented large chunks of the solver himself but also and did a lot of testing and experimentation, eagerly discovering multiple issues which had a significant impact on our approach. `@BoxyUwU` has also done some amazing work on the solver. Thank you for the endless hours of discussion resulting in the current approach. Especially the way aliases are handled has gone through multiple revisions to get to its current state. There were also many contributions from - and discussions with - other members of the community and the rest of `@rust-lang/types.` This solver builds upon previous improvements to the compiler, as well as lessons learned from `chalk` and `a-mir-formality`. Getting to this point would not have been possible without that and I am incredibly thankful to everyone involved. See the [list of relevant PRs](https://github.com/rust-lang/rust/pulls?q=is%3Apr+is%3Amerged+label%3AWG-trait-system-refactor+-label%3Arollup+closed%3A%3C2024-03-22+).
netbsd-srcmastr
pushed a commit
to NetBSD/pkgsrc
that referenced
this pull request
Oct 13, 2024
This is based on the pkgsrc-wip rust180 package, retaining the main pkgsrc changes as best as I could. Pkgsrc changes: * Adapt checksums and patches. * Make this work again on big-endian aarch64 (at least on NetBSD). * Make the choice of GCC = 12 work for sparc64 by testing options after options.mk is included (which is required...). Makes this work on NetBSD/sparc64 10.0 again. Upstream chnages: Version 1.80.1 (2024-08-08) =========================== - [Fix miscompilation in the jump threading MIR optimization when comparing floats] (rust-lang/rust#128271) - [Revert changes to the `dead_code` lint from 1.80.0] (rust-lang/rust#128618) Version 1.80.0 (2024-07-25) ========================== Language -------- - [Document maximum allocation size] (rust-lang/rust#116675) - [Allow zero-byte offsets and ZST read/writes on arbitrary pointers] (rust-lang/rust#117329) - [Support C23's variadics without a named parameter] (rust-lang/rust#124048) - [Stabilize `exclusive_range_pattern` feature] (rust-lang/rust#124459) - [Guarantee layout and ABI of `Result` in some scenarios] (rust-lang/rust#124870) Compiler -------- - [Update cc crate to v1.0.97 allowing additional spectre mitigations on MSVC targets] (rust-lang/rust#124892) - [Allow field reordering on types marked `repr(packed(1))`] (rust-lang/rust#125360) - [Add a lint against never type fallback affecting unsafe code] (rust-lang/rust#123939) - [Disallow cast with trailing braced macro in let-else] (rust-lang/rust#125049) - [Expand `for_loops_over_fallibles` lint to lint on fallibles behind references.] (rust-lang/rust#125156) - [self-contained linker: retry linking without `-fuse-ld=lld` on CCs that don't support it] (rust-lang/rust#125417) - [Do not parse CVarArgs (`...`) as a type in trait bounds] (rust-lang/rust#125863) - Improvements to LLDB formatting [#124458] (rust-lang/rust#124458) [#124500] (rust-lang/rust#124500) - [For the wasm32-wasip2 target default to PIC and do not use `-fuse-ld=lld`] (rust-lang/rust#124858) - [Add x86_64-unknown-linux-none as a tier 3 target] (rust-lang/rust#125023) - [Lint on `foo.into_iter()` resolving to `&Box<[T]>: IntoIterator`] (rust-lang/rust#124097) Libraries --------- - [Add `size_of` and `size_of_val` and `align_of` and `align_of_val` to the prelude] (rust-lang/rust#123168) - [Abort a process when FD ownership is violated] (rust-lang/rust#124210) - [io::Write::write_fmt: panic if the formatter fails when the stream does not fail] (rust-lang/rust#125012) - [Panic if `PathBuf::set_extension` would add a path separator] (rust-lang/rust#125070) - [Add assert_unsafe_precondition to unchecked_{add,sub,neg,mul,shl,shr} methods] (rust-lang/rust#121571) - [Update `c_char` on AIX to use the correct type] (rust-lang/rust#122986) - [`offset_of!` no longer returns a temporary] (rust-lang/rust#124484) - [Handle sigma in `str.to_lowercase` correctly] (rust-lang/rust#124773) - [Raise `DEFAULT_MIN_STACK_SIZE` to at least 64KiB] (rust-lang/rust#126059) Stabilized APIs --------------- - [`impl Default for Rc<CStr>`] (https://doc.rust-lang.org/beta/alloc/rc/struct.Rc.html#impl-Default-for-Rc%3CCStr%3E) - [`impl Default for Rc<str>`] (https://doc.rust-lang.org/beta/alloc/rc/struct.Rc.html#impl-Default-for-Rc%3Cstr%3E) - [`impl Default for Rc<[T]>`] (https://doc.rust-lang.org/beta/alloc/rc/struct.Rc.html#impl-Default-for-Rc%3C%5BT%5D%3E) - [`impl Default for Arc<str>`] (https://doc.rust-lang.org/beta/alloc/sync/struct.Arc.html#impl-Default-for-Arc%3Cstr%3E) - [`impl Default for Arc<CStr>`] (https://doc.rust-lang.org/beta/alloc/sync/struct.Arc.html#impl-Default-for-Arc%3CCStr%3E) - [`impl Default for Arc<[T]>`] (https://doc.rust-lang.org/beta/alloc/sync/struct.Arc.html#impl-Default-for-Arc%3C%5BT%5D%3E) - [`impl IntoIterator for Box<[T]>`] (https://doc.rust-lang.org/beta/alloc/boxed/struct.Box.html#impl-IntoIterator-for-Box%3C%5BI%5D,+A%3E) - [`impl FromIterator<String> for Box<str>`] (https://doc.rust-lang.org/beta/alloc/boxed/struct.Box.html#impl-FromIterator%3CString%3E-for-Box%3Cstr%3E) - [`impl FromIterator<char> for Box<str>`] (https://doc.rust-lang.org/beta/alloc/boxed/struct.Box.html#impl-FromIterator%3Cchar%3E-for-Box%3Cstr%3E) - [`LazyCell`] (https://doc.rust-lang.org/beta/core/cell/struct.LazyCell.html) - [`LazyLock`] (https://doc.rust-lang.org/beta/std/sync/struct.LazyLock.html) - [`Duration::div_duration_f32`] (https://doc.rust-lang.org/beta/std/time/struct.Duration.html#method.div_duration_f32) - [`Duration::div_duration_f64`] (https://doc.rust-lang.org/beta/std/time/struct.Duration.html#method.div_duration_f64) - [`Option::take_if`] (https://doc.rust-lang.org/beta/std/option/enum.Option.html#method.take_if) - [`Seek::seek_relative`] (https://doc.rust-lang.org/beta/std/io/trait.Seek.html#method.seek_relative) - [`BinaryHeap::as_slice`] (https://doc.rust-lang.org/beta/std/collections/struct.BinaryHeap.html#method.as_slice) - [`NonNull::offset`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.offset) - [`NonNull::byte_offset`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.byte_offset) - [`NonNull::add`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.add) - [`NonNull::byte_add`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.byte_add) - [`NonNull::sub`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.sub) - [`NonNull::byte_sub`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.byte_sub) - [`NonNull::offset_from`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.offset_from) - [`NonNull::byte_offset_from`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.byte_offset_from) - [`NonNull::read`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.read) - [`NonNull::read_volatile`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.read_volatile) - [`NonNull::read_unaligned`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.read_unaligned) - [`NonNull::write`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.write) - [`NonNull::write_volatile`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.write_volatile) - [`NonNull::write_unaligned`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.write_unaligned) - [`NonNull::write_bytes`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.write_bytes) - [`NonNull::copy_to`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.copy_to) - [`NonNull::copy_to_nonoverlapping`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.copy_to_nonoverlapping) - [`NonNull::copy_from`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.copy_from) - [`NonNull::copy_from_nonoverlapping`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.copy_from_nonoverlapping) - [`NonNull::replace`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.replace) - [`NonNull::swap`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.swap) - [`NonNull::drop_in_place`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.drop_in_place) - [`NonNull::align_offset`] (https://doc.rust-lang.org/beta/std/ptr/struct.NonNull.html#method.align_offset) - [`<[T]>::split_at_checked`] (https://doc.rust-lang.org/beta/std/primitive.slice.html#method.split_at_checked) - [`<[T]>::split_at_mut_checked`] (https://doc.rust-lang.org/beta/std/primitive.slice.html#method.split_at_mut_checked) - [`str::split_at_checked`] (https://doc.rust-lang.org/beta/std/primitive.str.html#method.split_at_checked) - [`str::split_at_mut_checked`] (https://doc.rust-lang.org/beta/std/primitive.str.html#method.split_at_mut_checked) - [`str::trim_ascii`] (https://doc.rust-lang.org/beta/std/primitive.str.html#method.trim_ascii) - [`str::trim_ascii_start`] (https://doc.rust-lang.org/beta/std/primitive.str.html#method.trim_ascii_start) - [`str::trim_ascii_end`] (https://doc.rust-lang.org/beta/std/primitive.str.html#method.trim_ascii_end) - [`<[u8]>::trim_ascii`] (https://doc.rust-lang.org/beta/core/primitive.slice.html#method.trim_ascii) - [`<[u8]>::trim_ascii_start`] (https://doc.rust-lang.org/beta/core/primitive.slice.html#method.trim_ascii_start) - [`<[u8]>::trim_ascii_end`] (https://doc.rust-lang.org/beta/core/primitive.slice.html#method.trim_ascii_end) - [`Ipv4Addr::BITS`] (https://doc.rust-lang.org/beta/core/net/struct.Ipv4Addr.html#associatedconstant.BITS) - [`Ipv4Addr::to_bits`] (https://doc.rust-lang.org/beta/core/net/struct.Ipv4Addr.html#method.to_bits) - [`Ipv4Addr::from_bits`] (https://doc.rust-lang.org/beta/core/net/struct.Ipv4Addr.html#method.from_bits) - [`Ipv6Addr::BITS`] (https://doc.rust-lang.org/beta/core/net/struct.Ipv6Addr.html#associatedconstant.BITS) - [`Ipv6Addr::to_bits`] (https://doc.rust-lang.org/beta/core/net/struct.Ipv6Addr.html#method.to_bits) - [`Ipv6Addr::from_bits`] (https://doc.rust-lang.org/beta/core/net/struct.Ipv6Addr.html#method.from_bits) - [`Vec::<[T; N]>::into_flattened`] (https://doc.rust-lang.org/beta/alloc/vec/struct.Vec.html#method.into_flattened) - [`<[[T; N]]>::as_flattened`] (https://doc.rust-lang.org/beta/core/primitive.slice.html#method.as_flattened) - [`<[[T; N]]>::as_flattened_mut`] (https://doc.rust-lang.org/beta/core/primitive.slice.html#method.as_flattened_mut) These APIs are now stable in const contexts: - [`<[T]>::last_chunk`] (https://doc.rust-lang.org/beta/core/primitive.slice.html#method.last_chunk) - [`BinaryHeap::new`] (https://doc.rust-lang.org/beta/std/collections/struct.BinaryHeap.html#method.new) Cargo ----- - [Stabilize `-Zcheck-cfg` as always enabled] (rust-lang/cargo#13571) - [Warn, rather than fail publish, if a target is excluded] (rust-lang/cargo#13713) - [Add special `check-cfg` lint config for the `unexpected_cfgs` lint] (rust-lang/cargo#13913) - [Stabilize `cargo update --precise <yanked>`] (rust-lang/cargo#13974) - [Don't change file permissions on `Cargo.toml` when using `cargo add`] (rust-lang/cargo#13898) - [Support using `cargo fix` on IPv6-only networks] (rust-lang/cargo#13907) Rustdoc ----- - [Allow searching for references] (rust-lang/rust#124148) - [Stabilize `custom_code_classes_in_docs` feature] (rust-lang/rust#124577) - [fix: In cross-crate scenarios show enum variants on type aliases of enums] (rust-lang/rust#125300) Compatibility Notes ------------------- - [rustfmt estimates line lengths differently when using non-ascii characters] (rust-lang/rustfmt#6203) - [Type aliases are now handled correctly in orphan check] (rust-lang/rust#117164) - [Allow instructing rustdoc to read from stdin via `-`] (rust-lang/rust#124611) - [`std::env::{set_var, remove_var}` can no longer be converted to safe function pointers and no longer implement the `Fn` family of traits] (rust-lang/rust#124636) - [Warn (or error) when `Self` constructor from outer item is referenced in inner nested item] (rust-lang/rust#124187) - [Turn `indirect_structural_match` and `pointer_structural_match` lints into hard errors] (rust-lang/rust#124661) - [Make `where_clause_object_safety` lint a regular object safety violation] (rust-lang/rust#125380) - [Turn `proc_macro_back_compat` lint into a hard error.] (rust-lang/rust#125596) - [Detect unused structs even when implementing private traits] (rust-lang/rust#122382) - [`std::sync::ReentrantLockGuard<T>` is no longer `Sync` if `T: !Sync`] (rust-lang/rust#125527) which means [`std::io::StdoutLock` and `std::io::StderrLock` are no longer Sync] (rust-lang/rust#127340) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - Misc improvements to size of generated html by rustdoc e.g. [#124738] (rust-lang/rust#124738) and [#123734] (rust-lang/rust#123734) - [MSVC targets no longer depend on libc] (rust-lang/rust#124050) Version 1.79.0 (2024-06-13) ========================== Language -------- - [Stabilize inline `const {}` expressions.] (rust-lang/rust#104087) - [Prevent opaque types being instantiated twice with different regions within the same function.] (rust-lang/rust#116935) - [Stabilize WebAssembly target features that are in phase 4 and 5.] (rust-lang/rust#117457) - [Add the `redundant_lifetimes` lint to detect lifetimes which are semantically redundant.] (rust-lang/rust#118391) - [Stabilize the `unnameable_types` lint for public types that can't be named.] (rust-lang/rust#120144) - [Enable debuginfo in macros, and stabilize `-C collapse-macro-debuginfo` and `#[collapse_debuginfo]`.] (rust-lang/rust#120845) - [Propagate temporary lifetime extension into `if` and `match` expressions.] (rust-lang/rust#121346) - [Restrict promotion of `const fn` calls.] (rust-lang/rust#121557) - [Warn against refining impls of crate-private traits with `refining_impl_trait` lint.] (rust-lang/rust#121720) - [Stabilize associated type bounds (RFC 2289).] (rust-lang/rust#122055) - [Stabilize importing `main` from other modules or crates.] (rust-lang/rust#122060) - [Check return types of function types for well-formedness] (rust-lang/rust#115538) - [Rework `impl Trait` lifetime inference] (rust-lang/rust#116891) - [Change inductive trait solver cycles to be ambiguous] (rust-lang/rust#122791) Compiler -------- - [Define `-C strip` to only affect binaries, not artifacts like `.pdb`.] (rust-lang/rust#115120) - [Stabilize `-Crelro-level` for controlling runtime link hardening.] (rust-lang/rust#121694) - [Stabilize checking of `cfg` names and values at compile-time with `--check-cfg`.] (rust-lang/rust#123501) *Note that this only stabilizes the compiler part, the Cargo part is still unstable in this release.* - [Add `aarch64-apple-visionos` and `aarch64-apple-visionos-sim` tier 3 targets.] (rust-lang/rust#121419) - [Add `riscv32ima-unknown-none-elf` tier 3 target.] (rust-lang/rust#122696) - [Promote several Windows targets to tier 2] (rust-lang/rust#121712): `aarch64-pc-windows-gnullvm`, `i686-pc-windows-gnullvm`, and `x86_64-pc-windows-gnullvm`. Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Implement `FromIterator` for `(impl Default + Extend, impl Default + Extend)`.] (rust-lang/rust#107462) - [Implement `{Div,Rem}Assign<NonZero<X>>` on `X`.] (rust-lang/rust#121952) - [Document overrides of `clone_from()` in core/std.] (rust-lang/rust#122201) - [Link MSVC default lib in core.] (rust-lang/rust#122268) - [Caution against using `transmute` between pointers and integers.] (rust-lang/rust#122379) - [Enable frame pointers for the standard library.] (rust-lang/rust#122646) Stabilized APIs --------------- - [`{integer}::unchecked_add`] (https://doc.rust-lang.org/stable/core/primitive.i32.html#method.unchecked_add) - [`{integer}::unchecked_mul`] (https://doc.rust-lang.org/stable/core/primitive.i32.html#method.unchecked_mul) - [`{integer}::unchecked_sub`] (https://doc.rust-lang.org/stable/core/primitive.i32.html#method.unchecked_sub) - [`<[T]>::split_at_unchecked`] (https://doc.rust-lang.org/stable/core/primitive.slice.html#method.split_at_unchecked) - [`<[T]>::split_at_mut_unchecked`] (https://doc.rust-lang.org/stable/core/primitive.slice.html#method.split_at_mut_unchecked) - [`<[u8]>::utf8_chunks`] (https://doc.rust-lang.org/stable/core/primitive.slice.html#method.utf8_chunks) - [`str::Utf8Chunks`] (https://doc.rust-lang.org/stable/core/str/struct.Utf8Chunks.html) - [`str::Utf8Chunk`] (https://doc.rust-lang.org/stable/core/str/struct.Utf8Chunk.html) - [`<*const T>::is_aligned`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.is_aligned) - [`<*mut T>::is_aligned`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.is_aligned-1) - [`NonNull::is_aligned`] (https://doc.rust-lang.org/stable/core/ptr/struct.NonNull.html#method.is_aligned) - [`<*const [T]>::len`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.len) - [`<*mut [T]>::len`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.len-1) - [`<*const [T]>::is_empty`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.is_empty) - [`<*mut [T]>::is_empty`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.is_empty-1) - [`NonNull::<[T]>::is_empty`] (https://doc.rust-lang.org/stable/core/ptr/struct.NonNull.html#method.is_empty) - [`CStr::count_bytes`] (https://doc.rust-lang.org/stable/core/ffi/c_str/struct.CStr.html#method.count_bytes) - [`io::Error::downcast`] (https://doc.rust-lang.org/stable/std/io/struct.Error.html#method.downcast) - [`num::NonZero<T>`] (https://doc.rust-lang.org/stable/core/num/struct.NonZero.html) - [`path::absolute`] (https://doc.rust-lang.org/stable/std/path/fn.absolute.html) - [`proc_macro::Literal::byte_character`] (https://doc.rust-lang.org/stable/proc_macro/struct.Literal.html#method.byte_character) - [`proc_macro::Literal::c_string`] (https://doc.rust-lang.org/stable/proc_macro/struct.Literal.html#method.c_string) These APIs are now stable in const contexts: - [`Atomic*::into_inner`] (https://doc.rust-lang.org/stable/core/sync/atomic/struct.AtomicUsize.html#method.into_inner) - [`io::Cursor::new`] (https://doc.rust-lang.org/stable/std/io/struct.Cursor.html#method.new) - [`io::Cursor::get_ref`] (https://doc.rust-lang.org/stable/std/io/struct.Cursor.html#method.get_ref) - [`io::Cursor::position`] (https://doc.rust-lang.org/stable/std/io/struct.Cursor.html#method.position) - [`io::empty`] (https://doc.rust-lang.org/stable/std/io/fn.empty.html) - [`io::repeat`] (https://doc.rust-lang.org/stable/std/io/fn.repeat.html) - [`io::sink`] (https://doc.rust-lang.org/stable/std/io/fn.sink.html) - [`panic::Location::caller`] (https://doc.rust-lang.org/stable/std/panic/struct.Location.html#method.caller) - [`panic::Location::file`] (https://doc.rust-lang.org/stable/std/panic/struct.Location.html#method.file) - [`panic::Location::line`] (https://doc.rust-lang.org/stable/std/panic/struct.Location.html#method.line) - [`panic::Location::column`] (https://doc.rust-lang.org/stable/std/panic/struct.Location.html#method.column) Cargo ----- - [Prevent dashes in `lib.name`, always normalizing to `_`.] (rust-lang/cargo#12783) - [Stabilize MSRV-aware version requirement selection in `cargo add`.] (rust-lang/cargo#13608) - [Switch to using `gitoxide` by default for listing files.] (rust-lang/cargo#13696) Rustdoc ----- - [Always display stability version even if it's the same as the containing item.] (rust-lang/rust#118441) - [Show a single search result for items with multiple paths.] (rust-lang/rust#119912) - [Support typing `/` in docs to begin a search.] (rust-lang/rust#123355) Misc ---- Compatibility Notes ------------------- - [Update the minimum external LLVM to 17.] (rust-lang/rust#122649) - [`RustcEncodable` and `RustcDecodable` are soft-destabilized, to be removed from the prelude in next edition.] (rust-lang/rust#116016) - [The `wasm_c_abi` future-incompatibility lint will warn about use of the non-spec-compliant C ABI.] (rust-lang/rust#117918) Use `wasm-bindgen v0.2.88` to generate forward-compatible bindings. - [Check return types of function types for well-formedness] (rust-lang/rust#115538) Version 1.78.0 (2024-05-02) =========================== Language -------- - [Stabilize `#[cfg(target_abi = ...)]`] (rust-lang/rust#119590) - [Stabilize the `#[diagnostic]` namespace and `#[diagnostic::on_unimplemented]` attribute] (rust-lang/rust#119888) - [Make async-fn-in-trait implementable with concrete signatures] (rust-lang/rust#120103) - [Make matching on NaN a hard error, and remove the rest of `illegal_floating_point_literal_pattern`] (rust-lang/rust#116284) - [static mut: allow mutable reference to arbitrary types, not just slices and arrays] (rust-lang/rust#117614) - [Extend `invalid_reference_casting` to include references casting to bigger memory layout] (rust-lang/rust#118983) - [Add `non_contiguous_range_endpoints` lint for singleton gaps after exclusive ranges] (rust-lang/rust#118879) - [Add `wasm_c_abi` lint for use of older wasm-bindgen versions] (rust-lang/rust#117918) This lint currently only works when using Cargo. - [Update `indirect_structural_match` and `pointer_structural_match` lints to match RFC] (rust-lang/rust#120423) - [Make non-`PartialEq`-typed consts as patterns a hard error] (rust-lang/rust#120805) - [Split `refining_impl_trait` lint into `_reachable`, `_internal` variants] (rust-lang/rust#121720) - [Remove unnecessary type inference when using associated types inside of higher ranked `where`-bounds] (rust-lang/rust#119849) - [Weaken eager detection of cyclic types during type inference] (rust-lang/rust#119989) - [`trait Trait: Auto {}`: allow upcasting from `dyn Trait` to `dyn Auto`] (rust-lang/rust#119338) Compiler -------- - [Made `INVALID_DOC_ATTRIBUTES` lint deny by default] (rust-lang/rust#111505) - [Increase accuracy of redundant `use` checking] (rust-lang/rust#117772) - [Suggest moving definition if non-found macro_rules! is defined later] (rust-lang/rust#121130) - [Lower transmutes from int to pointer type as gep on null] (rust-lang/rust#121282) Target changes: - [Windows tier 1 targets now require at least Windows 10] (rust-lang/rust#115141) - [Enable CMPXCHG16B, SSE3, SAHF/LAHF and 128-bit Atomics in tier 1 Windows] (rust-lang/rust#120820) - [Add `wasm32-wasip1` tier 2 (without host tools) target] (rust-lang/rust#120468) - [Add `wasm32-wasip2` tier 3 target] (rust-lang/rust#119616) - [Rename `wasm32-wasi-preview1-threads` to `wasm32-wasip1-threads`] (rust-lang/rust#122170) - [Add `arm64ec-pc-windows-msvc` tier 3 target] (rust-lang/rust#119199) - [Add `armv8r-none-eabihf` tier 3 target for the Cortex-R52] (rust-lang/rust#110482) - [Add `loongarch64-unknown-linux-musl` tier 3 target] (rust-lang/rust#121832) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Bump Unicode to version 15.1.0, regenerate tables] (rust-lang/rust#120777) - [Make align_offset, align_to well-behaved in all cases] (rust-lang/rust#121201) - [PartialEq, PartialOrd: document expectations for transitive chains] (rust-lang/rust#115386) - [Optimize away poison guards when std is built with panic=abort] (rust-lang/rust#100603) - [Replace pthread `RwLock` with custom implementation] (rust-lang/rust#110211) - [Implement unwind safety for Condvar on all platforms] (rust-lang/rust#121768) - [Add ASCII fast-path for `char::is_grapheme_extended`] (rust-lang/rust#121138) Stabilized APIs --------------- - [`impl Read for &Stdin`] (https://doc.rust-lang.org/stable/std/io/struct.Stdin.html#impl-Read-for-%26Stdin) - [Accept non `'static` lifetimes for several `std::error::Error` related implementations] (rust-lang/rust#113833) - [Make `impl<Fd: AsFd>` impl take `?Sized`] (rust-lang/rust#114655) - [`impl From<TryReserveError> for io::Error`] (https://doc.rust-lang.org/stable/std/io/struct.Error.html#impl-From%3CTryReserveError%3E-for-Error) These APIs are now stable in const contexts: - [`Barrier::new()`] (https://doc.rust-lang.org/stable/std/sync/struct.Barrier.html#method.new) Cargo ----- - [Stabilize lockfile v4](rust-lang/cargo#12852) - [Respect `rust-version` when generating lockfile] (rust-lang/cargo#12861) - [Control `--charset` via auto-detecting config value] (rust-lang/cargo#13337) - [Support `target.<triple>.rustdocflags` officially] (rust-lang/cargo#13197) - [Stabilize global cache data tracking] (rust-lang/cargo#13492) Misc ---- - [rustdoc: add `--test-builder-wrapper` arg to support wrappers such as RUSTC_WRAPPER when building doctests] (rust-lang/rust#114651) Compatibility Notes ------------------- - [Many unsafe precondition checks now run for user code with debug assertions enabled] (rust-lang/rust#120594) This change helps users catch undefined behavior in their code, though the details of how much is checked are generally not stable. - [riscv only supports split_debuginfo=off for now] (rust-lang/rust#120518) - [Consistently check bounds on hidden types of `impl Trait`] (rust-lang/rust#121679) - [Change equality of higher ranked types to not rely on subtyping] (rust-lang/rust#118247) - [When called, additionally check bounds on normalized function return type] (rust-lang/rust#118882) - [Expand coverage for `arithmetic_overflow` lint] (rust-lang/rust#119432) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Update to LLVM 18](rust-lang/rust#120055) - [Build `rustc` with 1CGU on `x86_64-pc-windows-msvc`] (rust-lang/rust#112267) - [Build `rustc` with 1CGU on `x86_64-apple-darwin`] (rust-lang/rust#112268) - [Introduce `run-make` V2 infrastructure, a `run_make_support` library and port over 2 tests as example] (rust-lang/rust#113026) - [Windows: Implement condvar, mutex and rwlock using futex] (rust-lang/rust#121956) Version 1.77.0 (2024-03-21) ========================== - [Reveal opaque types within the defining body for exhaustiveness checking.] (rust-lang/rust#116821) - [Stabilize C-string literals.] (rust-lang/rust#117472) - [Stabilize THIR unsafeck.] (rust-lang/rust#117673) - [Add lint `static_mut_refs` to warn on references to mutable statics.] (rust-lang/rust#117556) - [Support async recursive calls (as long as they have indirection).] (rust-lang/rust#117703) - [Undeprecate lint `unstable_features` and make use of it in the compiler.] (rust-lang/rust#118639) - [Make inductive cycles in coherence ambiguous always.] (rust-lang/rust#118649) - [Get rid of type-driven traversal in const-eval interning] (rust-lang/rust#119044), only as a [future compatiblity lint] (rust-lang/rust#122204) for now. - [Deny braced macro invocations in let-else.] (rust-lang/rust#119062) Compiler -------- - [Include lint `soft_unstable` in future breakage reports.] (rust-lang/rust#116274) - [Make `i128` and `u128` 16-byte aligned on x86-based targets.] (rust-lang/rust#116672) - [Use `--verbose` in diagnostic output.] (rust-lang/rust#119129) - [Improve spacing between printed tokens.] (rust-lang/rust#120227) - [Merge the `unused_tuple_struct_fields` lint into `dead_code`.] (rust-lang/rust#118297) - [Error on incorrect implied bounds in well-formedness check] (rust-lang/rust#118553), with a temporary exception for Bevy. - [Fix coverage instrumentation/reports for non-ASCII source code.] (rust-lang/rust#119033) - [Fix `fn`/`const` items implied bounds and well-formedness check.] (rust-lang/rust#120019) - [Promote `riscv32{im|imafc}-unknown-none-elf` targets to tier 2.] (rust-lang/rust#118704) - Add several new tier 3 targets: - [`aarch64-unknown-illumos`] (rust-lang/rust#112936) - [`hexagon-unknown-none-elf`] (rust-lang/rust#117601) - [`riscv32imafc-esp-espidf`] (rust-lang/rust#119738) - [`riscv32im-risc0-zkvm-elf`] (rust-lang/rust#117958) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Implement `From<&[T; N]>` for `Cow<[T]>`.] (rust-lang/rust#113489) - [Remove special-case handling of `vec.split_off (0)`.](rust-lang/rust#119917) Stabilized APIs --------------- - [`array::each_ref`] (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_ref) - [`array::each_mut`] (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_mut) - [`core::net`] (https://doc.rust-lang.org/stable/core/net/index.html) - [`f32::round_ties_even`] (https://doc.rust-lang.org/stable/std/primitive.f32.html#method.round_ties_even) - [`f64::round_ties_even`] (https://doc.rust-lang.org/stable/std/primitive.f64.html#method.round_ties_even) - [`mem::offset_of!`] (https://doc.rust-lang.org/stable/std/mem/macro.offset_of.html) - [`slice::first_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk) - [`slice::first_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk_mut) - [`slice::split_first_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk) - [`slice::split_first_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk_mut) - [`slice::last_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk) - [`slice::last_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk_mut) - [`slice::split_last_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk) - [`slice::split_last_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk_mut) - [`slice::chunk_by`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by) - [`slice::chunk_by_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by_mut) - [`Bound::map`] (https://doc.rust-lang.org/stable/std/ops/enum.Bound.html#method.map) - [`File::create_new`] (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.create_new) - [`Mutex::clear_poison`] (https://doc.rust-lang.org/stable/std/sync/struct.Mutex.html#method.clear_poison) - [`RwLock::clear_poison`] (https://doc.rust-lang.org/stable/std/sync/struct.RwLock.html#method.clear_poison) Cargo ----- - [Extend the build directive syntax with `cargo::`.] (rust-lang/cargo#12201) - [Stabilize metadata `id` format as `PackageIDSpec`.] (rust-lang/cargo#12914) - [Pull out as `cargo-util-schemas` as a crate.] (rust-lang/cargo#13178) - [Strip all debuginfo when debuginfo is not requested.] (rust-lang/cargo#13257) - [Inherit jobserver from env for all kinds of runners.] (rust-lang/cargo#12776) - [Deprecate rustc plugin support in cargo.] (rust-lang/cargo#13248) Rustdoc ----- - [Allows links in markdown headings.] (rust-lang/rust#117662) - [Search for tuples and unit by type with `()`.] (rust-lang/rust#118194) - [Clean up the source sidebar's hide button.] (rust-lang/rust#119066) - [Prevent JS injection from `localStorage`.] (rust-lang/rust#120250) Misc ---- - [Recommend version-sorting for all sorting in style guide.] (rust-lang/rust#115046) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Add more weirdness to `weird-exprs.rs`.] (rust-lang/rust#119028)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #99554, fixes rust-lang/types-team#104.
Fixes #114061.
Supersedes #100555.
Tracking issue for the future compatibility lint: #124559.
r? lcnr