This commit changes the way we treat the stability of attributes in the
`#[diagnostic]` namespace. Instead of relaying on ad-hoc checks to
ensure at call side that a certain attribute is really usable at that
location it centralises the logic to one place. For diagnostic
attributes comming from other crates it just skips serializing
attributes that are not stable and that do not have the corresponding
feature enabled. For attributes from the current crate we can just use
the feature information provided by `TyCtx`.
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+).
coverage: Count await when the Future is immediately ready
Currently `await` is only counted towards coverage if the containing
function is suspended and resumed at least once. This is because it
expands to code which contains a branch on the discriminant of `Poll`.
By treating it like a branching macro (e.g. `assert!`), these
implementation details will be hidden from the coverage results.
I added a test to ensure the fix works in simple cases, but the heuristic of picking only the first await-related covspan might be unreliable. I plan on testing more thoroughly with a real codebase over the next couple of weeks.
closes#98712
Make `Ty::boxed_ty` return an `Option`
Looks like a good place to use Rust's type system.
---
Most of 4ac7bcbaad/compiler/rustc_middle/src/ty/sty.rs (L971-L1963) looks like it could be moved to `TyKind` (then I guess `Ty` should be made to deref to `TyKind`).
Check WF of source type's signature on fn pointer cast
This PR patches the implied bounds holes slightly for #129005, #25860.
Like most implied bounds related unsoundness fixes, this isn't complete w.r.t. higher-ranked function signatures, but I believe it implements a pretty good heuristic for now.
### What does this do?
This PR makes a partial patch for a soundness hole in a `FnDef` -> `FnPtr` "reifying" pointer cast where we were never checking that the signature we are casting *from* is actually well-formed. Because of this, and because `FnDef` doesn't require its signature to be well-formed (just its predicates must hold), we are essentially allowed to "cast away" implied bounds that are assumed within the body of the `FnDef`:
```
fn foo<'a, 'b, T>(_: &'a &'b (), v: &'b T) -> &'a T { v }
fn bad<'short, T>(x: &'short T) -> &'static T {
let f: fn(_, &'short T) -> &'static T = foo;
f(&&(), x)
}
```
In this example, subtyping ends up casting the `_` type (which should be `&'static &'short ()`) to some other type that no longer serves as a "witness" to the lifetime relationship `'short: 'static` which would otherwise be required for this call to be WF. This happens regardless of if `foo`'s lifetimes are early- or late-bound.
This PR implements two checks:
1. We check that the signature of the `FnDef` is well-formed *before* casting it. This ensures that there is at least one point in the MIR where we ensure that the `FnDef`'s implied bounds are actually satisfied by the caller.
2. Implements a special case where if we're casting from a higher-ranked `FnDef` to a non-higher-ranked, we instantiate the binder of the `FnDef` with *infer vars* and ensure that it is a supertype of the target of the cast.
The (2.) is necessary to validate that these pointer casts are valid for higher-ranked `FnDef`. Otherwise, the example above would still pass even if `help`'s `'a` lifetime were late-bound.
### Further work
The WF checks for function calls are scattered all over the MIR. We check the WF of args in call terminators, we check the WF of `FnDef` when we create a `const` operand referencing it, and we check the WF of the return type in #115538, to name a few.
One way to make this a bit cleaner is to simply extend #115538 to always check that the signature is WF for `FnDef` types. I may do this as a follow-up, but I wanted to keep this simple since this leads to some pretty bad NLL diagnostics regressions, and AFAICT this solution is *complete enough*.
### Crater triage
Done here: https://github.com/rust-lang/rust/pull/129021#issuecomment-2297702647
r? lcnr
Currently `await` is only counted towards coverage if the containing
function is suspended and resumed at least once. This is because it
expands to code which contains a branch on the discriminant of `Poll`.
By treating it like a branching macro (e.g. `assert!`), these
implementation details will be hidden from the coverage results.
Don't emit `expect`/`assume` in opt-level=0
LLVM does not make use of expect/assume calls in `opt-level=0`, so we can simplify IR by not emitting them in this case.
explain why Rvalue::Len still exists
I just spent a bit of time trying to remove this until I realized why that's non-trivial. Let's document that for the next person. :)
fix ICE when `asm_const` and `const_refs_to_static` are combined
fixes https://github.com/rust-lang/rust/issues/129462fixes#126896fixes#124164
I think this is a case that was missed in the fix for https://github.com/rust-lang/rust/pull/125558, which inserts a type error in the case of an invalid (that is, non-integer) type being passed to an asm `const` operand.
I'm not 100% sure that `span_mirbug_and_err` is the right macro here, but it is used earlier with `builtin_deref` and seems to do the trick.
r? ``@lcnr``
Add an internal lint that warns when accessing untracked data
Some methods access data that is not tracked by the query system and should be used with caution. As suggested in https://github.com/rust-lang/rust/pull/128815#issuecomment-2275488683, in this PR I propose a lint (modeled on the `potential_query_instability` lint) that warns when using some specially-annotatted functions.
I can't tell myself if this lint would be that useful, compared to renaming `Steal::is_stolen` to `is_stolen_untracked`. This would depend on whether there are other functions we'd want to lint like this. So far it seems they're called `*_untracked`, which may be clear enough.
r? ``@oli-obk``
Rename dump of coroutine by-move-body to be more consistent, fix ICE in dump_mir
First, we add a missing match for `DefKind::SyntheticCoroutineBody` in `dump_mir`. Fixes#129703. The second commit (directly below) serves as a test.
Second, we reorder the `dump_mir` in `coroutine_by_move_body_def_id` to be *after* we adjust the body source, and change the disambiguator so it reads more like any other MIR body. This also serves as a test for the ICE, since we're dumping the MIR of a body with `DefKind::SyntheticCoroutineBody`.
Third, we change the parenting of the synthetic MIR body to have the *coroutine-closure* (i.e. async closure) as its parent, so we don't have long strings of `{closure#0}-{closure#0}-{closure#0}`.
try-job: test-various
`impl_trait_overcaptures`: Don't worry about uncaptured contravariant lifetimes if they outlive a captured lifetime
**NOTE:** Review only the first commit carefully. The second one is just moving stuff around, so you can turn whitespace off for that one.
This PR relaxes the `impl_trait_overcaptures` lint to not fire in cases like:
```rust
struct Ctxt<'tcx>(&'tcx ());
impl<'tcx> Ctxt<'tcx> {
fn compute(&self) -> impl Sized + '_ { }
}
```
Specifically, the lint will not fire if **all** overcaptured regions (i.e. those which will be captured in edition 2024, which are not captured today) **satisfy**:
* The region is contravariant (or bivariant) in the function signature
* The region outlives some other region which is captured by the opaque
### The idea behind this
Why is this OK? My reasoning is that since the region is contravariant in the function signature, we know that it can be shortened arbitrarily at the call site. And specifically, we know it can be shortened to be equal to one of the regions that it outlives -- that's why we need to prove that it outlives some other region that *is* captured.
We could technically relax this further, but there would be (IMO somewhat easy) cases to make this a false negative in real code. For example, if the region is invariant, then we can hit issues like:
```rust
struct Ctxt<'tcx>(&'tcx mut &'tcx mut ());
impl<'tcx> Ctxt<'tcx> {
fn compute(&self) -> impl Sized + use<'_, 'tcx> { }
// We use `use<'_, 'tcx>` to show what happens in edition 2024
}
fn test<'a, 'b>(x: &'a Ctxt<'b>, y: &'a Ctxt<'a>) {
let results = [x.compute(), y.compute()];
//~^ ERROR lifetime may not live long enough
// Since both opaques now capture `'tcx`, this enforces that `'a == 'b`.
}
```
### Is this actually totally fine?
There's one case where users might still hit issues, and it's if we turbofish lifetimes directly:
```rust
struct Ctxt<'tcx>(&'tcx ());
impl<'tcx> Ctxt<'tcx> {
fn compute(&self) -> impl Sized + use<'_, 'tcx> { }
}
fn test<'a, 'b>(x: &'a Ctxt<'b>, y: &'a Ctxt<'a>) {
let results = [Ctxt::<'b>::compute(x), Ctxt::<'a>::compute(y)];
//~^ ERROR lifetime may not live long enough
// Since both opaques now capture `'tcx`, this enforces that `'a == 'b`.
// Note that we don't shorten `'b` to `'a` since we turbofished it.
}
```
### Well... we should still warn?
I kinda don't care about this case, though I guess we could possibly downgrade the lint to something like `IMPL_TRAIT_OVERCAPTURES_STRICT` instead of suppressing it altogether. Thoughts? If we were to do this, then I'd probably also opt to include the invariant case in `IMPL_TRAIT_OVERCAPTURES_STRICT` and move it out of `IMPL_TRAIT_OVERCAPTURES`.
- Merge minimum OS version list into one function (makes it easier to
see the logic in it).
- Parse patch deployment target versions.
- Consistently specify deployment target in LLVM target (previously
omitted on `aarch64-apple-watchos`).
Update docs of `missing_abi` lint
The lint docs still said that function ABIs other than "C" have not been added yet.
`@rustbot` labels: +A-docs +A-lint
Arbitrary self types v2: pointers feature gate.
The main `arbitrary_self_types` feature gate will shortly be reused for a new version of arbitrary self types which we are amending per [this RFC](https://github.com/rust-lang/rfcs/blob/master/text/3519-arbitrary-self-types-v2.md). The main amendments are:
* _do_ support `self` types which can't safely implement `Deref`
* do _not_ support generic `self` types
* do _not_ support raw pointers as `self` types.
This PR relates to the last of those bullet points: this strips pointer support from the current `arbitrary_self_types` feature. We expect this to cause some amount of breakage for crates using this unstable feature to allow raw pointer self types. If that's the case, we want to know about it, and we want crate authors to know of the upcoming changes.
For now, this can be resolved by adding the new
`arbitrary_self_types_pointers` feature to such crates. If we determine that use of raw pointers as self types is common, then we may maintain that as an unstable feature even if we come to stabilize the rest of the `arbitrary_self_types` support in future. If we don't hear that this PR is causing breakage, then perhaps we don't need it at all, even behind an unstable feature gate.
[Tracking issue](https://github.com/rust-lang/rust/issues/44874)
This is [step 4 of the plan outlined here](https://github.com/rust-lang/rust/issues/44874#issuecomment-2122179688)
Add target support for RTEMS Arm
# `armv7-rtems-eabihf`
This PR adds a new target for the RTEMS RTOS. To get things started it focuses on Xilinx/AMD Zynq-based targets, but in theory it should also support other armv7-based board support packages in the future.
Given that RTEMS has support for many POSIX functions it is mostly enabling corresponding unix features for the new target.
I also previously started a PR in libc (https://github.com/rust-lang/libc/pull/3561) to add the needed OS specific C-bindings and was told that a PR in this repo is needed first. I will update the PR to the newest version after approval here.
I will probably also need to change one line in the backtrace repo.
Current status is that I could compile rustc for the new target locally (with the updated libc and backtrace) and could compile binaries, link, and execute a simple "Hello World" RTEMS application for the target hardware.
> A proposed target or target-specific patch that substantially changes code shared with other targets (not just target-specific code) must be reviewed and approved by the appropriate team for that shared code before acceptance.
There should be no breaking changes for existing targets. Main changes are adding corresponding `cfg` switches for the RTEMS OS and adding the C binding in libc.
# Tier 3 target policy
> - A tier 3 target must have a designated developer or developers (the "target maintainers") on record to be CCed when issues arise regarding the target. (The mechanism to track and CC such developers may evolve over time.)
I will do the maintenance (for now) further members of the RTEMS community will most likely join once the first steps have been done.
> - Targets must use naming consistent with any existing targets; for instance, a target for the same CPU or OS as an existing Rust target should use the same name for that CPU or OS. Targets should normally use the same names and naming conventions as used elsewhere in the broader ecosystem beyond Rust (such as in other toolchains), unless they have a very good reason to diverge. Changing the name of a target can be highly disruptive, especially once the target reaches a higher tier, so getting the name right is important even for a tier 3 target.
> - Target names should not introduce undue confusion or ambiguity unless absolutely necessary to maintain ecosystem compatibility. For example, if the name of the target makes people extremely likely to form incorrect beliefs about what it targets, the name should be changed or augmented to disambiguate it.
> - If possible, use only letters, numbers, dashes and underscores for the name. Periods (`.`) are known to cause issues in Cargo.
The proposed triple is `armv7-rtems-eabihf`
> - Tier 3 targets may have unusual requirements to build or use, but must not create legal issues or impose onerous legal terms for the Rust project or for Rust developers or users.
> - The target must not introduce license incompatibilities.
> - Anything added to the Rust repository must be under the standard Rust license (`MIT OR Apache-2.0`).
> - The target must not cause the Rust tools or libraries built for any other host (even when supporting cross-compilation to the target) to depend on any new dependency less permissive than the Rust licensing policy. This applies whether the dependency is a Rust crate that would require adding new license exceptions (as specified by the `tidy` tool in the rust-lang/rust repository), or whether the dependency is a native library or binary. In other words, the introduction of the target must not cause a user installing or running a version of Rust or the Rust tools to be subject to any new license requirements.
> - Compiling, linking, and emitting functional binaries, libraries, or other code for the target (whether hosted on the target itself or cross-compiling from another target) must not depend on proprietary (non-FOSS) libraries. Host tools built for the target itself may depend on the ordinary runtime libraries supplied by the platform and commonly used by other applications built for the target, but those libraries must not be required for code generation for the target; cross-compilation to the target must not require such libraries at all. For instance, `rustc` built for the target may depend on a common proprietary C runtime library or console output library, but must not depend on a proprietary code generation library or code optimization library. Rust's license permits such combinations, but the Rust project has no interest in maintaining such combinations within the scope of Rust itself, even at tier 3.
> - "onerous" here is an intentionally subjective term. At a minimum, "onerous" legal/licensing terms include but are _not_ limited to: non-disclosure requirements, non-compete requirements, contributor license agreements (CLAs) or equivalent, "non-commercial"/"research-only"/etc terms, requirements conditional on the employer or employment of any particular Rust developers, revocable terms, any requirements that create liability for the Rust project or its developers or users, or any requirements that adversely affect the livelihood or prospects of the Rust project or its developers or users.
The tools consists of the cross-compiler toolchain (gcc-based). The RTEMS kernel (BSD license) and parts of the driver stack of FreeBSD (BSD license). All tools are FOSS and publicly available here: https://gitlab.rtems.org/rtems
There are also no new features or dependencies introduced to the Rust code.
> - Neither this policy nor any decisions made regarding targets shall create any binding agreement or estoppel by any party. If any member of an approving Rust team serves as one of the maintainers of a target, or has any legal or employment requirement (explicit or implicit) that might affect their decisions regarding a target, they must recuse themselves from any approval decisions regarding the target's tier status, though they may otherwise participate in discussions.
N/A to me. I am not a reviewer nor Rust team member.
> - Tier 3 targets should attempt to implement as much of the standard libraries as possible and appropriate (`core` for most targets, `alloc` for targets that can support dynamic memory allocation, `std` for targets with an operating system or equivalent layer of system-provided functionality), but may leave some code unimplemented (either unavailable or stubbed out as appropriate), whether because the target makes it impossible to implement or challenging to implement. The authors of pull requests are not obligated to avoid calling any portions of the standard library on the basis of a tier 3 target not implementing those portions.
`core` and `std` compile. Some advanced features of the `std` lib might not work yet. However, the goal of this tier 3 target it to make it easier for other people to build and run test applications to better identify the unsupported features and work towards enabling them.
> - The target must provide documentation for the Rust community explaining how to build for the target, using cross-compilation if possible. If the target supports running binaries, or running tests (even if they do not pass), the documentation must explain how to run such binaries or tests for the target, using emulation if possible or dedicated hardware if necessary.
Building is described in platform support doc. Running simple unit tests works. Running the test suite of the stdlib is currently not that easy. Trying to work towards that after the this target has been added to the nightly.
> - Tier 3 targets must not impose burden on the authors of pull requests, or other developers in the community, to maintain the target. In particular, do not post comments (automated or manual) on a PR that derail or suggest a block on the PR based on a tier 3 target. Do not send automated messages or notifications (via any medium, including via ````@`)``` to a PR author or others involved with a PR regarding a tier 3 target, unless they have opted into such messages.
Understood.
> - Backlinks such as those generated by the issue/PR tracker when linking to an issue or PR are not considered a violation of this policy, within reason. However, such messages (even on a separate repository) must not generate notifications to anyone involved with a PR who has not requested such notifications.
Ok
> - Patches adding or updating tier 3 targets must not break any existing tier 2 or tier 1 target, and must not knowingly break another tier 3 target without approval of either the compiler team or the maintainers of the other tier 3 target.
> - In particular, this may come up when working on closely related targets, such as variations of the same architecture with different features. Avoid introducing unconditional uses of features that another variation of the target may not have; use conditional compilation or runtime detection, as appropriate, to let each target run code supported by that target.
I think, I didn't add any breaking changes for any existing targets (see the comment regarding features above).
> - Tier 3 targets must be able to produce assembly using at least one of rustc's supported backends from any host target.
Can produce assembly code via the llvm backend (tested on Linux).
>
> If a tier 3 target stops meeting these requirements, or the target maintainers no longer have interest or time, or the target shows no signs of activity and has not built for some time, or removing the target would improve the quality of the Rust codebase, we may post a PR to remove it; any such PR will be CCed to the target maintainers (and potentially other people who have previously worked on the target), to check potential interest in improving the situation.GIAt this tier, the Rust project provides no official support for a target, so we place minimal requirements on the introduction of targets.
Understood.
r? compiler-team
enable -Zrandomize-layout in debug CI builds
This builds rustc/libs/tools with `-Zrandomize-layout` on *-debug CI runners.
Only a handful of tests and asserts break with that enabled, which is promising. One test was fixable, the rest is dealt with by disabling them through new cargo features or compiletest directives.
The config.toml flag `rust.randomize-layout` defaults to false, so it has to be explicitly enabled for now.
rustc_driver_impl: remove some old dead logic
This got added in 5013952e4a, before `cfg(target_feature)` was stable. It should not be needed any more ever since `cfg(target_feature)` is stable.
Move `SanityCheck` and `MirPass`
They are currently in `rustc_middle`. This PR moves them to `rustc_mir_transform`, which makes more sense.
r? ``@cjgillot``
do not attempt to prove unknowable goals
In case a goal is unknowable, we previously still checked all other possible ways to prove this goal, even though its final result is already guaranteed to be ambiguous. By ignoring all other candidates in that case we can avoid a lot of unnecessary work, fixing the performance regression in typenum found in #121848.
This is already the behavior in the old solver. This could in theory cause future-compatability issues as considering fewer goals unknowable may end up causing performance regressions/hangs. I am quite confident that this will not be an issue.
r? ``@compiler-errors``
update comment regarding TargetOptions.features
The claim that `-Ctarget-features` cannot disable these features set in the target spec is definitely wrong -- I tried it for `x86_64-pc-windows-gnu`, which enables SSE3 that way. Building with `-Ctarget-feature=-sse3` works fine, and `cfg!(target_feature = "sse3")` is `false` in that build.
There are also some indications that these are actually intended to be overwritten:
3b14526cea/compiler/rustc_target/src/spec/targets/i686_unknown_uefi.rs (L22-L23)84ac80f192/compiler/rustc_target/src/spec/targets/x86_64h_apple_darwin.rs (L18-L23)
So... let's update the comment to match reality, I guess?
The claim that they overwrite `-Ctarget-cpu` is based on
- for `native`, the comment in the apple target spec quoted above
- for other CPU strings, the assumption that `LLVMRustCreateTargetMachine` will apply these features after doing whatever the base CPU model does. I am not sure how to check that, I hope some LLVM backend people can chime in. :)
Non-exhaustive structs may be empty
This is a follow-up to a discrepancy noticed in https://github.com/rust-lang/rust/pull/122792: today, the following struct is considered inhabited (non-empty) outside its defining crate:
```rust
#[non_exhaustive]
pub struct UninhabitedStruct {
pub never: !,
// other fields
}
```
`#[non_exhaustive]` on a struct should mean that adding fields to it isn't a breaking change. There is no way that adding fields to this struct could make it non-empty since the `never` field must stay and is inconstructible. I suspect this was implemented this way due to confusion with `#[non_exhaustive]` enums, which indeed should be considered non-empty outside their defining crate.
I propose that we consider such a struct uninhabited (empty), just like it would be without the `#[non_exhaustive]` annotation.
Code that doesn't pass today and will pass after this:
```rust
// In a different crate
fn empty_match_on_empty_struct<T>(x: UninhabitedStruct) -> T {
match x {}
}
```
This is not a breaking change.
r? ``@compiler-errors``
Don't Suggest Labeling `const` and `unsafe` Blocks
Fixes#128604
Previously, both anonymous constant blocks (E.g. The labeled block
inside `['_'; 'block: { break 'block 1 + 2; }]`) and inline const
blocks (E.g. `const { ... }`) were considered to be the same
kind of blocks. This caused the compiler to incorrectly suggest
labeling both the blocks when only anonymous constant blocks can be
labeled.
This PR adds an other enum variant to `Context` so that both the
blocks can be handled appropriately.
Also, adds some doc comments and removes unnecessary `&mut` in a
couple of places.
Suggest `impl Trait` for References to Bare Trait in Function Header
Fixes#125139
This PR suggests `impl Trait` when `&Trait` is found as a function parameter type or return type. This makes use of existing diagnostics by adding `peel_refs()` when checking for type equality.
Additionaly, it makes a few other improvements:
1. Checks if functions inside impl blocks have bare trait in their headers.
2. Introduces a trait `NextLifetimeParamName` similar to the existing `NextTypeParamName` for suggesting a lifetime name. Also, abstracts out the common logic between the two trait impls.
### Related Issues
I ran into a bunch of related diagnostic issues but couldn't fix them within the scope of this PR. So, I have created the following issues:
1. [Misleading Suggestion when Returning a Reference to a Bare Trait from a Function](https://github.com/rust-lang/rust/issues/127689)
2. [Verbose Error When a Function Takes a Bare Trait as Parameter](https://github.com/rust-lang/rust/issues/127690)
3. [Incorrect Suggestion when Returning a Bare Trait from a Function](https://github.com/rust-lang/rust/issues/127691)
r? ```@estebank``` since you implemented #119148
Because that's now the only crate that uses it.
Moving stuff out of `rustc_middle` is always welcome.
I chose to use `impl crate::MirPass`/`impl crate::MirLint` (with
explicit `crate::`) everywhere because that's the only mention of
`MirPass`/`MirLint` used in all of these files. (Prior to this change,
`MirPass` was mostly imported via `use rustc_middle::mir::*` items.)
The actual implementation remains in `rustc_mir_dataflow`, but this
commit moves the `MirPass` impl to `rustc_mir_transform` and changes it
to a `MirLint` (fixing a `FIXME` comment).
(I originally tried moving the full implementation from
`rustc_mir_dataflow` but I had some trait problems with `HasMoveData`
and `RustcPeekAt` and `MaybeLiveLocals`. This commit was much smaller
and simpler, but still will allow some follow-up cleanups.)
Make decoding non-optional `LazyArray` panic if not set
Tables may be [defined](9649706ead/compiler/rustc_metadata/src/rmeta/mod.rs (L377)) as `optional:` or `defaulted:`. If optional, if we try to read a value from a key that was never encoded, we should panic. This has high value in ensuring correctness over a defaulted table, so the tradeoff is worth considering, since it signals the compiler has a buggy encode impl, rather than just defaulting to a value.
HOWEVER, `optional:` arrays were side-stepping this. So this PR fixes that, and makes `optional:` tables of `LazyArray` act like `LazyValue`, and panic if it's not assigned a value during encoding.
During this PR, I found that `deduced_param_attrs` has buggy (?? i think??) implementation where it will refuse to encode cross-crate `deduced_param_attrs` unless we're codegening, we're optimizing the library, and incremental is disabled. This seems incredibly wrong, but I don't want to fix it in this PR.
9649706ead/compiler/rustc_metadata/src/rmeta/encoder.rs (L1733-L1747)
This shrinks `compiler/rustc_codegen_gcc/Cargo.lock` quite a bit. The
only remaining dependencies in `compiler/rustc_codegen_gcc/Cargo.lock`
are `gccjit`, `lang_tester`, and `boml`, all of which aren't used in any
other compiler crates.
The commit also reorders and adds comments to the `extern crate` items
so they match those in miri.
Replace walk with visit so we dont skip outermost expr kind in def collector
This affects async closures with macros as their body expr. Fixes#129855.
r? ``@cjgillot`` or anyone else
Fix LLVM ABI NAME for riscv64imac-unknown-nuttx-elf
This patch fix https://github.com/rust-lang/rust/issues/129825
For the riscv64imac target, the LLVM ABI NAME should be "lp64", which is the default ABI if not specified for the riscv64imac target.
Deny imports of `rustc_type_ir::inherent` outside of type ir + new trait solver
We shouldn't encourage using `rustc_type_ir::inherent` outside of the new solver[^1], though this can happen by accident due to rust-analyzer, for example. See https://github.com/rust-lang/rust/pull/127537#discussion_r1733813842 for an example in practice.
r? fmease
[^1]: Unless we go the fully radical approach of always using these inherent methods everywhere in favor of inherent methods, which would be a major overhaul of the compiler, IMO. I don't really want to consider that possibility right now, tho.
