Port `run-make/issue-7349` to a codegen test
The test does not need to be a run-make test, it can use the codegen test infrastructure.
Also took the opportunity to rename the test to `no-redundant-item-monomorphization` so it's not just some opaque issue number.
Part of #121876.
Actually use the inferred `ClosureKind` from signature inference in coroutine-closures
A follow-up to https://github.com/rust-lang/rust/pull/123349, which fixes another subtle bug: We were not taking into account the async closure kind we infer during closure signature inference.
When I pass a closure directly to an arg like `fn(x: impl async FnOnce())`, that should have the side-effect of artificially restricting the kind of the async closure to `ClosureKind::FnOnce`. We weren't doing this -- that's a quick fix; however, it uncovers a second, more subtle bug with the way that `move`, async closures, and `FnOnce` interact.
Specifically, when we have an async closure like:
```
let x = Struct;
let c = infer_as_fnonce(async move || {
println!("{x:?}");
}
```
The outer closure captures `x` by move, but the inner coroutine still immutably borrows `x` from the outer closure. Since we've forced the closure to by `async FnOnce()`, we can't actually *do* a self borrow, since the signature of `AsyncFnOnce::call_once` doesn't have a borrowed lifetime. This means that all `async move` closures that are constrained to `FnOnce` will fail borrowck.
We can fix that by detecting this case specifically, and making the *inner* async closure `move` as well. This is always beneficial to closure analysis, since if we have an `async FnOnce()` that's `move`, there's no reason to ever borrow anything, so `move` isn't artificially restrictive.
Match ergonomics: implement "`&`pat everywhere"
Implements the eat-two-layers (feature gate `and_pat_everywhere`, all editions) ~and the eat-one-layer (feature gate `and_eat_one_layer_2024`, edition 2024 only, takes priority on that edition when both feature gates are active)~ (EDIT: will be done in later PR) semantics.
cc #123076
r? ``@Nadrieril``
``@rustbot`` label A-patterns A-edition-2024
Restore typeid_for_instance default behavior of performing self type
erasure, since it's the most common case and what it does most of the
time. Using concrete self (or not performing self type erasure) is for
assigning a secondary type id, and secondary type ids are only assigned
when they're unique and to methods, and also are only tested for when
methods are used as function pointers.
CFI: Add test for `call_once` addr taken
One of the proposed ways to reduce the non-passed argument erasure would cause this test to fail. Adding this now ensures that any attempt to reduce non-passed argument erasure won't make the same mistake.
r? `@compiler-errors`
cc `@rcvalle`
Default to light theme if JS is enabled but not working
It doesn't [fix] #123399 but it allows to reduce the problem:
* if JS is completely disabled, then `noscript.css` will be applied
* if JS failed for any reason, then the light theme will be applied (because `noscript.css` won't be applied)
r? `@notriddle`
change `NormalizesTo` to fully structurally normalize
notes in https://hackmd.io/wZ016dE4QKGIhrOnHLlThQ
need to also update the dev-guide once this PR lands. in short, the setup is now as follows:
`normalizes-to` internally implements one step normalization, applying that normalization to the `goal.predicate.term` causes the projected term to get recursively normalized. With this `normalizes-to` normalizes until the projected term is rigid, meaning that we normalize as many steps necessary, but at least 1.
To handle rigid aliases, we add another candidate only if the 1 to inf step normalization failed. With this `normalizes-to` is now full structural normalization. We can now change `AliasRelate` to simply emit `normalizes-to` goals for the rhs and lhs.
This avoids the concerns from https://github.com/rust-lang/trait-system-refactor-initiative/issues/103 and generally feels cleaner
One of the proposed ways to reduce the non-passed argument erasure would
cause this test to fail. Adding this now ensures that any attempt to
reduce non-passed argument erasure won't make the same mistake.
some smaller DefiningOpaqueTypes::No -> Yes switches
r? `@compiler-errors`
These are some easy cases, so let's get them out of the way first.
I added tests exercising the specialization code paths that I believe weren't tested so far.
follow-up to https://github.com/rust-lang/rust/pull/117348
Only inspect user-written predicates for privacy concerns
fixes#123288
Previously we looked at the elaborated predicates, which, due to adding various bounds on fields, end up requiring trivially true bounds. But these bounds can contain private types, which the privacy visitor then found and errored about.
match lowering: make false edges more precise
When lowering match expressions, we add false edges to hide details of the lowering from borrowck. Morally we pretend we're testing the patterns (and guards) one after the other in order. See the tests for examples. Problem is, the way we implement this today is too coarse for deref patterns.
In deref patterns, a pattern like `deref [1, x]` matches on a `Vec` by creating a temporary to store the output of the call to `deref()` and then uses that to continue matching. Here the pattern has a binding, which we set up after the pre-binding block. Problem is, currently the false edges tell borrowck that the pre-binding block can be reached from a previous arm as well, so the `deref()` temporary may not be initialized. This triggers an error when we try to use the binding `x`.
We could call `deref()` a second time, but this opens the door to soundness issues if the deref impl is weird. Instead in this PR I rework false edges a little bit.
What we need from false edges is a (fake) path from each candidate to the next, specifically from candidate C's pre-binding block to next candidate D's pre-binding block. Today, we link the pre-binding blocks directly. In this PR, I link them indirectly by choosing an earlier node on D's success path. Specifically, I choose the earliest block on D's success path that doesn't make a loop (if I chose e.g. the start block of the whole match (which is on the success path of all candidates), that would make a loop). This turns out to be rather straightforward to implement.
r? `@matthewjasper` if you have the bandwidth, otherwise let me know
check `FnDef` return type for WF
better version of #106807, fixes#84533 (mostly). It's not perfect given that we still ignore WF requirements involving bound regions but I wasn't able to quickly write an example, so even if theoretically exploitable, it should be far harder to trigger.
This is strictly more restrictive than checking the return type for WF as part of the builtin `FnDef: FnOnce` impl (#106807) and moving to this approach in the future will not break any code.
~~It also agrees with my theoretical view of how this should behave~~
r? types
CFI: Support function pointers for trait methods
Adds support for both CFI and KCFI for function pointers to trait methods by attaching both concrete and abstract types to functions.
KCFI does this through generation of a `ReifyShim` on any function pointer for a method that could go into a vtable, and keeping this separate from `ReifyShim`s that are *intended* for vtable us by setting a `ReifyReason` on them.
CFI does this by setting both the concrete and abstract type on every instance.
This should land after #123024 or a similar PR, as it diverges the implementation of CFI vs KCFI.
r? `@compiler-errors`
instantiate higher ranked goals outside of candidate selection
This PR modifies `evaluate` to more eagerly instantiate higher-ranked goals, preventing the `leak_check` during candidate selection from detecting placeholder errors involving that binder.
For a general background regarding higher-ranked region solving and the leak check, see https://hackmd.io/qd9Wp03cQVy06yOLnro2Kg.
> The first is something called the **leak check**. You can think of it as a "quick and dirty" approximation for the region check, which will come later. The leak check detects some kinds of errors early, essentially deciding between "this set of outlives constraints are guaranteed to result in an error eventually" or "this set of outlives constraints may be solvable".
## The ideal future
We would like to end up with the following idealized design to handle universal binders:
```rust
fn enter_forall<'tcx, T, R>(
forall: Binder<'tcx, T>,
f: impl FnOnce(T) -> R,
) -> R {
let new_universe = infcx.increment_universe_index();
let value = instantiate_binder_with_placeholders_in(new_universe, forall);
let result = f(value);
eagerly_handle_higher_ranked_region_constraints_in(new_universe);
infcx.decrement_universe_index();
assert!(!result.has_placeholders_in_or_above(new_universe));
result
}
```
That is, when universally instantiating a binder, anything using the placeholders has to happen inside of a limited scope (the closure `f`). After this closure has completed, all constraints involving placeholders are known.
We then handle any *external constraints* which name these placeholders. We destructure `TypeOutlives` constraints involving placeholders and eagerly handle any region constraints involving these placeholders. We do not return anything mentioning the placeholders created inside of this function to the caller.
Being able to eagerly handle *all* region constraints involving placeholders will be difficult due to complex `TypeOutlives` constraints, involving inference variables or alias types, and higher ranked implied bounds. The exact issues and possible solutions are out of scope of this FCP.
#### How does the leak check fit into this
The `leak_check` is an underapproximation of `eagerly_handle_higher_ranked_region_constraints_in`. It detects some kinds of errors involving placeholders from `new_universe`, but not all of them.
It only looks at region outlives constraints, ignoring `TypeOutlives`, and checks whether one of the following two conditions are met for **placeholders in or above `new_universe`**, in which case it results in an error:
- `'!p1: '!p2` a placeholder `'!p2` outlives a different placeholder `'!p1`
- `'!p1: '?2` an inference variable `'?2` outlives a placeholder `'!p1` *which it cannot name*
It does not handle all higher ranked region constraints, so we still return constraints involving placeholders from `new_universe` which are then (re)checked by `lexical_region_resolve` or MIR borrowck.
As we check higher ranked constraints in the full regionck anyways, the `leak_check` is not soundness critical. It's current only purpose is to move some higher ranked region errors earlier, enabling it to guide type inference and trait solving. Adding additional uses of the `leak_check` in the future would only strengthen inference and is therefore not breaking.
## Where do we use currently use the leak check
The `leak_check` is currently used in two places:
Coherence does not use a proper regionck, only relying on the `leak_check` called [at the end of the implicit negative overlap check](8b94152af6/compiler/rustc_trait_selection/src/traits/coherence.rs (L235-L238)). During coherence all parameters are instantiated with inference variables, so the only possible region errors are higher-ranked. We currently also sometimes make guesses when destructuring `TypeOutlives` constraints which can theoretically result in incorrect errors. This could result in overlapping impls.
We also use the `leak_check` [at the end of `fn evaluation_probe`](8b94152af6/compiler/rustc_trait_selection/src/traits/select/mod.rs (L607-L610)). This function is used during candidate assembly for `Trait` goals. Most notably we use [inside of `evaluate_candidate` during winnowing](0e4243538b/compiler/rustc_trait_selection/src/traits/select/mod.rs (L491-L502)). Conceptionally, it is as if we compute each candidate in a separate `enter_forall`.
## The current use in `fn evaluation_probe` is undesirable
Because we only instantiate a higher-ranked goal once inside of `fn evaluation_probe`, errors involving placeholders from that binder can impact selection. This results in inconsistent behavior ([playground](
*[playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=dac60ebdd517201788899ffa77364831)*)):
```rust
trait Leak<'a> {}
impl Leak<'_> for Box<u32> {}
impl Leak<'static> for Box<u16> {}
fn impls_leak<T: for<'a> Leak<'a>>() {}
trait IndirectLeak<'a> {}
impl<'a, T: Leak<'a>> IndirectLeak<'a> for T {}
fn impls_indirect_leak<T: for<'a> IndirectLeak<'a>>() {}
fn main() {
// ok
//
// The `Box<u16>` impls fails the leak check,
// meaning that we apply the `Box<u32>` impl.
impls_leak::<Box<_>>();
// error: type annotations needed
//
// While the `Box<u16>` impl would fail the leak check
// we have already instantiated the binder while applying
// the generic `IndirectLeak` impl, so during candidate
// selection of `Leak` we do not detect the placeholder error.
// Evaluation of `Box<_>: Leak<'!a>` is therefore ambiguous,
// resulting in `for<'a> Box<_>: Leak<'a>` also being ambiguous.
impls_indirect_leak::<Box<_>>();
}
```
We generally prefer `where`-bounds over implementations during candidate selection, both for [trait goals](11f32b73e0/compiler/rustc_trait_selection/src/traits/select/mod.rs (L1863-L1887)) and during [normalization](11f32b73e0/compiler/rustc_trait_selection/src/traits/project.rs (L184-L198)). However, we currently **do not** use the `leak_check` during candidate assembly in normalizing. This can result in inconsistent behavior:
```rust
trait Trait<'a> {
type Assoc;
}
impl<'a, T> Trait<'a> for T {
type Assoc = usize;
}
fn trait_bound<T: for<'a> Trait<'a>>() {}
fn projection_bound<T: for<'a> Trait<'a, Assoc = usize>>() {}
// A function with a trivial where-bound which is more
// restrictive than the impl.
fn function<T: Trait<'static, Assoc = usize>>() {
// ok
//
// Proving `for<'a> T: Trait<'a>` using the where-bound results
// in a leak check failure, so we use the more general impl,
// causing this to succeed.
trait_bound::<T>();
// error
//
// Proving the `Projection` goal `for<'a> T: Trait<'a, Assoc = usize>`
// does not use the leak check when trying the where-bound, causing us
// to prefer it over the impl, resulting in a placeholder error.
projection_bound::<T>();
// error
//
// Trying to normalize the type `for<'a> fn(<T as Trait<'a>>::Assoc)`
// only gets to `<T as Trait<'a>>::Assoc` once `'a` has been already
// instantiated, causing us to prefer the where-bound over the impl
// resulting in a placeholder error. Even if were were to also use the
// leak check during candidate selection for normalization, this
// case would still not compile.
let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
}
```
This is also likely to be more performant. It enables more caching in the new trait solver by simply [recursively calling the canonical query][new solver] after instantiating the higher-ranked goal.
It is also unclear how to add the leak check to normalization in the new solver. To handle https://github.com/rust-lang/trait-system-refactor-initiative/issues/1 `Projection` goals are implemented via `AliasRelate`. This again means that we instantiate the binder before ever normalizing any alias. Even if we were to avoid this, we lose the ability to [cache normalization by itself, ignoring the expected `term`](5bd5d214ef/compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs (L34-L49)). We cannot replace the `term` with an inference variable before instantiating the binder, as otherwise `for<'a> T: Trait<Assoc<'a> = &'a ()>` breaks. If we only replace the term after instantiating the binder, we cannot easily evaluate the goal in a separate context, as [we'd then lose the information necessary for the leak check](11f32b73e0/compiler/rustc_next_trait_solver/src/canonicalizer.rs (L230-L232)). Adding this information to the canonical input also seems non-trivial.
## Proposed solution
I propose to instantiate the binder outside of candidate assembly, causing placeholders from higher-ranked goals to get ignored while selecting their candidate. This mostly[^1] matches the [current behavior of the new solver][new solver]. The impact of this change is therefore as follows:
```rust
trait Leak<'a> {}
impl Leak<'_> for Box<u32> {}
impl Leak<'static> for Box<u16> {}
fn impls_leak<T: for<'a> Leak<'a>>() {}
trait IndirectLeak<'a> {}
impl<'a, T: Leak<'a>> IndirectLeak<'a> for T {}
fn impls_indirect_leak<T: for<'a> IndirectLeak<'a>>() {}
fn guide_selection() {
// ok -> ambiguous
impls_leak::<Box<_>>();
// ambiguous
impls_indirect_leak::<Box<_>>();
}
trait Trait<'a> {
type Assoc;
}
impl<'a, T> Trait<'a> for T {
type Assoc = usize;
}
fn trait_bound<T: for<'a> Trait<'a>>() {}
fn projection_bound<T: for<'a> Trait<'a, Assoc = usize>>() {}
// A function which a trivial where-bound which is more
// restrictive than the impl.
fn function<T: Trait<'static, Assoc = usize>>() {
// ok -> error
trait_bound::<T>();
// error
projection_bound::<T>();
// error
let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
}
```
This does not change the behavior if candidates have higher ranked nested goals, as in this case the `leak_check` causes the nested goal to result in an error ([playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=a74c25300b23db9022226de99d8a2fa6)):
```rust
trait LeakCheckFailure<'a> {}
impl LeakCheckFailure<'static> for () {}
trait Trait<T> {}
impl Trait<u32> for () where for<'a> (): LeakCheckFailure<'a> {}
impl Trait<u16> for () {}
fn impls_trait<T: Trait<U>, U>() {}
fn main() {
// ok
//
// It does not matter whether candidate assembly
// considers the placeholders from higher-ranked goal.
//
// Either `for<'a> (): LeakCheckFailure<'a>` has no
// applicable candidate or it has a single applicable candidate
// when then later results in an error. This allows us to
// infer `U` to `u16`.
impls_trait::<(), _>()
}
```
## Impact on existing crates
This is a **breaking change**. [A crater run](https://github.com/rust-lang/rust/pull/119820#issuecomment-1926862174) found 17 regressed crates with 7 root causes.
For a full analysis of all affected crates, see https://gist.github.com/lcnr/7c1c652f30567048ea240554a36ed95c.
---
I believe this breakage to be acceptable and would merge this change. I am confident that the new position of the leak check matches our idealized future and cannot envision any other consistent alternative. Where possible, I intend to open PRs fixing/avoiding the regressions before landing this PR.
I originally intended to remove the `coherence_leak_check` lint in the same PR. However, while I am confident in the *position* of the leak check, deciding on its exact behavior is left as future work, cc #112999. This PR therefore only moves the leak check while keeping the lint when relying on it in coherence.
[new solver]: https://github.com/rust-lang/rust/blob/master/compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs#L479-L484
[^1]: the new solver has a separate cause of inconsistent behavior rn https://github.com/rust-lang/trait-system-refactor-initiative/issues/53#issuecomment-1914310171
r? `@nikomatsakis`
