When we encounter a `dyn Trait` that isn't object safe, look for its
implementors. If there's one, mention using it directly If there are
less than 9, mention the possibility of creating a new enum and using
that instead.
Account for object unsafe `impl Trait on dyn Trait {}`. Make a
distinction between public and sealed traits.
Fix#80194.
Consider alias bounds when computing liveness in NLL (but this time sound hopefully)
This is a revival of #116040, except removing the changes to opaque lifetime captures check to make sure that we're not triggering any unsoundness due to the lack of general existential regions and the currently-existing `ReErased` hack we use instead.
r? `@aliemjay` -- I appreciate you pointing out the unsoundenss in the previous iteration of this PR, and I'd like to hear that you're happy with this iteration of this PR before this goes back into FCP :>
Fixes#116794 as well
---
(mostly copied from #116040 and reworked slightly)
# Background
Right now, liveness analysis in NLL is a bit simplistic. It simply walks through all of the regions of a type and marks them as being live at points. This is problematic in the case of aliases, since it requires that we mark **all** of the regions in their args[^1] as live, leading to bugs like #42940.
In reality, we may be able to deduce that fewer regions are allowed to be present in the projected type (or "hidden type" for opaques) via item bounds or where clauses, and therefore ideally, we should be able to soundly require fewer regions to be live in the alias.
For example:
```rust
trait Captures<'a> {}
impl<T> Captures<'_> for T {}
fn capture<'o>(_: &'o mut ()) -> impl Sized + Captures<'o> + 'static {}
fn test_two_mut(mut x: ()) {
let _f1 = capture(&mut x);
let _f2 = capture(&mut x);
//~^ ERROR cannot borrow `x` as mutable more than once at a time
}
```
In the example above, we should be able to deduce from the `'static` bound on `capture`'s opaque that even though `'o` is a captured region, it *can never* show up in the opaque's hidden type, and can soundly be ignored for liveness purposes.
# The Fix
We apply a simple version of RFC 1214's `OutlivesProjectionEnv` and `OutlivesProjectionTraitDef` rules to NLL's `make_all_regions_live` computation.
Specifically, when we encounter an alias type, we:
1. Look for a unique outlives bound in the param-env or item bounds for that alias. If there is more than one unique region, bail, unless any of the outlives bound's regions is `'static`, and in that case, prefer `'static`. If we find such a unique region, we can mark that outlives region as live and skip walking through the args of the opaque.
2. Otherwise, walk through the alias's args recursively, as we do today.
## Limitation: Multiple choices
This approach has some limitations. Firstly, since liveness doesn't use the same type-test logic as outlives bounds do, we can't really try several options when we're faced with a choice.
If we encounter two unique outlives regions in the param-env or bounds, we simply fall back to walking the opaque via its args. I expect this to be mostly mitigated by the special treatment of `'static`, and can be fixed in a forwards-compatible by a more sophisticated analysis in the future.
## Limitation: Opaque hidden types
Secondly, we do not employ any of these rules when considering whether the regions captured by a hidden type are valid. That causes this code (cc #42940) to fail:
```rust
trait Captures<'a> {}
impl<T> Captures<'_> for T {}
fn a() -> impl Sized + 'static {
b(&vec![])
}
fn b<'o>(_: &'o Vec<i32>) -> impl Sized + Captures<'o> + 'static {}
```
We need to have existential regions to avoid [unsoundness](https://github.com/rust-lang/rust/pull/116040#issuecomment-1751628189) when an opaque captures a region which is not represented in its own substs but which outlives a region that does.
## Read more
Context: https://github.com/rust-lang/rust/pull/115822#issuecomment-1731153952 (for the liveness case)
More context: https://github.com/rust-lang/rust/issues/42940#issuecomment-455198309 (for the opaque capture case, which this does not fix)
[^1]: except for bivariant region args in opaques, which will become less relevant when we move onto edition 2024 capture semantics for opaques.
Stash and cancel cycle errors for auto trait leakage in opaques
We don't need to emit a traditional cycle error when we have a selection error that explains what's going on but in more detail.
We may want to augment this error to actually point out the cycle, now that the cycle error is not being emitted. We could do that by storing the set of opaques that was in the `CyclePlaceholder` that gets returned from `type_of_opaque`.
r? `@oli-obk` cc `@estebank` #117235
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
Esteban Küber