Use root obligation on E0277 for some cases #121826

estebank · 2024-03-01T00:46:29Z

When encountering trait bound errors that satisfy some heuristics that tell us that the relevant trait for the user comes from the root obligation and not the current obligation, we use the root predicate for the main message.

This allows to talk about "X doesn't implement Pattern<'_>" over the most specific case that just happened to fail, like "char doesn't implement Fn(&mut char)" in
tests/ui/traits/suggest-dereferences/root-obligation.rs

The heuristics are:

the type of the leaf predicate is (roughly) the same as the type from the root predicate, as a proxy for "we care about the root"
the leaf trait and the root trait are different, so as to avoid talking about &mut T: Trait and instead remain talking about T: Trait instead
the root trait is not Unsize, as to avoid talking about it in tests/ui/coercion/coerce-issue-49593-box-never.rs.

error[E0277]: the trait bound `&char: Pattern<'_>` is not satisfied
  --> $DIR/root-obligation.rs:6:38
   |
LL |         .filter(|c| "aeiou".contains(c))
   |                             -------- ^ the trait `Fn<(char,)>` is not implemented for `&char`, which is required by `&char: Pattern<'_>`
   |                             |
   |                             required by a bound introduced by this call
   |
   = note: required for `&char` to implement `FnOnce<(char,)>`
   = note: required for `&char` to implement `Pattern<'_>`
note: required by a bound in `core::str::<impl str>::contains`
  --> $SRC_DIR/core/src/str/mod.rs:LL:COL
help: consider dereferencing here
   |
LL |         .filter(|c| "aeiou".contains(*c))
   |                                      +

Fix #79359, fix #119983, fix #118779, cc #118415 (the suggestion needs to change), cc #121398 (doesn't fix the underlying issue).

rustbot · 2024-03-01T00:46:36Z

r? @pnkfelix

rustbot has assigned @pnkfelix.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

estebank · 2024-03-01T00:48:47Z

CC @lcnr, @compiler-errors, @oli-obk & @fmease. This is an attempt to mitigate the issue highlighted at https://rust-lang.zulipchat.com/#narrow/stream/144729-t-types/topic/Change.20to.20default.20E0277.20message and the linked tickets. There are many more things to be fixed in these errors, but this seemed like a small enough change that could pull its weight to stop confusing people in cases like the T: Pattern<'_> one.

library/core/src/iter/traits/collect.rs

compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs

estebank · 2024-03-01T16:56:42Z

tests/ui/traits/suggest-dereferences/root-obligation.stderr

@@ -1,12 +1,11 @@
-error[E0277]: expected a `Fn(char)` closure, found `char`
+error[E0277]: the trait bound `&char: Pattern<'_>` is not satisfied


This error, in the future (not this PR) it should actually be:

error[E0277]: the trait bound `&char: Pattern<'_>` is not satisfied --> $DIR/root-obligation.rs:6:38 | LL | .filter(|c| "aeiou".contains(c)) | -------- ^ the trait `Pattern<'_>` is not implemented for `&char` | | | required by a bound introduced by this call | note: required by a bound in `core::str::contains` --> $SRC_DIR/core/src/str/mod.rs:LL:COL help: consider dereferencing here | LL | .filter(|c| "aeiou".contains(*c)) | +

instead of

error[E0277]: the trait bound `&char: Pattern<'_>` is not satisfied --> $DIR/root-obligation.rs:6:38 | LL | .filter(|c| "aeiou".contains(c)) | -------- ^ the trait `Fn<(char,)>` is not implemented for `&char`, which is required by `&char: Pattern<'_>` | | | required by a bound introduced by this call | = note: required for `&char` to implement `FnOnce<(char,)>` = note: required for `&char` to implement `Pattern<'_>` note: required by a bound in `core::str::<impl str>::contains` --> $SRC_DIR/core/src/str/mod.rs:LL:COL help: consider dereferencing here | LL | .filter(|c| "aeiou".contains(*c)) | +

oli-obk · 2024-03-01T22:07:53Z

compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs

