layout computation: gracefully handle unsized types in unexpected locations
This PR reworks the layout computation to eagerly return an error when encountering an unsized field where a sized field was expected, rather than delaying a bug and attempting to recover a layout. This is required, because with trivially false where clauses like `[T]: Sized`, any field can possible be an unsized type, without causing a compile error.
Since this PR removes the `delayed_bug` method from the `LayoutCalculator` trait, it essentially becomes the same as the `HasDataLayout` trait, so I've also refactored the `LayoutCalculator` to be a simple wrapper struct around a type that implements `HasDataLayout`.
The majority of the diff is whitespace changes, so viewing with whitespace ignored is advised.
implements https://github.com/rust-lang/rust/pull/123169#issuecomment-2025788480
r? `@compiler-errors` or compiler
fixes https://github.com/rust-lang/rust/issues/123134
fixes https://github.com/rust-lang/rust/issues/124182
fixes https://github.com/rust-lang/rust/issues/126939
fixes https://github.com/rust-lang/rust/issues/127737
For structs that cannot be unsized, the layout algorithm sometimes moves
unsized fields to the end of the struct, which circumvented the error
for unexpected unsized fields and returned an unsized layout anyway.
This commit makes it so that the unexpected unsized error is always
returned for structs that cannot be unsized, allowing us to remove an
old hack and fixing some old ICE.
Rollup of 3 pull requests
Successful merges:
- #130033 (Don't call `fn_arg_names` query for non-`fn` foreign items in resolver)
- #130282 (Do not report an excessive number of overflow errors for an ever-growing deref impl)
- #130437 (Avoid crashing on variadic functions when producing arg-mismatch errors)
r? `@ghost`
`@rustbot` modify labels: rollup
Rollup of 4 pull requests
Successful merges:
- #123436 (linker: Allow MSVC to use import libraries following the Meson/MinGW convention)
- #130410 (Don't ICE when generating `Fn` shim for async closure with borrowck error)
- #130412 (Don't ICE when RPITIT captures more method args than trait definition)
- #130436 (Ignore reduce-fadd-unordered on SGX platform)
r? `@ghost`
`@rustbot` modify labels: rollup
Don't ICE when RPITIT captures more method args than trait definition
Make sure we don't ICE when an RPITIT captures more method args than the trait definition, which is not allowed. This was because we were using the wrong def id for error reporting.
Due to the default lifetime capture rules of RPITITs (capturing everything in scope), this is only doable if we use precise capturing, which isn't currently allowed for RPITITs anyways but we still end up reaching the relevant codepaths.
Fixes#129850
Correctly account for niche-optimized tags in rustc_transmute
This is a bit hacky, but it fixes the ICE and makes it possible to run the safe transmute check on every `mem::transmute` check we instantiate. I want to write a lint that needs to do that, but this stands well on its own.
cc `@jswrenn` here's the fix I alluded to yesterday :)
Fixes#123693
const-eval interning: accept interior mutable pointers in final value
…but keep rejecting mutable references
This fixes https://github.com/rust-lang/rust/issues/121610 by no longer firing the lint when there is a pointer with interior mutability in the final value of the constant. On stable, such pointers can be created with code like:
```rust
pub enum JsValue {
Undefined,
Object(Cell<bool>),
}
impl Drop for JsValue {
fn drop(&mut self) {}
}
// This does *not* get promoted since `JsValue` has a destructor.
// However, the outer scope rule applies, still giving this 'static lifetime.
const UNDEFINED: &JsValue = &JsValue::Undefined;
```
It's not great to accept such values since people *might* think that it is legal to mutate them with unsafe code. (This is related to how "infectious" `UnsafeCell` is, which is a [wide open question](https://github.com/rust-lang/unsafe-code-guidelines/issues/236).) However, we [explicitly document](https://doc.rust-lang.org/reference/behavior-considered-undefined.html) that things created by `const` are immutable. Furthermore, we also accept the following even more questionable code without any lint today:
```rust
let x: &'static Option<Cell<i32>> = &None;
```
This is even more questionable since it does *not* involve a `const`, and yet still puts the data into immutable memory. We could view this as promotion [potentially introducing UB](https://github.com/rust-lang/unsafe-code-guidelines/issues/493). However, we've accepted this since ~forever and it's [too late to reject this now](https://github.com/rust-lang/rust/pull/122789); the pattern is just too useful.
So basically, if you think that `UnsafeCell` should be tracked fully precisely, then you should want the lint we currently emit to be removed, which this PR does. If you think `UnsafeCell` should "infect" surrounding `enum`s, the big problem is really https://github.com/rust-lang/unsafe-code-guidelines/issues/493 which does not trigger the lint -- the cases the lint triggers on are actually the "harmless" ones as there is an explicit surrounding `const` explaining why things end up being immutable.
What all this goes to show is that the hard error added in https://github.com/rust-lang/rust/pull/118324 (later turned into the future-compat lint that I am now suggesting we remove) was based on some wrong assumptions, at least insofar as it concerns shared references. Furthermore, that lint does not help at all for the most problematic case here where the potential UB is completely implicit. (In fact, the lint is actively in the way of [my preferred long-term strategy](https://github.com/rust-lang/unsafe-code-guidelines/issues/493#issuecomment-2028674105) for dealing with this UB.) So I think we should go back to square one and remove that error/lint for shared references. For mutable references, it does seem to work as intended, so we can keep it. Here it serves as a safety net in case the static checks that try to contain mutable references to the inside of a const initializer are not working as intended; I therefore made the check ICE to encourage users to tell us if that safety net is triggered.
Closes https://github.com/rust-lang/rust/issues/122153 by removing the lint.
Cc `@rust-lang/opsem` `@rust-lang/lang`
Fix double handling in `collect_tokens`
Double handling of AST nodes can occur in `collect_tokens`. This is when an inner call to `collect_tokens` produces an AST node, and then an outer call to `collect_tokens` produces the same AST node. This can happen in a few places, e.g. expression statements where the statement delegates `HasTokens` and `HasAttrs` to the expression. It will also happen more after #124141.
This PR fixes some double handling cases that cause problems, including #129166.
r? `@petrochenkov`
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 https://github.com/rust-lang/rust/issues/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](bea5bebf3d/compiler/rustc_trait_selection/src/solve/assembly/mod.rs (L858-L883)).
[orphan check]: fd80c02c16/compiler/rustc_trait_selection/src/traits/coherence.rs (L566-L579)
[overlap check]: fd80c02c16/compiler/rustc_trait_selection/src/traits/coherence.rs (L92-L98)
[implicit negative]: fd80c02c16/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 https://github.com/rust-lang/rust/issues/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 https://github.com/rust-lang/rust/issues/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](47dd709bed/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 https://github.com/rust-lang/rust/pull/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](bea5bebf3d/compiler/rustc_trait_selection/src/traits/coherence.rs (L323-L338)). The new solver instead [uses a `FulfillmentCtxt`](bea5bebf3d/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 https://github.com/rust-lang/rust/issues/107887#issuecomment-1997261676. 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](ccb1415eac/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](b14fd2359f/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`](https://github.com/rust-lang/trait-system-refactor-initiative/issues/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](https://github.com/rust-lang/trait-system-refactor-initiative/issues/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
- https://github.com/rust-lang/rust/issues/105787#issuecomment-1750112388
- 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+).
add crashtests for several old unfixed ICEs
Adds several new crashtests for some older ICEs that did not yet have any.
Tests were added for #128097, #119095, #117460 and #126443.
As our implementation of MCP411 nears completion and we begin to
solicit testing, it's no longer reasonable to expect testers to
type or remember `BikeshedIntrinsicFrom`. The name degrades the
ease-of-reading of documentation, and the overall experience of
using compiler safe transmute.
Tentatively, we'll instead adopt `TransmuteFrom`.
This name seems to be the one most likely to be stabilized, after
discussion on Zulip [1]. We may want to revisit the ordering of
`Src` and `Dst` before stabilization, at which point we'd likely
consider `TransmuteInto` or `Transmute`.
[1] https://rust-lang.zulipchat.com/#narrow/stream/216762-project-safe-transmute/topic/What.20should.20.60BikeshedIntrinsicFrom.60.20be.20named.3F
Retroactively feature gate `ConstArgKind::Path`
This puts the lowering introduced by #125915 under a feature gate until we fix the regressions introduced by it. Alternative to whole sale reverting the PR since it didn't seem like a very clean revert and I think this is generally a step in the right direction and don't want to get stuck landing and reverting the PR over and over :)
cc #129137 ``@camelid,`` tests taken from there. beta is branching soon so I think it makes sense to not try and rush that fix through since it wont have much time to bake and if it has issues we can't simply revert it on beta.
Fixes#128016
This example triggers an assertion failure:
```
fn f() -> u32 {
#[cfg_eval] #[cfg(not(FALSE))] 0
}
```
The sequence of events:
- `configure_annotatable` calls `parse_expr_force_collect`, which calls
`collect_tokens`.
- Within that, we end up in `parse_expr_dot_or_call`, which again calls
`collect_tokens`.
- The return value of the `f` call is the expression `0`.
- This inner call collects tokens for `0` (parser range 10..11) and
creates a replacement covering `#[cfg(not(FALSE))] 0` (parser range
0..11).
- We return to the outer `collect_tokens` call. The return value of the
`f` call is *again* the expression `0`, again with the range 10..11,
but the replacement from earlier covers the range 0..11. The code
mistakenly assumes that any attributes from an inner `collect_tokens`
call fit entirely within the body of the result of an outer
`collect_tokens` call. So it adjusts the replacement parser range
0..11 to a node range by subtracting 10, resulting in -10..1. This is
an invalid range and triggers an assertion failure.
It's tricky to follow, but basically things get complicated when an AST
node is returned from an inner `collect_tokens` call and then returned
again from an outer `collect_token` node without being wrapped in any
kind of additional layer.
This commit changes `collect_tokens` to return early in some extra cases,
avoiding the construction of lazy tokens. In the example above, the
outer `collect_tokens` returns earlier because the `0` token already has
tokens and `self.capture_state.capturing` is `Capturing::No`. This early
return avoids the creation of the invalid range and the assertion
failure.
Fixes#129166. Note: these invalid ranges have been happening for a long
time. #128725 looks like it's at fault only because it introduced the
assertion that catches the invalid ranges.
Return correct HirId when finding body owner in diagnostics
Fixes#129145Fixes#128810
r? ```@compiler-errors```
```rust
fn generic<const N: u32>() {}
trait Collate<const A: u32> {
type Pass;
fn collate(self) -> Self::Pass;
}
impl<const B: u32> Collate<B> for i32 {
type Pass = ();
fn collate(self) -> Self::Pass {
generic::<{ true }>()
//~^ ERROR: mismatched types
}
}
```
When type checking the `{ true }` anon const we would error with a type mismatch. This then results in diagnostics code attempting to check whether its due to a type mismatch with the return type. That logic was implemented by walking up the hir until we reached the body owner, except instead of using the `enclosing_body_owner` function it special cased various hir nodes incorrectly resulting in us walking out of the anon const and stopping at `fn collate` instead.
This then resulted in diagnostics logic inside of the anon consts `ParamEnv` attempting to do trait solving involving the `<i32 as Collate<B>>::Pass` type which ICEs because it is in the wrong environment.
I have rewritten this function to just walk up until it hits the `enclosing_body_owner` and made some other changes since I found this pretty hard to read/understand. Hopefully it's easier to understand now, it also makes it more obvious that this is not implemented in a very principled way and is definitely missing cases :)
Special-case alias ty during the delayed bug emission in `try_from_lit`
This PR tries to fix#116308.
A delayed bug in `try_from_lit` will not be emitted so that the compiler will not ICE when it sees the pair `(ast::LitKind::Int, ty::TyKind::Alias)` in `lit_to_const` (called from `try_from_lit`).
This PR is related to an unstable feature `adt_const_params` (#95174).
r? ``@BoxyUwU``
WF-check struct field types at construction site
Fixes#126272.
Fixes#127299.
Rustc of course already WF-checked the field types at the definition
site, but for error tainting of consts to work properly, there needs to
be an error emitted at the use site. Previously, with no use-site error,
we proceeded with CTFE and ran into ICEs since we are running code with
type errors.
Emitting use-site errors also brings struct-like constructors more in
line with fn-like constructors since they already emit use-site errors
for WF issues.
r? `@BoxyUwU`
Make `validate_mir` ensure the final MIR for all bodies
A lot of the crashes tests use `-Zpolymorphize` or `-Zdump-mir` for their side effect of computing the `optimized_mir` for all bodies, which will uncover bugs with late MIR passes like the inliner. I don't like having all these tests depend on `-Zpolymorphize` (or other hacky ways) for no reason, so this PR extends the `-Zvalidate-mir` flag to ensure `optimized_mir`/`mir_for_ctfe` for all body owners during the analysis phase.
Two thoughts:
1. This could be moved later in the compilation pipeline I guess? I don't really think it matters, though.
1. This could alternatively be expressed using a new flag, though I don't necessarily see much value in separating these.
For example, #128171 could have used this flag, in the `tests/ui/polymorphization/inline-incorrect-early-bound.rs`.
r? mir
Rustc of course already WF-checked the field types at the definition
site, but for error tainting of consts to work properly, there needs to
be an error emitted at the use site. Previously, with no use-site error,
we proceeded with CTFE and ran into ICEs since we are running code with
type errors.
Emitting use-site errors also brings struct-like constructors more in
line with fn-like constructors since they already emit use-site errors
for WF issues.
Update to LLVM 19
The LLVM 19.1.0 final release is planned for Sep 3rd. The rustc 1.82 stable release will be on Oct 17th.
The unstable MC/DC coverage support is temporarily broken by this update. It will be restored by https://github.com/rust-lang/rust/pull/126733. The implementation changed substantially in LLVM 19, and there are no plans to support both the LLVM 18 and LLVM 19 implementation at the same time.
Compatibility note for wasm:
> WebAssembly target support for the `multivalue` target feature has changed when upgrading to LLVM 19. Support for generating functions with multiple returns no longer works and `-Ctarget-feature=+multivalue` has a different meaning than it did in LLVM 18 and prior. The WebAssembly target features `multivalue` and `reference-types` are now both enabled by default, but generated code is not affected by default. These features being enabled are encoded in the `target_features` custom section and may affect downstream tooling such as `wasm-opt` consuming the module, but the actual generated WebAssembly will continue to not use either `multivalue` or `reference-types` by default. There is no longer any supported means to generate a module that has a function with multiple returns.
Related changes:
* https://github.com/rust-lang/rust/pull/127605
* https://github.com/rust-lang/rust/pull/127613
* https://github.com/rust-lang/rust/pull/127654
* https://github.com/rust-lang/rust/pull/128141
* https://github.com/llvm/llvm-project/pull/98933
Fixes https://github.com/rust-lang/rust/issues/121444.
Fixes https://github.com/rust-lang/rust/issues/128212.
Mark `Parser::eat`/`check` methods as `#[must_use]`
These methods return a `bool`, but we probably should either use these values or explicitly throw them away (e.g. when we just want to unconditionally eat a token if it exists).
I changed a few places from `eat` to `expect`, but otherwise I tried to leave a comment explaining why the `eat` was okay.
This also adds a test for the `pattern_type!` macro, which used to silently accept a missing `is` token.
Detect non-lifetime binder params shadowing item params
We should check that `for<T>` shadows `T` from an item in the same way that `for<'a>` shadows `'a` from an item.
r? ``@petrochenkov`` since you're familiar w the nuances of rib kinds
Matthias Krüger