Show detailed expected/found types in error message when trait paths are the same
Fixes#65230.
### Issue solved by this PR
```rust
trait T {
type U;
fn f(&self) -> Self::U;
}
struct X<'a>(&'a mut i32);
impl<'a> T for X<'a> {
type U = &'a i32;
fn f(&self) -> Self::U {
self.0
}
}
fn main() {}
```
Compiler generates the following note:
```
note: ...so that the types are compatible
--> test.rs:10:28
|
10 | fn f(&self) -> Self::U {
| ____________________________^
11 | | self.0
12 | | }
| |_____^
= note: expected `T`
found `T`
```
This note is not useful since the expected type and the found type are the same.
### How this PR solve the issue
When the expected type and the found type are exactly the same in string representation, the note falls back to the detailed string representation of trait ref:
```
note: ...so that the types are compatible
--> test.rs:10:28
|
10 | fn f(&self) -> Self::U {
| ____________________________^
11 | | self.0
12 | | }
| |_____^
= note: expected `<X<'a> as T>`
found `<X<'_> as T>`
```
So that a user can notice what was different between the expected one and the found one.
Add two inline annotations for hot functions
These two functions are essentially no-ops (and compile to just a load and
return), but show up in process_obligations profiles with a high call count --
so worthwhile to try and inline them. This is not normally possible as they're
non-generic, so they don't get offered for inlining by our current algorithm.
These two functions are essentially no-ops (and compile to just a load and
return), but show up in process_obligations profiles with a high call count --
so worthwhile to try and inline them away.
This PR has several interconnected pieces:
1. In some of the NLL region error code, we now pass
around an `ObligationCause`, instead of just a plain `Span`.
This gets forwarded into `fulfill_cx.register_predicate_obligation`
during error reporting.
2. The general InferCtxt error reporting code is extended to
handle `ObligationCauseCode::BindingObligation`
3. A new enum variant `ConstraintCategory::Predicate` is added.
We try to avoid using this as the 'best blame constraint' - instead,
we use it to enhance the `ObligationCause` of the `BlameConstraint`
that we do end up choosing.
As a result, several NLL error messages now contain the same
"the lifetime requirement is introduced here" message as non-NLL
errors.
Having an `ObligationCause` available will likely prove useful
for future improvements to NLL error messages.
Be explicit about using Binder::dummy
This is somewhat of a late followup to the binder refactor PR. It removes `ToPredicate` and `ToPolyTraitImpls` that hide the use of `Binder::dummy`. While this does make code a bit more verbose, it allows us be more careful about where we create binders.
Another alternative here might be to add a new trait `ToBinder` or something with a `dummy()` fn. Which could still allow grepping but allows doing something like `trait_ref.dummy()` (but I also wonder if longer-term, it would be better to be even more explicit with a `bind_with_vars(ty::List::empty())` *but* that's not clear yet.
r? ``@nikomatsakis``
Revise never type fallback algorithm
This is a rebase of https://github.com/rust-lang/rust/pull/84573, but dropping the stabilization of never type (and the accompanying large test diff).
Each commit builds & has tests updated alongside it, and could be reviewed in a more or less standalone fashion. But it may make more sense to review the PR as a whole, I'm not sure. It should be noted that tests being updated isn't really a good indicator of final behavior -- never_type_fallback is not enabled by default in this PR, so we can't really see the full effects of the commits here.
This combines the work by Niko, which is [documented in this gist](https://gist.github.com/nikomatsakis/7a07b265dc12f5c3b3bd0422018fa660), with some additional rules largely derived to target specific known patterns that regress with the algorithm solely derived by Niko. We build these from an intuition that:
* In general, fallback to `()` is *sound* in all cases
* But, in general, we *prefer* fallback to `!` as it accepts more code, particularly that written to intentionally use `!` (e.g., Result's with a Infallible/! variant).
When evaluating Niko's proposed algorithm, we find that there are certain cases where fallback to `!` leads to compilation failures in real-world code, and fallback to `()` fixes those errors. In order to allow for stabilization, we need to fix a good portion of these patterns.
The final rule set this PR proposes is that, by default, we fallback from `?T` to `!`, with the following exceptions:
1. `?T: Foo` and `Bar::Baz = ?T` and `(): Foo`, then fallback to `()`
2. Per [Niko's algorithm](https://gist.github.com/nikomatsakis/7a07b265dc12f5c3b3bd0422018fa660#proposal-fallback-chooses-between--and--based-on-the-coercion-graph), the "live" `?T` also fallback to `()`.
The first rule is necessary to address a fairly common pattern which boils down to something like the snippet below. Without rule 1, we do not see the closure's return type as needing a () fallback, which leads to compilation failure.
```rust
#![feature(never_type_fallback)]
trait Bar { }
impl Bar for () { }
impl Bar for u32 { }
fn foo<R: Bar>(_: impl Fn() -> R) {}
fn main() {
foo(|| panic!());
}
```
r? `@jackh726`
Migrate in-tree crates to 2021
This replaces #89075 (cherry picking some of the commits from there), and closes#88637 and fixes#89074.
It excludes a migration of the library crates for now (see tidy diff) because we have some pending bugs around macro spans to fix there.
I instrumented bootstrap during the migration to make sure all crates moved from 2018 to 2021 had the compatibility warnings applied first.
Originally, the intent was to support cargo fix --edition within bootstrap, but this proved fairly difficult to pull off. We'd need to architect the check functionality to support running cargo check and cargo fix within the same x.py invocation, and only resetting sysroots on check. Further, it was found that cargo fix doesn't behave too well with "not quite workspaces", such as Clippy which has several crates. Bootstrap runs with --manifest-path ... for all the tools, and this makes cargo fix only attempt migration for that crate. We can't use e.g. --workspace due to needing to maintain sysroots for different phases of compilation appropriately.
It is recommended to skip the mass migration of Cargo.toml's to 2021 for review purposes; you can also use `git diff d6cd2c6c87 -I'^edition = .20...$'` to ignore the edition = 2018/21 lines in the diff.
This just applies the suggested fixes from the compatibility warnings,
leaving any that are in practice spurious in. This is primarily intended to
provide a starting point to identify possible fixes to the migrations (e.g., by
avoiding spurious warnings).
A secondary commit cleans these up where they are false positives (as is true in
many of the cases).
The comment seems incorrect. Testing revealed that the examples in
question still work (as well as some variants) even without the
special casing here.
We now fallback type variables using the following rules:
* Construct a coercion graph `A -> B` where `A` and `B` are unresolved
type variables or the `!` type.
* Let D be those variables that are reachable from `!`.
* Let N be those variables that are reachable from a variable not in
D.
* All variables in (D \ N) fallback to `!`.
* All variables in (D & N) fallback to `()`.
Remove concept of 'completion' from the projection cache
Fixes#88910
When we initially store a `NormalizedTy` in the projection cache,
we discard all obligations that we can (while ensuring that we
don't cause any issues with incremental compilation).
Marking a projection cache entry as 'completed' discards all
obligations associated with it. This can only cause problems,
since any obligations stored in the cache are there for a reason
(e.g. they evaluate to `EvaluatedToOkModuloRegions`).
This commit removes `complete` and `complete_normalized` entirely.
Point at argument instead of call for their obligations
When an obligation is introduced by a specific `fn` argument, point at
the argument instead of the `fn` call if the obligation fails to be
fulfilled.
Move the information about pointing at the call argument expression in
an unmet obligation span from the `FulfillmentError` to a new
`ObligationCauseCode`.
When giving an error about an obligation introduced by a function call
that an argument doesn't fulfill, and that argument is a block, add a
span_label pointing at the innermost tail expression.
Current output:
```
error[E0425]: cannot find value `x` in this scope
--> f10.rs:4:14
|
4 | Some(x * 2)
| ^ not found in this scope
error[E0277]: expected a `FnOnce<({integer},)>` closure, found `Option<_>`
--> f10.rs:2:31
|
2 | let p = Some(45).and_then({
| ______________________--------_^
| | |
| | required by a bound introduced by this call
3 | | |x| println!("doubling {}", x);
4 | | Some(x * 2)
| | -----------
5 | | });
| |_____^ expected an `FnOnce<({integer},)>` closure, found `Option<_>`
|
= help: the trait `FnOnce<({integer},)>` is not implemented for `Option<_>`
```
Previous output:
```
error[E0425]: cannot find value `x` in this scope
--> f10.rs:4:14
|
4 | Some(x * 2)
| ^ not found in this scope
error[E0277]: expected a `FnOnce<({integer},)>` closure, found `Option<_>`
--> f10.rs:2:22
|
2 | let p = Some(45).and_then({
| ^^^^^^^^ expected an `FnOnce<({integer},)>` closure, found `Option<_>`
|
= help: the trait `FnOnce<({integer},)>` is not implemented for `Option<_>`
```
Partially address #27300. Will require rebasing on top of #88546.
Move the information about pointing at the call argument expression in
an unmet obligation span from the `FulfillmentError` to a new
`ObligationCauseCode`.
Fixes#88910
When we initially store a `NormalizedTy` in the projection cache,
we discard all obligations that we can (while ensuring that we
don't cause any issues with incremental compilation).
Marking a projection cache entry as 'completed' discards all
obligations associated with it. This can only cause problems,
since any obligations stored in the cache are there for a reason
(e.g. they evaluate to `EvaluatedToOkModuloRegions`).
This commit removes `complete` and `complete_normalized` entirely.
Detect stricter constraints on gats where clauses in impls vs trait
I might try to see if I can do a bit more to improve these diagnostics, but any initial feedback is appreciated. I can also do any additional work in a followup PR.
r? `@estebank`
Path remapping: Make behavior of diagnostics output dependent on presence of --remap-path-prefix.
This PR fixes a regression (#87745) with `--remap-path-prefix` where the flag stopped causing diagnostic messages to be remapped as well. The regression was introduced in https://github.com/rust-lang/rust/pull/83813 where we erroneously assumed that remapping of diagnostic messages was not desired anymore (because #70642 partially undid that functionality with nobody objecting).
The issue is fixed by making `--remap-path-prefix` remap diagnostic messages again, including for paths that have been remapped in upstream crates (e.g. the standard library). This means that "sysroot-localization" (implemented in #70642) is also disabled if `rustc` is invoked with `--remap-path-prefix`. The assumption is that once someone starts explicitly remapping paths they also don't want paths to their local Rust installation in their build output.
In the future we might want to give more fine-grained control over this behavior via compiler flags (see https://github.com/rust-lang/rfcs/pull/3127 for a related RFC). For now this PR is intended as a regression fix.
This PR is an alternative to https://github.com/rust-lang/rust/pull/88191, which makes diagnostic messages be remapped unconditionally. That approach, however, would effectively revert #70642.
Fixes https://github.com/rust-lang/rust/issues/87745.
cc `@cbeuw`
r? `@ghost`