+                            && (
+                                // `T: Trait` && `&&T: OtherTrait`, we want `OtherTrait`
+                                trait_predicate.self_ty().skip_binder()
+                                    == root_pred.self_ty().peel_refs()


is it intended that we treat types with lifetimes differently if their lifetimes differ? We'll just have fewer cases where we switch to the root obligation, but I'm not sure whether it'll miss out on cases where we'd want to use the root.

I'll give it a more thorough look next week, but if you have ideas here, maybe add some tests where the self type has lifetimes

We could use can_eq instead here.

Yea good point. If that shows some tests that are affected, good, otherwise we need to write some

None of new tests were affected (after I checked for no escaping vars). I'll have to think of a case where lifetime divergence is the issue but that doesn't manifest as an OutlivesPredicate clause.

fmease · 2024-03-02T08:54:05Z

tests/ui/associated-types/substs-ppaux.normal.stderr

  --> $DIR/substs-ppaux.rs:49:6
   |
 LL |     <str as Foo<u8>>::bar;
-   |      ^^^ doesn't have a size known at compile-time
+   |      ^^^ the trait `Sized` is not implemented for `str`, which is required by `str: Foo<'_, '_, u8>`


I haven't read the implementation or the most recent Zulip comments and just saw this stderr diff, so sorry if this has been discussed already.

Is this intentional? Shouldn't we still use the #[rustc_on_unimplemented]/#[diagnostic::on_unimplemented] message here despite the derived/root obligation changes?

It is somewhat intentional. When you have T: Sized then you want T to have a compile time known size. When you have T: Foo, Foo: Sized, I think it makes sense to go back to "T does not implement Sized", it's a subtle difference of "what does the user care about".

Also, I do want to do a full refactor of this logic, too many ad-hoc hacks have piled beyond maintainability (in no small part due to me), but now that we know what the output we want to keep and change is, it will be easier to clean this up in a way that makes sense. This PR is about putting the output piece closer to that "ideal end state" in anticipation of that work.

bors · 2024-03-02T14:35:25Z

☔ The latest upstream changes (presumably #121890) made this pull request unmergeable. Please resolve the merge conflicts.

When encountering trait bound errors that satisfy some heuristics that tell us that the relevant trait for the user comes from the root obligation and not the current obligation, we use the root predicate for the main message. This allows to talk about "X doesn't implement Pattern<'_>" over the most specific case that just happened to fail, like "char doesn't implement Fn(&mut char)" in `tests/ui/traits/suggest-dereferences/root-obligation.rs` The heuristics are: - the type of the leaf predicate is (roughly) the same as the type from the root predicate, as a proxy for "we care about the root" - the leaf trait and the root trait are different, so as to avoid talking about `&mut T: Trait` and instead remain talking about `T: Trait` instead - the root trait is not `Unsize`, as to avoid talking about it in `tests/ui/coercion/coerce-issue-49593-box-never.rs`. ``` error[E0277]: the trait bound `&char: Pattern<'_>` is not satisfied --> $DIR/root-obligation.rs:6:38 | LL | .filter(|c| "aeiou".contains(c)) | -------- ^ the trait `Fn<(char,)>` is not implemented for `&char`, which is required by `&char: Pattern<'_>` | | | required by a bound introduced by this call | = note: required for `&char` to implement `FnOnce<(char,)>` = note: required for `&char` to implement `Pattern<'_>` note: required by a bound in `core::str::<impl str>::contains` --> $SRC_DIR/core/src/str/mod.rs:LL:COL help: consider dereferencing here | LL | .filter(|c| "aeiou".contains(*c)) | + ``` Fix rust-lang#79359, fix rust-lang#119983, fix rust-lang#118779, cc rust-lang#118415 (the suggestion needs to change).

… methods on non-`Iterator` ``` error[E0599]: no method named `map` found for struct `Vec<bool>` in the current scope --> $DIR/vec-on-unimplemented.rs:3:23 | LL | vec![true, false].map(|v| !v).collect::<Vec<_>>(); | ^^^ `Vec<bool>` is not an iterator | help: call `.into_iter()` first | LL | vec![true, false].into_iter().map(|v| !v).collect::<Vec<_>>(); | ++++++++++++ ``` We used to provide some help through `rustc_on_unimplemented` on non-`impl Trait` and non-type-params, but this lets us get rid of some otherwise unnecessary conditions in the annotation on `Iterator`.

oli-obk · 2024-03-04T10:52:59Z

@bors r+

bors · 2024-03-04T10:53:03Z

📌 Commit 40f9dcc has been approved by oli-obk

It is now in the queue for this repository.

…r=oli-obk Use root obligation on E0277 for some cases When encountering trait bound errors that satisfy some heuristics that tell us that the relevant trait for the user comes from the root obligation and not the current obligation, we use the root predicate for the main message. This allows to talk about "X doesn't implement Pattern<'_>" over the most specific case that just happened to fail, like "char doesn't implement Fn(&mut char)" in `tests/ui/traits/suggest-dereferences/root-obligation.rs` The heuristics are: - the type of the leaf predicate is (roughly) the same as the type from the root predicate, as a proxy for "we care about the root" - the leaf trait and the root trait are different, so as to avoid talking about `&mut T: Trait` and instead remain talking about `T: Trait` instead - the root trait is not `Unsize`, as to avoid talking about it in `tests/ui/coercion/coerce-issue-49593-box-never.rs`. ``` error[E0277]: the trait bound `&char: Pattern<'_>` is not satisfied --> $DIR/root-obligation.rs:6:38 | LL | .filter(|c| "aeiou".contains(c)) | -------- ^ the trait `Fn<(char,)>` is not implemented for `&char`, which is required by `&char: Pattern<'_>` | | | required by a bound introduced by this call | = note: required for `&char` to implement `FnOnce<(char,)>` = note: required for `&char` to implement `Pattern<'_>` note: required by a bound in `core::str::<impl str>::contains` --> $SRC_DIR/core/src/str/mod.rs:LL:COL help: consider dereferencing here | LL | .filter(|c| "aeiou".contains(*c)) | + ``` Fix rust-lang#79359, fix rust-lang#119983, fix rust-lang#118779, cc rust-lang#118415 (the suggestion needs to change), cc rust-lang#121398 (doesn't fix the underlying issue).

…llaumeGomez Rollup of 9 pull requests Successful merges: - rust-lang#120976 (constify a couple thread_local statics) - rust-lang#121576 (Convert the rest of the visitors to use `VisitorResult`) - rust-lang#121826 (Use root obligation on E0277 for some cases) - rust-lang#121928 (Extract an arguments struct for `Builder::then_else_break`) - rust-lang#121958 (Fix redundant import errors for preload extern crate) - rust-lang#121959 (Removing absolute path in proc-macro) - rust-lang#121968 (Don't run test_get_os_named_thread on win7) - rust-lang#121977 (Doc: Fix incorrect reference to integer in Atomic{Ptr,Bool}::as_ptr.) - rust-lang#121978 (Fix duplicated path in the "not found dylib" error) r? `@ghost` `@rustbot` modify labels: rollup

…r=oli-obk Use root obligation on E0277 for some cases When encountering trait bound errors that satisfy some heuristics that tell us that the relevant trait for the user comes from the root obligation and not the current obligation, we use the root predicate for the main message. This allows to talk about "X doesn't implement Pattern<'_>" over the most specific case that just happened to fail, like "char doesn't implement Fn(&mut char)" in `tests/ui/traits/suggest-dereferences/root-obligation.rs` The heuristics are: - the type of the leaf predicate is (roughly) the same as the type from the root predicate, as a proxy for "we care about the root" - the leaf trait and the root trait are different, so as to avoid talking about `&mut T: Trait` and instead remain talking about `T: Trait` instead - the root trait is not `Unsize`, as to avoid talking about it in `tests/ui/coercion/coerce-issue-49593-box-never.rs`. ``` error[E0277]: the trait bound `&char: Pattern<'_>` is not satisfied --> $DIR/root-obligation.rs:6:38 | LL | .filter(|c| "aeiou".contains(c)) | -------- ^ the trait `Fn<(char,)>` is not implemented for `&char`, which is required by `&char: Pattern<'_>` | | | required by a bound introduced by this call | = note: required for `&char` to implement `FnOnce<(char,)>` = note: required for `&char` to implement `Pattern<'_>` note: required by a bound in `core::str::<impl str>::contains` --> $SRC_DIR/core/src/str/mod.rs:LL:COL help: consider dereferencing here | LL | .filter(|c| "aeiou".contains(*c)) | + ``` Fix rust-lang#79359, fix rust-lang#119983, fix rust-lang#118779, cc rust-lang#118415 (the suggestion needs to change), cc rust-lang#121398 (doesn't fix the underlying issue).

