Skip to content

Commit

Permalink
Rollup merge of rust-lang#126142 - compiler-errors:trait-ref-split, r…
Browse files Browse the repository at this point in the history
…=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
  • Loading branch information
compiler-errors committed Jun 13, 2024
2 parents 0e30c7d + c90b1fc commit d4d7cc3
Show file tree
Hide file tree
Showing 23 changed files with 153 additions and 110 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ impl<T> Trait<T> for X {
for pred in hir_generics.bounds_for_param(def_id) {
if self.constrain_generic_bound_associated_type_structured_suggestion(
diag,
&trait_ref,
trait_ref,
pred.bounds,
assoc,
assoc_args,
Expand Down Expand Up @@ -715,7 +715,7 @@ fn foo(&self) -> Self::T { String::new() }

self.constrain_generic_bound_associated_type_structured_suggestion(
diag,
&trait_ref,
trait_ref,
opaque_hir_ty.bounds,
assoc,
assoc_args,
Expand Down Expand Up @@ -869,7 +869,7 @@ fn foo(&self) -> Self::T { String::new() }
fn constrain_generic_bound_associated_type_structured_suggestion(
&self,
diag: &mut Diag<'_>,
trait_ref: &ty::TraitRef<'tcx>,
trait_ref: ty::TraitRef<'tcx>,
bounds: hir::GenericBounds<'_>,
assoc: ty::AssocItem,
assoc_args: &[ty::GenericArg<'tcx>],
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_infer/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,7 @@ impl<'tcx, O: Elaboratable<'tcx>> Elaborator<'tcx, O> {
let obligations =
predicates.predicates.iter().enumerate().map(|(index, &(clause, span))| {
elaboratable.child_with_derived_cause(
clause
.instantiate_supertrait(tcx, &bound_clause.rebind(data.trait_ref)),
clause.instantiate_supertrait(tcx, bound_clause.rebind(data.trait_ref)),
span,
bound_clause.rebind(data),
index,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl<'tcx> Elaborator<'tcx> {
let super_predicates =
self.tcx.super_predicates_of(trait_ref.def_id()).predicates.iter().filter_map(
|&(pred, _)| {
let clause = pred.instantiate_supertrait(self.tcx, &trait_ref);
let clause = pred.instantiate_supertrait(self.tcx, trait_ref);
self.visited.insert(clause).then_some(clause)
},
);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/predicate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ impl<'tcx> Clause<'tcx> {
pub fn instantiate_supertrait(
self,
tcx: TyCtxt<'tcx>,
trait_ref: &ty::PolyTraitRef<'tcx>,
trait_ref: ty::PolyTraitRef<'tcx>,
) -> Clause<'tcx> {
// The interaction between HRTB and supertraits is not entirely
// obvious. Let me walk you (and myself) through an example.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3597,7 +3597,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
&self,
obligation: &PredicateObligation<'tcx>,
err: &mut Diag<'_>,
trait_ref: &ty::PolyTraitRef<'tcx>,
trait_ref: ty::PolyTraitRef<'tcx>,
) {
let rhs_span = match obligation.cause.code() {
ObligationCauseCode::BinOp { rhs_span: Some(span), rhs_is_lit, .. } if *rhs_is_lit => {
Expand Down Expand Up @@ -4863,14 +4863,13 @@ impl<'a, 'hir> hir::intravisit::Visitor<'hir> for ReplaceImplTraitVisitor<'a> {
pub(super) fn get_explanation_based_on_obligation<'tcx>(
tcx: TyCtxt<'tcx>,
obligation: &PredicateObligation<'tcx>,
trait_ref: ty::PolyTraitRef<'tcx>,
trait_predicate: &ty::PolyTraitPredicate<'tcx>,
trait_predicate: ty::PolyTraitPredicate<'tcx>,
pre_message: String,
) -> String {
if let ObligationCauseCode::MainFunctionType = obligation.cause.code() {
"consider using `()`, or a `Result`".to_owned()
} else {
let ty_desc = match trait_ref.skip_binder().self_ty().kind() {
let ty_desc = match trait_predicate.self_ty().skip_binder().kind() {
ty::FnDef(_, _) => Some("fn item"),
ty::Closure(_, _) => Some("closure"),
_ => None,
Expand All @@ -4895,7 +4894,7 @@ pub(super) fn get_explanation_based_on_obligation<'tcx>(
format!(
"{pre_message}the trait `{}` is not implemented for{desc} `{}`{post}",
trait_predicate.print_modifiers_and_trait_path(),
tcx.short_ty_string(trait_ref.skip_binder().self_ty(), &mut None),
tcx.short_ty_string(trait_predicate.self_ty().skip_binder(), &mut None),
)
} else {
// "the trait bound `T: !Send` is not satisfied" reads better than "`!Send` is
Expand Down

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/object_safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ fn predicates_reference_self(
predicates
.predicates
.iter()
.map(|&(predicate, sp)| (predicate.instantiate_supertrait(tcx, &trait_ref), sp))
.map(|&(predicate, sp)| (predicate.instantiate_supertrait(tcx, trait_ref), sp))
.filter_map(|predicate| predicate_references_self(tcx, predicate))
.collect()
}
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1866,7 +1866,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
// the param_env so that it can be given the lowest priority. See
// #50825 for the motivation for this.
let is_global =
|cand: &ty::PolyTraitPredicate<'tcx>| cand.is_global() && !cand.has_bound_vars();
|cand: ty::PolyTraitPredicate<'tcx>| cand.is_global() && !cand.has_bound_vars();

// (*) Prefer `BuiltinCandidate { has_nested: false }`, `PointeeCandidate`,
// `DiscriminantKindCandidate`, `ConstDestructCandidate`
Expand Down Expand Up @@ -1909,7 +1909,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
}

(
ParamCandidate(ref other_cand),
ParamCandidate(other_cand),
ImplCandidate(..)
| AutoImplCandidate
| ClosureCandidate { .. }
Expand All @@ -1934,12 +1934,12 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
//
// Global bounds from the where clause should be ignored
// here (see issue #50825).
DropVictim::drop_if(!is_global(other_cand))
DropVictim::drop_if(!is_global(*other_cand))
}
(ObjectCandidate(_) | ProjectionCandidate(_), ParamCandidate(ref victim_cand)) => {
(ObjectCandidate(_) | ProjectionCandidate(_), ParamCandidate(victim_cand)) => {
// Prefer these to a global where-clause bound
// (see issue #50825).
if is_global(victim_cand) { DropVictim::Yes } else { DropVictim::No }
if is_global(*victim_cand) { DropVictim::Yes } else { DropVictim::No }
}
(
ImplCandidate(_)
Expand All @@ -1957,12 +1957,12 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
| TraitUpcastingUnsizeCandidate(_)
| BuiltinCandidate { has_nested: true }
| TraitAliasCandidate,
ParamCandidate(ref victim_cand),
ParamCandidate(victim_cand),
) => {
// Prefer these to a global where-clause bound
// (see issue #50825).
DropVictim::drop_if(
is_global(victim_cand) && other.evaluation.must_apply_modulo_regions(),
is_global(*victim_cand) && other.evaluation.must_apply_modulo_regions(),
)
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ impl<'tcx> TraitAliasExpander<'tcx> {
debug!(?predicates);

let items = predicates.predicates.iter().rev().filter_map(|(pred, span)| {
pred.instantiate_supertrait(tcx, &trait_ref)
pred.instantiate_supertrait(tcx, trait_ref)
.as_trait_clause()
.map(|trait_ref| item.clone_and_push(trait_ref.map_bound(|t| t.trait_ref), *span))
});
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/vtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ fn prepare_vtable_segments_inner<'tcx, T>(
.predicates
.into_iter()
.filter_map(move |(pred, _)| {
pred.instantiate_supertrait(tcx, &inner_most_trait_ref).as_trait_clause()
pred.instantiate_supertrait(tcx, inner_most_trait_ref).as_trait_clause()
});

// Find an unvisited supertrait
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0277]: the trait bound `&'static u32: Defaulted` is not satisfied
--> $DIR/typeck-default-trait-impl-precedence.rs:19:20
|
LL | is_defaulted::<&'static u32>();
| ^^^^^^^^^^^^ the trait `Signed` is not implemented for `&'static u32`, which is required by `&'static u32: Defaulted`
| ^^^^^^^^^^^^ the trait `Signed` is not implemented for `u32`, which is required by `&'static u32: Defaulted`
|
note: required for `&'static u32` to implement `Defaulted`
--> $DIR/typeck-default-trait-impl-precedence.rs:10:19
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0277]: the trait bound `i32: Baz<Self>` is not satisfied
--> $DIR/assume-gat-normalization-for-nested-goals.rs:9:30
|
LL | type Bar<T>: Baz<Self> = i32;
| ^^^ the trait `Eq<i32>` is not implemented for `i32`, which is required by `i32: Baz<Self>`
| ^^^ the trait `Eq<i32>` is not implemented for `<Self as Foo>::Bar<()>`, which is required by `i32: Baz<Self>`
|
note: required for `i32` to implement `Baz<Self>`
--> $DIR/assume-gat-normalization-for-nested-goals.rs:16:23
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/kindck/kindck-send-object.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0277]: `&'static (dyn Dummy + 'static)` cannot be sent between threads sa
LL | assert_send::<&'static (dyn Dummy + 'static)>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `&'static (dyn Dummy + 'static)` cannot be sent between threads safely
|
= help: the trait `Sync` is not implemented for `&'static (dyn Dummy + 'static)`, which is required by `&'static (dyn Dummy + 'static): Send`
= help: the trait `Sync` is not implemented for `(dyn Dummy + 'static)`, which is required by `&'static (dyn Dummy + 'static): Send`
= note: required for `&'static (dyn Dummy + 'static)` to implement `Send`
note: required by a bound in `assert_send`
--> $DIR/kindck-send-object.rs:5:18
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/kindck/kindck-send-object1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0277]: `&'a (dyn Dummy + 'a)` cannot be sent between threads safely
LL | assert_send::<&'a dyn Dummy>();
| ^^^^^^^^^^^^^ `&'a (dyn Dummy + 'a)` cannot be sent between threads safely
|
= help: the trait `Sync` is not implemented for `&'a (dyn Dummy + 'a)`, which is required by `&'a (dyn Dummy + 'a): Send`
= help: the trait `Sync` is not implemented for `(dyn Dummy + 'a)`, which is required by `&'a (dyn Dummy + 'a): Send`
= note: required for `&'a (dyn Dummy + 'a)` to implement `Send`
note: required by a bound in `assert_send`
--> $DIR/kindck-send-object1.rs:5:18
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/kindck/kindck-send-object2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0277]: `&'static (dyn Dummy + 'static)` cannot be sent between threads sa
LL | assert_send::<&'static dyn Dummy>();
| ^^^^^^^^^^^^^^^^^^ `&'static (dyn Dummy + 'static)` cannot be sent between threads safely
|
= help: the trait `Sync` is not implemented for `&'static (dyn Dummy + 'static)`, which is required by `&'static (dyn Dummy + 'static): Send`
= help: the trait `Sync` is not implemented for `(dyn Dummy + 'static)`, which is required by `&'static (dyn Dummy + 'static): Send`
= note: required for `&'static (dyn Dummy + 'static)` to implement `Send`
note: required by a bound in `assert_send`
--> $DIR/kindck-send-object2.rs:3:18
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/suggestions/suggest-remove-refs-5.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0277]: `&mut &mut &mut &mut Vec<i32>` is not an iterator
LL | for _ in &mut &mut v {}
| ^^^^^^^^^^^ `&mut &mut &mut &mut Vec<i32>` is not an iterator
|
= help: the trait `Iterator` is not implemented for `&mut &mut &mut &mut Vec<i32>`, which is required by `&mut &mut &mut &mut Vec<i32>: IntoIterator`
= help: the trait `Iterator` is not implemented for `Vec<i32>`, which is required by `&mut &mut &mut &mut Vec<i32>: IntoIterator`
= note: required for `&mut Vec<i32>` to implement `Iterator`
= note: 3 redundant requirements hidden
= note: required for `&mut &mut &mut &mut Vec<i32>` to implement `Iterator`
Expand All @@ -21,7 +21,7 @@ error[E0277]: `&mut &mut &mut [u8; 1]` is not an iterator
LL | for _ in &mut v {}
| ^^^^^^ `&mut &mut &mut [u8; 1]` is not an iterator
|
= help: the trait `Iterator` is not implemented for `&mut &mut &mut [u8; 1]`, which is required by `&mut &mut &mut [u8; 1]: IntoIterator`
= help: the trait `Iterator` is not implemented for `[u8; 1]`, which is required by `&mut &mut &mut [u8; 1]: IntoIterator`
= note: required for `&mut [u8; 1]` to implement `Iterator`
= note: 2 redundant requirements hidden
= note: required for `&mut &mut &mut [u8; 1]` to implement `Iterator`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error: future cannot be sent between threads safely
LL | is_send(foo());
| ^^^^^ future returned by `foo` is not `Send`
|
= help: the trait `Sync` is not implemented for `impl Future<Output = ()>`, which is required by `impl Future<Output = ()>: Send`
= help: the trait `Sync` is not implemented for `NotSync`, which is required by `impl Future<Output = ()>: Send`
note: future is not `Send` as this value is used across an await
--> $DIR/auto-with-drop_tracking_mir.rs:16:11
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ error[E0277]: the trait bound `(): Foo` is not satisfied
LL | needs_foo::<()>();
| ^^ the trait `Bar` is not implemented for `()`, which is required by `(): Foo`
|
help: this trait has no implementations, consider adding one
--> $DIR/point-at-failing-nested.rs:4:1
|
LL | trait Bar {}
| ^^^^^^^^^
note: required for `()` to implement `Foo`
--> $DIR/point-at-failing-nested.rs:9:12
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ LL | needs_foo(());
| |
| required by a bound introduced by this call
|
help: this trait has no implementations, consider adding one
--> $DIR/where-clause-doesnt-apply.rs:2:1
|
LL | trait Bar {}
| ^^^^^^^^^
note: required for `()` to implement `Foo`
--> $DIR/where-clause-doesnt-apply.rs:4:9
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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<'_>`
| -------- ^ the trait `Fn(char)` is not implemented for `char`, which is required by `&char: Pattern<'_>`
| |
| required by a bound introduced by this call
|
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/transmutability/references/unsafecell.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0277]: `&u8` cannot be safely transmuted into `&UnsafeCell<u8>`
--> $DIR/unsafecell.rs:27:50
|
LL | assert::is_maybe_transmutable::<&'static u8, &'static UnsafeCell<u8>>();
| ^^^^^^^^^^^^^^^^^^^^^^^ the trait `Freeze` is not implemented for `&'static UnsafeCell<u8>`
| ^^^^^^^^^^^^^^^^^^^^^^^ the trait `Freeze` is not implemented for `UnsafeCell<u8>`
|
note: required by a bound in `is_maybe_transmutable`
--> $DIR/unsafecell.rs:12:14
Expand All @@ -17,7 +17,7 @@ error[E0277]: `&UnsafeCell<u8>` cannot be safely transmuted into `&UnsafeCell<u8
--> $DIR/unsafecell.rs:29:62
|
LL | assert::is_maybe_transmutable::<&'static UnsafeCell<u8>, &'static UnsafeCell<u8>>();
| ^^^^^^^^^^^^^^^^^^^^^^^ the trait `Freeze` is not implemented for `&'static UnsafeCell<u8>`
| ^^^^^^^^^^^^^^^^^^^^^^^ the trait `Freeze` is not implemented for `UnsafeCell<u8>`
|
note: required by a bound in `is_maybe_transmutable`
--> $DIR/unsafecell.rs:12:14
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/typeck/suggest-similar-impls-for-root-obligation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
trait Foo<'s> {}

impl<'s> Foo<'s> for () {}

struct Bar;

impl<'s, T: Foo<'s>> From<T> for Bar {
fn from(_: T) -> Self {
Bar
}
}

fn main() {
let _: Bar = ((),).into();
//~^ ERROR he trait bound `((),): Into<Bar>` is not satisfied
}
20 changes: 20 additions & 0 deletions tests/ui/typeck/suggest-similar-impls-for-root-obligation.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error[E0277]: the trait bound `((),): Into<Bar>` is not satisfied
--> $DIR/suggest-similar-impls-for-root-obligation.rs:14:24
|
LL | let _: Bar = ((),).into();
| ^^^^ the trait `Foo<'_>` is not implemented for `((),)`, which is required by `((),): Into<_>`
|
= help: the trait `Foo<'_>` is implemented for `()`
= help: for that trait implementation, expected `()`, found `((),)`
note: required for `Bar` to implement `From<((),)>`
--> $DIR/suggest-similar-impls-for-root-obligation.rs:7:22
|
LL | impl<'s, T: Foo<'s>> From<T> for Bar {
| ------- ^^^^^^^ ^^^
| |
| unsatisfied trait bound introduced here
= note: required for `((),)` to implement `Into<Bar>`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0277`.

0 comments on commit d4d7cc3

Please sign in to comment.