`-Ztreat-err-as-bug` treats normal errors and delayed bugs equally,
which can lead to some really surprising results.
This commit changes `-Ztreat-err-as-bug` so it ignores delayed bugs,
unless they get promoted to proper bugs and are printed.
This feels to me much simpler and more logical. And it simplifies the
implementation:
- The `-Ztreat-err-as-bug` check is removed from in
`DiagCtxt::{delayed_bug,span_delayed_bug}`.
- `treat_err_as_bug` doesn't need to count delayed bugs.
- The `-Ztreat-err-as-bug` panic message is simpler, because it doesn't
have to mention delayed bugs.
Output of delayed bugs is now more consistent. They're always printed
the same way. Previously when they triggered `-Ztreat-err-as-bug` they
would be printed slightly differently, via `span_bug` in
`span_delayed_bug` or `delayed_bug`.
A minor behaviour change: the "no errors encountered even though
`span_delayed_bug` issued" printed before delayed bugs is now a note
rather than a bug. This is done so it doesn't get counted as an error
that might trigger `-Ztreat-err-as-bug`, which would be silly.
This means that if you use `-Ztreat-err-as-bug=1` and there are no
normal errors but there are delayed bugs, the first delayed bug will be
shown (and the panic will happen after it's printed).
Also, I have added a second note saying "those delayed bugs will now be
shown as internal compiler errors". I think this makes it clearer what
is happening, because the whole concept of delayed bugs is non-obvious.
There are some test changes.
- equality-in-canonical-query.rs: Minor output changes, and the error
count reduces by one because the "no errors encountered even though
`span_delayed_bug` issued" message is no longer counted as an error.
- rpit_tait_equality_in_canonical_query.rs: Ditto.
- storage-live.rs: The query stack disappears because these delayed bugs
are now printed at the end, rather than when they are created.
- storage-return.rs, span_delayed_bug.rs: now need
`-Zeagerly-emit-delayed-bugs` because they need the delayed bugs
emitted immediately to preserve behaviour.
Remove special-casing around `AliasKind::Opaque` when structurally resolving in new solver
This fixes a few inconsistencies around where we don't eagerly resolve opaques to their (locally-defined) hidden types in the new solver. It essentially allows this code to work:
```rust
fn main() {
type Tait = impl Sized;
struct S {
i: i32,
}
let x: Tait = S { i: 0 };
println!("{}", x.i);
}
```
Since `Tait` is defined in `main`, we are able to poke through the type of `x` with deref.
r? lcnr
Errors in `DiagCtxtInner::emit_diagnostic` are never set to
`Level::Bug`, because the condition never succeeds, because
`self.treat_err_as_bug()` is called *before* the error counts are
incremented.
This commit switches to `self.treat_next_err_as_bug()`, fixing the
problem. This changes the error message output to actually say "internal
compiler error".
Add regression test for #106630
This PR adds a regression test for #106630. I was unsure where exactly to place the test or how to test it locally so please let me know if I should change something.
On borrow return type, suggest borrowing from arg or owned return type
When we encounter a function with a return type that has an anonymous lifetime with no argument to borrow from, besides suggesting the `'static` lifetime we now also suggest changing the arguments to be borrows or changing the return type to be an owned type.
```
error[E0106]: missing lifetime specifier
--> $DIR/variadic-ffi-6.rs:7:6
|
LL | ) -> &usize {
| ^ expected named lifetime parameter
|
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
|
LL | ) -> &'static usize {
| +++++++
help: instead, you are more likely to want to change one of the arguments to be borrowed...
|
LL | x: &usize,
| +
help: ...or alternatively, to want to return an owned value
|
LL - ) -> &usize {
LL + ) -> usize {
|
```
Fix#85843.
When we encounter a function with a return type that has an anonymous
lifetime with no argument to borrow from, besides suggesting the
`'static` lifetime we now also suggest changing the arguments to be
borrows or changing the return type to be an owned type.
```
error[E0106]: missing lifetime specifier
--> $DIR/variadic-ffi-6.rs:7:6
|
LL | ) -> &usize {
| ^ expected named lifetime parameter
|
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
|
LL | ) -> &'static usize {
| +++++++
help: instead, you are more likely to want to change one of the arguments to be borrowed...
|
LL | x: &usize,
| +
help: ...or alternatively, to want to return an owned value
|
LL - ) -> &usize {
LL + ) -> usize {
|
```
Fix#85843.
new solver normalization improvements
cool beans
At the core of this PR is a `try_normalize_ty` which stops for rigid aliases by using `commit_if_ok`.
Reworks alias-relate to fully normalize both the lhs and rhs and then equate the resulting rigid (or inference) types. This fixes https://github.com/rust-lang/trait-system-refactor-initiative/issues/68 by avoiding the exponential blowup. Also supersedes #116369 by only defining opaque types if the hidden type is rigid.
I removed the stability check in `EvalCtxt::evaluate_goal` due to https://github.com/rust-lang/trait-system-refactor-initiative/issues/75. While I personally have opinions on how to fix it, that still requires further t-types/`@nikomatsakis` buy-in, so I removed that for now. Once we've decided on our approach there, we can revert this commit.
r? `@compiler-errors`
Fix depth check in ProofTreeVisitor.
The hack to cutoff overflows and cycles in the new trait solver was incorrect. We want to inspect everything with depth [0..10].
This fix exposed a previously unseen bug, which caused the compiler to ICE when invoking `trait_ref` on a non-assoc type projection. I simply added the guard in the `AmbiguityCausesVisitor`, and updated the expected output for the `auto-trait-coherence` test which now includes the extra note:
```text
|
= note: upstream crates may add a new impl of trait `std::marker::Send` for type `OpaqueType` in future versions
```
r? `@lcnr`
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.