…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#121213 (Add an example to demonstrate how Rc::into_inner works) - rust-lang#121262 (Add vector time complexity) - rust-lang#121287 (Clarify/add `must_use` message for Rc/Arc/Weak::into_raw.) - rust-lang#121664 (Adjust error `yield`/`await` lowering) - rust-lang#121826 (Use root obligation on E0277 for some cases) - rust-lang#121838 (Use the correct logic for nested impl trait in assoc types) - rust-lang#121913 (Don't panic when waiting on poisoned queries) - rust-lang#121987 (pattern analysis: abort on arity mismatch) - rust-lang#121993 (Avoid using unnecessary queries when printing the query stack in panics) - rust-lang#121997 (interpret/cast: make more matches on FloatTy properly exhaustive) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#121826 - estebank:e0277-root-obligation-2, r=oli-obk Use root obligation on E0277 for some cases When encountering trait bound errors that satisfy some heuristics that tell us that the relevant trait for the user comes from the root obligation and not the current obligation, we use the root predicate for the main message. This allows to talk about "X doesn't implement Pattern<'_>" over the most specific case that just happened to fail, like "char doesn't implement Fn(&mut char)" in `tests/ui/traits/suggest-dereferences/root-obligation.rs` The heuristics are: - the type of the leaf predicate is (roughly) the same as the type from the root predicate, as a proxy for "we care about the root" - the leaf trait and the root trait are different, so as to avoid talking about `&mut T: Trait` and instead remain talking about `T: Trait` instead - the root trait is not `Unsize`, as to avoid talking about it in `tests/ui/coercion/coerce-issue-49593-box-never.rs`. ``` error[E0277]: the trait bound `&char: Pattern<'_>` is not satisfied --> $DIR/root-obligation.rs:6:38 | LL | .filter(|c| "aeiou".contains(c)) | -------- ^ the trait `Fn<(char,)>` is not implemented for `&char`, which is required by `&char: Pattern<'_>` | | | required by a bound introduced by this call | = note: required for `&char` to implement `FnOnce<(char,)>` = note: required for `&char` to implement `Pattern<'_>` note: required by a bound in `core::str::<impl str>::contains` --> $SRC_DIR/core/src/str/mod.rs:LL:COL help: consider dereferencing here | LL | .filter(|c| "aeiou".contains(*c)) | + ``` Fix rust-lang#79359, fix rust-lang#119983, fix rust-lang#118779, cc rust-lang#118415 (the suggestion needs to change), cc rust-lang#121398 (doesn't fix the underlying issue).

