Skip to content

Commit f5dc388

Browse files
estebankestebank
committed
Point at source of trait bound obligations in more places
Be more thorough in using `ItemObligation` and `BindingObligation` when evaluating obligations so that we can point at trait bounds that introduced unfulfilled obligations. We no longer incorrectly point at unrelated trait bounds (`substs-ppaux.verbose.stderr`). In particular, we now point at trait bounds on method calls. We no longer point at "obvious" obligation sources (we no longer have a note pointing at `Trait` saying "required by a bound in `Trait`", like in `associated-types-no-suitable-supertrait*`). Address part of rust-lang#89418.
1 parent faea820 commit f5dc388

File tree

101 files changed

+550
-421
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

101 files changed

+550
-421
lines changed

‎compiler/rustc_infer/src/infer/error_reporting/mod.rs

+12-3
Original file line numberDiff line numberDiff line change
@@ -2113,10 +2113,19 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
21132113
None
21142114
},
21152115
self.tcx.generics_of(owner.to_def_id()),
2116+
hir.span(hir_id),
21162117
)
21172118
});
2119+
2120+
let span = match generics {
2121+
// This is to get around the trait identity obligation, that has a `DUMMY_SP` as signal
2122+
// for other diagnostics, so we need to recover it here.
2123+
Some((_, _, node)) if span.is_dummy() => node,
2124+
_ => span,
2125+
};
2126+
21182127
let type_param_span = match (generics, bound_kind) {
2119-
(Some((_, ref generics)), GenericKind::Param(ref param)) => {
2128+
(Some((_, ref generics, _)), GenericKind::Param(ref param)) => {
21202129
// Account for the case where `param` corresponds to `Self`,
21212130
// which doesn't have the expected type argument.
21222131
if !(generics.has_self && param.index == 0) {
@@ -2153,7 +2162,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
21532162
};
21542163
let new_lt = generics
21552164
.as_ref()
2156-
.and_then(|(parent_g, g)| {
2165+
.and_then(|(parent_g, g, _)| {
21572166
let mut possible = (b'a'..=b'z').map(|c| format!("'{}", c as char));
21582167
let mut lts_names = g
21592168
.params
@@ -2175,7 +2184,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
21752184
.unwrap_or("'lt".to_string());
21762185
let add_lt_sugg = generics
21772186
.as_ref()
2178-
.and_then(|(_, g)| g.params.first())
2187+
.and_then(|(_, g, _)| g.params.first())
21792188
.and_then(|param| param.def_id.as_local())
21802189
.map(|def_id| {
21812190
(

‎compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -192,14 +192,16 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
192192
ObligationCauseCode::MatchImpl(parent, ..) => &parent.code,
193193
_ => &cause.code,
194194
};
195-
if let ObligationCauseCode::ItemObligation(item_def_id) = *code {
195+
if let (ObligationCauseCode::ItemObligation(item_def_id), None) =
196+
(code, override_error_code)
197+
{
196198
// Same case of `impl Foo for dyn Bar { fn qux(&self) {} }` introducing a `'static`
197199
// lifetime as above, but called using a fully-qualified path to the method:
198200
// `Foo::qux(bar)`.
199201
let mut v = TraitObjectVisitor(FxHashSet::default());
200202
v.visit_ty(param.param_ty);
201203
if let Some((ident, self_ty)) =
202-
self.get_impl_ident_and_self_ty_from_trait(item_def_id, &v.0)
204+
self.get_impl_ident_and_self_ty_from_trait(*item_def_id, &v.0)
203205
{
204206
if self.suggest_constrain_dyn_trait_in_impl(&mut err, &v.0, ident, self_ty) {
205207
override_error_code = Some(ident);

‎compiler/rustc_trait_selection/src/traits/util.rs

+13-6
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ use rustc_middle::ty::subst::{GenericArg, Subst, SubstsRef};
99
use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness};
1010

1111
use super::{Normalized, Obligation, ObligationCause, PredicateObligation, SelectionContext};
12-
pub use rustc_infer::traits::util::*;
12+
pub use rustc_infer::traits::{self, util::*};
13+
14+
use std::iter;
1315

1416
///////////////////////////////////////////////////////////////////////////
1517
// `TraitAliasExpander` iterator
@@ -229,11 +231,16 @@ pub fn predicates_for_generics<'tcx>(
229231
) -> impl Iterator<Item = PredicateObligation<'tcx>> {
230232
debug!("predicates_for_generics(generic_bounds={:?})", generic_bounds);
231233

232-
generic_bounds.predicates.into_iter().map(move |predicate| Obligation {
233-
cause: cause.clone(),
234-
recursion_depth,
235-
param_env,
236-
predicate,
234+
iter::zip(generic_bounds.predicates, generic_bounds.spans).map(move |(predicate, span)| {
235+
let cause = match cause.code {
236+
traits::ItemObligation(def_id) if !span.is_dummy() => traits::ObligationCause::new(
237+
cause.span,
238+
cause.body_id,
239+
traits::BindingObligation(def_id, span),
240+
),
241+
_ => cause.clone(),
242+
};
243+
Obligation { cause, recursion_depth, param_env, predicate }
237244
})
238245
}
239246

‎compiler/rustc_trait_selection/src/traits/wf.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -709,7 +709,12 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
709709

710710
iter::zip(iter::zip(predicates.predicates, predicates.spans), origins.into_iter().rev())
711711
.map(|((pred, span), origin_def_id)| {
712-
let cause = self.cause(traits::BindingObligation(origin_def_id, span));
712+
let code = if span.is_dummy() {
713+
traits::MiscObligation
714+
} else {
715+
traits::BindingObligation(origin_def_id, span)
716+
};
717+
let cause = self.cause(code);
713718
traits::Obligation::with_depth(cause, self.recursion_depth, self.param_env, pred)
714719
})
715720
.filter(|pred| !pred.has_escaping_bound_vars())

‎compiler/rustc_typeck/src/check/compare_method.rs

+7-6
Original file line numberDiff line numberDiff line change
@@ -1391,12 +1391,13 @@ pub fn check_type_bounds<'tcx>(
13911391

13921392
let impl_ty_hir_id = tcx.hir().local_def_id_to_hir_id(impl_ty.def_id.expect_local());
13931393
let normalize_cause = traits::ObligationCause::misc(impl_ty_span, impl_ty_hir_id);
1394-
let mk_cause = |span| {
1395-
ObligationCause::new(
1396-
impl_ty_span,
1397-
impl_ty_hir_id,
1398-
ObligationCauseCode::BindingObligation(trait_ty.def_id, span),
1399-
)
1394+
let mk_cause = |span: Span| {
1395+
let code = if span.is_dummy() {
1396+
traits::MiscObligation
1397+
} else {
1398+
traits::BindingObligation(trait_ty.def_id, span)
1399+
};
1400+
ObligationCause::new(impl_ty_span, impl_ty_hir_id, code)
14001401
};
14011402

14021403
let obligations = tcx

‎compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs

+4-42
Original file line numberDiff line numberDiff line change
@@ -586,38 +586,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
586586
}
587587
}
588588

589-
/// Given a fully substituted set of bounds (`generic_bounds`), and the values with which each
590-
/// type/region parameter was instantiated (`substs`), creates and registers suitable
591-
/// trait/region obligations.
592-
///
593-
/// For example, if there is a function:
594-
///
595-
/// ```
596-
/// fn foo<'a,T:'a>(...)
597-
/// ```
598-
///
599-
/// and a reference:
600-
///
601-
/// ```
602-
/// let f = foo;
603-
/// ```
604-
///
605-
/// Then we will create a fresh region variable `'$0` and a fresh type variable `$1` for `'a`
606-
/// and `T`. This routine will add a region obligation `$1:'$0` and register it locally.
607-
pub fn add_obligations_for_parameters(
608-
&self,
609-
cause: traits::ObligationCause<'tcx>,
610-
predicates: ty::InstantiatedPredicates<'tcx>,
611-
) {
612-
assert!(!predicates.has_escaping_bound_vars());
613-
614-
debug!("add_obligations_for_parameters(predicates={:?})", predicates);
615-
616-
for obligation in traits::predicates_for_generics(cause, self.param_env, predicates) {
617-
self.register_predicate(obligation);
618-
}
619-
}
620-
621589
// FIXME(arielb1): use this instead of field.ty everywhere
622590
// Only for fields! Returns <none> for methods>
623591
// Indifferent to privacy flags
@@ -1522,20 +1490,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
15221490

15231491
/// Add all the obligations that are required, substituting and normalized appropriately.
15241492
#[tracing::instrument(level = "debug", skip(self, span, def_id, substs))]
1525-
fn add_required_obligations(&self, span: Span, def_id: DefId, substs: &SubstsRef<'tcx>) {
1526-
let (bounds, spans) = self.instantiate_bounds(span, def_id, &substs);
1493+
crate fn add_required_obligations(&self, span: Span, def_id: DefId, substs: &SubstsRef<'tcx>) {
1494+
let (bounds, _) = self.instantiate_bounds(span, def_id, &substs);
15271495

1528-
for (i, mut obligation) in traits::predicates_for_generics(
1496+
for obligation in traits::predicates_for_generics(
15291497
traits::ObligationCause::new(span, self.body_id, traits::ItemObligation(def_id)),
15301498
self.param_env,
15311499
bounds,
1532-
)
1533-
.enumerate()
1534-
{
1535-
// This makes the error point at the bound, but we want to point at the argument
1536-
if let Some(span) = spans.get(i) {
1537-
obligation.cause.make_mut().code = traits::BindingObligation(def_id, *span);
1538-
}
1500+
) {
15391501
self.register_predicate(obligation);
15401502
}
15411503
}

‎compiler/rustc_typeck/src/check/fn_ctxt/checks.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -509,10 +509,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
509509
self.write_user_type_annotation_from_substs(hir_id, did, substs, None);
510510

511511
// Check bounds on type arguments used in the path.
512-
let (bounds, _) = self.instantiate_bounds(path_span, did, substs);
513-
let cause =
514-
traits::ObligationCause::new(path_span, self.body_id, traits::ItemObligation(did));
515-
self.add_obligations_for_parameters(cause, bounds);
512+
self.add_required_obligations(path_span, did, substs);
516513

517514
Some((variant, ty))
518515
} else {

‎compiler/rustc_typeck/src/check/method/confirm.rs

+18-6
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,12 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
120120
// We won't add these if we encountered an illegal sized bound, so that we can use
121121
// a custom error in that case.
122122
if illegal_sized_bound.is_none() {
123-
self.add_obligations(self.tcx.mk_fn_ptr(method_sig), all_substs, method_predicates);
123+
self.add_obligations(
124+
self.tcx.mk_fn_ptr(method_sig),
125+
all_substs,
126+
method_predicates,
127+
pick.item.def_id,
128+
);
124129
}
125130

126131
// Create the final `MethodCallee`.
@@ -471,16 +476,23 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
471476
fty: Ty<'tcx>,
472477
all_substs: SubstsRef<'tcx>,
473478
method_predicates: ty::InstantiatedPredicates<'tcx>,
479+
def_id: DefId,
474480
) {
475481
debug!(
476-
"add_obligations: fty={:?} all_substs={:?} method_predicates={:?}",
477-
fty, all_substs, method_predicates
482+
"add_obligations: fty={:?} all_substs={:?} method_predicates={:?} def_id={:?}",
483+
fty, all_substs, method_predicates, def_id
478484
);
479485

480-
self.add_obligations_for_parameters(
481-
traits::ObligationCause::misc(self.span, self.body_id),
486+
// FIXME: could replace with the following, but we already calculated `method_predicates`,
487+
// so we just call `predicates_for_generics` directly to avoid redoing work.
488+
// `self.add_required_obligations(self.span, def_id, &all_substs);`
489+
for obligation in traits::predicates_for_generics(
490+
traits::ObligationCause::new(self.span, self.body_id, traits::ItemObligation(def_id)),
491+
self.param_env,
482492
method_predicates,
483-
);
493+
) {
494+
self.register_predicate(obligation);
495+
}
484496

485497
// this is a projection from a trait reference, so we have to
486498
// make sure that the trait reference inputs are well-formed.

‎compiler/rustc_typeck/src/collect.rs

+6-10
Original file line numberDiff line numberDiff line change
@@ -1990,16 +1990,12 @@ fn predicates_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::GenericPredicates<'_> {
19901990
// prove that the trait applies to the types that were
19911991
// used, and adding the predicate into this list ensures
19921992
// that this is done.
1993-
let mut span = tcx.def_span(def_id);
1994-
if tcx.sess.source_map().is_local_span(span) {
1995-
// `guess_head_span` reads the actual source file from
1996-
// disk to try to determine the 'head' snippet of the span.
1997-
// Don't do this for a span that comes from a file outside
1998-
// of our crate, since this would make our query output
1999-
// (and overall crate metadata) dependent on the
2000-
// *current* state of an external file.
2001-
span = tcx.sess.source_map().guess_head_span(span);
2002-
}
1993+
//
1994+
// We use a DUMMY_SP here as a way to signal trait bounds that come
1995+
// from the trait itself that *shouldn't* be shown as the source of
1996+
// an obligation and instead be skipped. Otherwise we'd use
1997+
// `tcx.def_span(def_id);`
1998+
let span = rustc_span::DUMMY_SP;
20031999
result.predicates =
20042000
tcx.arena.alloc_from_iter(result.predicates.iter().copied().chain(std::iter::once((
20052001
ty::TraitRef::identity(tcx, def_id).without_const().to_predicate(tcx),

‎src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr

-29
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,6 @@ LL | type C: Clone + Iterator<Item: Send + Iterator<Item: for<'a> Lam<&'a u8
55
| ^^^^ `<<Self as Case1>::C as Iterator>::Item` cannot be sent between threads safely
66
|
77
= help: the trait `Send` is not implemented for `<<Self as Case1>::C as Iterator>::Item`
8-
note: required by a bound in `Send`
9-
--> $SRC_DIR/core/src/marker.rs:LL:COL
10-
|
11-
LL | / pub unsafe auto trait Send {
12-
LL | | // empty.
13-
LL | | }
14-
| |_^ required by this bound in `Send`
158
help: consider further restricting the associated type
169
|
1710
LL | trait Case1 where <<Self as Case1>::C as Iterator>::Item: Send {
@@ -24,17 +17,6 @@ LL | type C: Clone + Iterator<Item: Send + Iterator<Item: for<'a> Lam<&'a u8
2417
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `<<Self as Case1>::C as Iterator>::Item` is not an iterator
2518
|
2619
= help: the trait `Iterator` is not implemented for `<<Self as Case1>::C as Iterator>::Item`
27-
note: required by a bound in `Iterator`
28-
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
29-
|
30-
LL | / pub trait Iterator {
31-
LL | | /// The type of the elements being iterated over.
32-
LL | | #[stable(feature = "rust1", since = "1.0.0")]
33-
LL | | type Item;
34-
... |
35-
LL | | }
36-
LL | | }
37-
| |_^ required by this bound in `Iterator`
3820
help: consider further restricting the associated type
3921
|
4022
LL | trait Case1 where <<Self as Case1>::C as Iterator>::Item: Iterator {
@@ -47,17 +29,6 @@ LL | type C: Clone + Iterator<Item: Send + Iterator<Item: for<'a> Lam<&'a u8
4729
| ^^^^ `<<Self as Case1>::C as Iterator>::Item` cannot be shared between threads safely
4830
|
4931
= help: the trait `Sync` is not implemented for `<<Self as Case1>::C as Iterator>::Item`
50-
note: required by a bound in `Sync`
51-
--> $SRC_DIR/core/src/marker.rs:LL:COL
52-
|
53-
LL | / pub unsafe auto trait Sync {
54-
LL | | // FIXME(estebank): once support to add notes in `rustc_on_unimplemented`
55-
LL | | // lands in beta, and it has been extended to check whether a closure is
56-
LL | | // anywhere in the requirement chain, extend it as such (#48534):
57-
... |
58-
LL | | // Empty
59-
LL | | }
60-
| |_^ required by this bound in `Sync`
6132
help: consider further restricting the associated type
6233
|
6334
LL | trait Case1 where <<Self as Case1>::C as Iterator>::Item: Sync {

‎src/test/ui/associated-type-bounds/bounds-on-assoc-in-trait.stderr

-22
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,6 @@ LL | type A: Iterator<Item: Debug>;
55
| ^^^^^ `<<Self as Case1>::A as Iterator>::Item` cannot be formatted using `{:?}` because it doesn't implement `Debug`
66
|
77
= help: the trait `Debug` is not implemented for `<<Self as Case1>::A as Iterator>::Item`
8-
note: required by a bound in `Debug`
9-
--> $SRC_DIR/core/src/fmt/mod.rs:LL:COL
10-
|
11-
LL | / pub trait Debug {
12-
LL | | /// Formats the value using the given formatter.
13-
LL | | ///
14-
LL | | /// # Examples
15-
... |
16-
LL | | fn fmt(&self, f: &mut Formatter<'_>) -> Result;
17-
LL | | }
18-
| |_^ required by this bound in `Debug`
198
help: consider further restricting the associated type
209
|
2110
LL | trait Case1 where <<Self as Case1>::A as Iterator>::Item: Debug {
@@ -27,17 +16,6 @@ error[E0277]: the trait bound `<<Self as Foo>::Out as Baz>::Assoc: Default` is n
2716
LL | pub trait Foo { type Out: Baz<Assoc: Default>; }
2817
| ^^^^^^^ the trait `Default` is not implemented for `<<Self as Foo>::Out as Baz>::Assoc`
2918
|
30-
note: required by a bound in `Default`
31-
--> $SRC_DIR/core/src/default.rs:LL:COL
32-
|
33-
LL | / pub trait Default: Sized {
34-
LL | | /// Returns the "default value" for a type.
35-
LL | | ///
36-
LL | | /// Default values are often some kind of initial value, identity value, or anything else that
37-
... |
38-
LL | | fn default() -> Self;
39-
LL | | }
40-
| |_^ required by this bound in `Default`
4119
help: consider further restricting the associated type
4220
|
4321
LL | pub trait Foo where <<Self as Foo>::Out as Baz>::Assoc: Default { type Out: Baz<Assoc: Default>; }

‎src/test/ui/associated-types/associated-types-for-unimpl-trait.stderr

-5
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,6 @@ error[E0277]: the trait bound `Self: Get` is not satisfied
44
LL | fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) {}
55
| ^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `Self`
66
|
7-
note: required by a bound in `Get`
8-
--> $DIR/associated-types-for-unimpl-trait.rs:4:1
9-
|
10-
LL | trait Get {
11-
| ^^^^^^^^^ required by this bound in `Get`
127
help: consider further restricting `Self`
138
|
149
LL | fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) where Self: Get {}

‎src/test/ui/associated-types/associated-types-no-suitable-bound.stderr

-5
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,6 @@ error[E0277]: the trait bound `T: Get` is not satisfied
44
LL | fn uhoh<T>(foo: <T as Get>::Value) {}
55
| ^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `T`
66
|
7-
note: required by a bound in `Get`
8-
--> $DIR/associated-types-no-suitable-bound.rs:1:1
9-
|
10-
LL | trait Get {
11-
| ^^^^^^^^^ required by this bound in `Get`
127
help: consider restricting type parameter `T`
138
|
149
LL | fn uhoh<T: Get>(foo: <T as Get>::Value) {}

‎src/test/ui/associated-types/associated-types-no-suitable-supertrait-2.stderr

-5
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,6 @@ error[E0277]: the trait bound `Self: Get` is not satisfied
44
LL | fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) {}
55
| ^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `Self`
66
|
7-
note: required by a bound in `Get`
8-
--> $DIR/associated-types-no-suitable-supertrait-2.rs:12:1
9-
|
10-
LL | trait Get {
11-
| ^^^^^^^^^ required by this bound in `Get`
127
help: consider further restricting `Self`
138
|
149
LL | fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) where Self: Get {}

0 commit comments

Comments
 (0)