Rework negative coherence to properly consider impls that only partly overlap
This PR implements a modified negative coherence that handles impls that only have partial overlap.
It does this by:
1. taking both impl trait refs, instantiating them with infer vars
2. equating both trait refs
3. taking the equated trait ref (which represents the two impls' intersection), and resolving any vars
4. plugging all remaining infer vars with placeholder types
these placeholder-plugged trait refs can then be used normally with the new trait solver, since we no longer have to worry about the issue with infer vars in param-envs.
We use the **new trait solver** to reason correctly about unnormalized trait refs (due to deferred projection equality), since this avoid having to normalize anything under param-envs with infer vars in them.
This PR then additionally:
* removes the `FnPtr` knowable hack by implementing proper negative `FnPtr` trait bounds for rigid types.
---
An example:
Consider these two partially overlapping impls:
```
impl<T, U> PartialEq<&U> for &T where T: PartialEq<U> {}
impl<F> PartialEq<F> for F where F: FnPtr {}
```
Under the old algorithm, we would take one of these impls and replace it with infer vars, then try unifying it with the other impl under identity substitutions. This is not possible in either direction, since it either sets `T = U`, or tries to equate `F = &?0`.
Under the new algorithm, we try to unify `?0: PartialEq<?0>` with `&?1: PartialEq<&?2>`. This gives us `?0 = &?1 = &?2` and thus `?1 = ?2`. The intersection of these two trait refs therefore looks like: `&?1: PartialEq<&?1>`. After plugging this with placeholders, we get a trait ref that looks like `&!0: PartialEq<&!0>`, with the first impl having substs `?T = ?U = !0` and the second having substs `?F = &!0`[^1].
Then we can take the param-env from the first impl, and try to prove the negated where clause of the second.
We know that `&!0: !FnPtr` never holds, since it's a rigid type that is also not a fn ptr, we successfully detect that these impls may never overlap.
[^1]: For the purposes of this example, I just ignored lifetimes, since it doesn't really matter.
Fix implied outlives check for GAT in RPITIT
We enforce certain `Self: 'lt` bounds for GATs to save space for more sophisticated implied bounds, but those currently operate on the HIR. Code was easily reworked to operate on def-ids so that we can properly let these suggestions propagate through synthetic associated types like RPITITs and AFITs.
r? `@jackh726` or `@aliemjay`
Fixes#116789
Relate alias ty with variance
In the new solver, turns out that the subst-relate branch of the alias-relate predicate was relating args invariantly even for opaques, which have variance 💀.
This change is a bit more invasive, but I'd rather not special-case it [here](aeaa5c30e5/compiler/rustc_trait_selection/src/solve/alias_relate.rs (L171-L190)) and then have it break elsewhere. I'm doing a perf run to see if the extra call to `def_kind` is that expensive, if it is, I'll reconsider.
r? ``@lcnr``
Show more information when multiple `impl`s apply
- When there are `impl`s without type params, show only those (to avoid showing overly generic `impl`s).
```
error[E0283]: type annotations needed
--> $DIR/multiple-impl-apply.rs:34:9
|
LL | let y = x.into();
| ^ ---- type must be known at this point
|
note: multiple `impl`s satisfying `_: From<Baz>` found
--> $DIR/multiple-impl-apply.rs:14:1
|
LL | impl From<Baz> for Bar {
| ^^^^^^^^^^^^^^^^^^^^^^
...
LL | impl From<Baz> for Foo {
| ^^^^^^^^^^^^^^^^^^^^^^
= note: required for `Baz` to implement `Into<_>`
help: consider giving `y` an explicit type
|
LL | let y: /* Type */ = x.into();
| ++++++++++++
```
- Lower the importance of `T: Sized`, `T: WellFormed` and coercion errors, to prioritize more relevant errors. The pre-existing deduplication logic deals with hiding redundant errors better that way, and we show errors with more metadata that is useful to the user.
- Show `<SelfTy as Trait>::assoc_fn` suggestion in more cases.
```
error[E0790]: cannot call associated function on trait without specifying the corresponding `impl` type
--> $DIR/cross-return-site-inference.rs:38:16
|
LL | return Err(From::from("foo"));
| ^^^^^^^^^^ cannot call associated function of trait
|
help: use a fully-qualified path to a specific available implementation
|
LL | return Err(</* self type */ as From>::from("foo"));
| +++++++++++++++++++ +
```
Fix#88284.
Add a note to duplicate diagnostics
Helps explain why there may be a difference between manual testing and the test suite output and highlights them as something to potentially look into
For existing duplicate diagnostics I just blessed them other than a few files that had other `NOTE` annotations in
More accurately point to where default return type should go
When getting the "default return type" span, instead of pointing to the low span of the next token, point to the high span of the previous token. This:
1. Makes forming return type suggestions more uniform, since we expect them all in the same place.
2. Arguably makes labels easier to understand, since we're pointing to where the implicit `-> ()` would've gone, rather than the starting brace or the semicolon.
r? ```@estebank```
In `report_fullfillment_errors` push back `T: Sized`, `T: WellFormed`
and coercion errors to the end of the list. The pre-existing
deduplication logic eliminates redundant errors better that way, keeping
the resulting output with fewer errors than before, while also having
more detail.
Stabilize `impl_trait_projections`
Closes#115659
## TL;DR:
This allows us to mention `Self` and `T::Assoc` in async fn and return-position `impl Trait`, as you would expect you'd be able to.
Some examples:
```rust
#![feature(return_position_impl_trait_in_trait, async_fn_in_trait)]
// (just needed for final tests below)
// ---------------------------------------- //
struct Wrapper<'a, T>(&'a T);
impl Wrapper<'_, ()> {
async fn async_fn() -> Self {
//^ Previously rejected because it returns `-> Self`, not `-> Wrapper<'_, ()>`.
Wrapper(&())
}
fn impl_trait() -> impl Iterator<Item = Self> {
//^ Previously rejected because it mentions `Self`, not `Wrapper<'_, ()>`.
std::iter::once(Wrapper(&()))
}
}
// ---------------------------------------- //
trait Trait<'a> {
type Assoc;
fn new() -> Self::Assoc;
}
impl Trait<'_> for () {
type Assoc = ();
fn new() {}
}
impl<'a, T: Trait<'a>> Wrapper<'a, T> {
async fn mk_assoc() -> T::Assoc {
//^ Previously rejected because `T::Assoc` doesn't mention `'a` in the HIR,
// but ends up resolving to `<T as Trait<'a>>::Assoc`, which does rely on `'a`.
// That's the important part -- the elided trait.
T::new()
}
fn a_few_assocs() -> impl Iterator<Item = T::Assoc> {
//^ Previously rejected for the same reason
[T::new(), T::new(), T::new()].into_iter()
}
}
// ---------------------------------------- //
trait InTrait {
async fn async_fn() -> Self;
fn impl_trait() -> impl Iterator<Item = Self>;
}
impl InTrait for &() {
async fn async_fn() -> Self { &() }
//^ Previously rejected just like inherent impls
fn impl_trait() -> impl Iterator<Item = Self> {
//^ Previously rejected just like inherent impls
[&()].into_iter()
}
}
```
## Technical:
Lifetimes in return-position `impl Trait` (and `async fn`) are duplicated as early-bound generics local to the opaque in order to make sure we are able to substitute any late-bound lifetimes from the function in the opaque's hidden type. (The [dev guide](https://rustc-dev-guide.rust-lang.org/return-position-impl-trait-in-trait.html#aside-opaque-lifetime-duplication) has a small section about why this is necessary -- this was written for RPITITs, but it applies to all RPITs)
Prior to #103491, all of the early-bound lifetimes not local to the opaque were replaced with `'static` to avoid issues where relating opaques caused their *non-captured* lifetimes to be related. This `'static` replacement led to strange and possibly unsound behaviors (https://github.com/rust-lang/rust/issues/61949#issuecomment-508836314) (https://github.com/rust-lang/rust/issues/53613) when referencing the `Self` type alias in an impl or indirectly referencing a lifetime parameter via a projection type (via a `T::Assoc` projection without an explicit trait), since lifetime resolution is performed on the HIR, when neither `T::Assoc`-style projections or `Self` in impls are expanded.
Therefore an error was implemented in #62849 to deny this subtle behavior as a known limitation of the compiler. It was attempted by `@cjgillot` to fix this in #91403, which was subsequently unlanded. Then it was re-attempted to much success (🎉) in #103491, which is where we currently are in the compiler.
The PR above (#103491) fixed this issue technically by *not* replacing the opaque's parent lifetimes with `'static`, but instead using variance to properly track which lifetimes are captured and are not. The PR gated any of the "side-effects" of the PR behind a feature gate (`impl_trait_projections`) presumably to avoid having to involve T-lang or T-types in the PR as well. `@cjgillot` can clarify this if I'm misunderstanding what their intention was with the feature gate.
Since we're not replacing (possibly *invariant*!) lifetimes with `'static` anymore, there are no more soundness concerns here. Therefore, this PR removes the feature gate.
Tests:
* `tests/ui/async-await/feature-self-return-type.rs`
* `tests/ui/impl-trait/feature-self-return-type.rs`
* `tests/ui/async-await/issues/issue-78600.rs`
* `tests/ui/impl-trait/capture-lifetime-not-in-hir.rs`
---
r? cjgillot on the impl (not much, just removing the feature gate)
I'm gonna mark this as FCP for T-lang and T-types.
Anonymize binders for `refining_impl_trait` check
We're naively using the equality impl for `ty::Clause` in the refinement check, which is okay *except* for binders, which carry some information about where they come from in the AST. Those locations are not gonna be equal between traits and impls, so anonymize those clauses so that this doesn't matter.
Fixes#116135
Only prevent field projections into opaque types, not types containing opaque types
fixes https://github.com/rust-lang/rust/issues/115778
I did not think that original condition through properly... I'll also need to check the similar check around the other `ProjectionKind::OpaqueCast` creation site (this one is in hir, the other one is in mir), but I'll do that change in another PR that doesn't go into a beta backport.
Use placeholders to prevent using inferred RPITIT types to imply their own well-formedness
The issue here is that we use the same signature to do RPITIT inference as we do to compute implied bounds. To fix this, when gathering the assumed wf types for the method, we replace all of the infer vars (that will be eventually used to infer RPITIT types) with type placeholders, which imply nothing about lifetime bounds.
This solution kind of sucks, but I'm not certain there's another feasible way to fix this. If anyone has a better solution, I'd be glad to hear it.
My naive first solution was, instead of using placeholders, to replace the signature with the RPITIT projections that it originally started out with. But turns out that we can't just use the unnormalized signature of the trait method in `implied_outlives_bounds` since we normalize during WF computation -- that would cause a query cycle in `collect_return_position_impl_trait_in_trait_tys`.
idk who to request review...
r? `@lcnr` or `@aliemjay` i guess.
Fixes#116060