…=jackh726 Harmonize using root or leaf obligation in trait error reporting When rust-lang#121826 changed the error reporting to use root obligation and not the leafmost obligation, it didn't actually make sure that all the other diagnostics helper functions used the right obligation. Specifically, when reporting similar impl candidates we are looking for impls of the root obligation, but trying to match them against the trait ref of the leaf obligation. This does a few other miscellaneous changes. There's a lot more clean-up that could be done here, but working with this code is really grief-inducing due to how messy it has become over the years. Someone really needs to show it love. 😓 r? `@estebank` Fixes rust-lang#126129

…=jackh726 Harmonize using root or leaf obligation in trait error reporting When rust-lang#121826 changed the error reporting to use root obligation and not the leafmost obligation, it didn't actually make sure that all the other diagnostics helper functions used the right obligation. Specifically, when reporting similar impl candidates we are looking for impls of the root obligation, but trying to match them against the trait ref of the leaf obligation. This does a few other miscellaneous changes. There's a lot more clean-up that could be done here, but working with this code is really grief-inducing due to how messy it has become over the years. Someone really needs to show it love. 😓 r? ``@estebank`` Fixes rust-lang#126129

Rollup merge of rust-lang#126142 - compiler-errors:trait-ref-split, r=jackh726 Harmonize using root or leaf obligation in trait error reporting When rust-lang#121826 changed the error reporting to use root obligation and not the leafmost obligation, it didn't actually make sure that all the other diagnostics helper functions used the right obligation. Specifically, when reporting similar impl candidates we are looking for impls of the root obligation, but trying to match them against the trait ref of the leaf obligation. This does a few other miscellaneous changes. There's a lot more clean-up that could be done here, but working with this code is really grief-inducing due to how messy it has become over the years. Someone really needs to show it love. 😓 r? ``@estebank`` Fixes rust-lang#126129

rustbot assigned pnkfelix Mar 1, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 1, 2024

oli-obk reviewed Mar 1, 2024

View reviewed changes

library/core/src/iter/traits/collect.rs Show resolved Hide resolved

oli-obk reviewed Mar 1, 2024

View reviewed changes

compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs Outdated Show resolved Hide resolved

estebank commented Mar 1, 2024

View reviewed changes

estebank force-pushed the e0277-root-obligation-2 branch 2 times, most recently from 9c8f2b9 to ccb8b8a Compare March 1, 2024 18:37

This comment has been minimized.

Sign in to view

estebank force-pushed the e0277-root-obligation-2 branch 2 times, most recently from 72ee35a to 6022301 Compare March 1, 2024 21:38

oli-obk reviewed Mar 1, 2024

View reviewed changes

fmease reviewed Mar 2, 2024

View reviewed changes

estebank added 3 commits March 3, 2024 18:53

Use can_eq instead of Ty<'_> == Ty<'_>

40f9dcc

estebank force-pushed the e0277-root-obligation-2 branch from 82f92d8 to 40f9dcc Compare March 3, 2024 18:59

estebank mentioned this pull request Mar 3, 2024

Don't Add Specialized Notes to Error Messages Pointing at a Type #121717

Closed

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2024

GuillaumeGomez mentioned this pull request Mar 4, 2024

Rollup of 9 pull requests #121982

Closed

This was referenced Mar 5, 2024

Rollup of 10 pull requests #122011

Closed

Rollup of 10 pull requests #122012

Merged

bors merged commit 35f6eee into rust-lang:master Mar 5, 2024
11 checks passed

rustbot added this to the 1.78.0 milestone Mar 5, 2024

compiler-errors mentioned this pull request Jun 7, 2024

Harmonize using root or leaf obligation in trait error reporting #126142

Merged

matthiaskrgr mentioned this pull request Jun 9, 2024

Relating different kinds: '?1 Bar #126129

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use root obligation on E0277 for some cases #121826

Use root obligation on E0277 for some cases #121826

estebank commented Mar 1, 2024 •

edited

Loading

rustbot commented Mar 1, 2024

estebank commented Mar 1, 2024

estebank Mar 1, 2024

This comment has been minimized.

oli-obk Mar 1, 2024

estebank Mar 1, 2024

oli-obk Mar 1, 2024

estebank Mar 3, 2024

fmease Mar 2, 2024 •

edited

Loading

estebank Mar 2, 2024

estebank Mar 2, 2024

bors commented Mar 2, 2024

oli-obk commented Mar 4, 2024

bors commented Mar 4, 2024

		@@ -1,12 +1,11 @@
		error[E0277]: expected a `Fn(char)` closure, found `char`
		error[E0277]: the trait bound `&char: Pattern<'_>` is not satisfied

Use root obligation on E0277 for some cases #121826

Use root obligation on E0277 for some cases #121826

Conversation

estebank commented Mar 1, 2024 • edited Loading

rustbot commented Mar 1, 2024

estebank commented Mar 1, 2024

estebank Mar 1, 2024

Choose a reason for hiding this comment

This comment has been minimized.

oli-obk Mar 1, 2024

Choose a reason for hiding this comment

estebank Mar 1, 2024

Choose a reason for hiding this comment

oli-obk Mar 1, 2024

Choose a reason for hiding this comment

estebank Mar 3, 2024

Choose a reason for hiding this comment

fmease Mar 2, 2024 • edited Loading

Choose a reason for hiding this comment

estebank Mar 2, 2024

Choose a reason for hiding this comment

estebank Mar 2, 2024

Choose a reason for hiding this comment

bors commented Mar 2, 2024

oli-obk commented Mar 4, 2024

bors commented Mar 4, 2024

estebank commented Mar 1, 2024 •

edited

Loading

fmease Mar 2, 2024 •

edited

Loading