Skip to content

Commit

Permalink
Point at source of trait bound obligations in more places
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
estebank committed Nov 20, 2021
1 parent 93542a8 commit 6b9d910
Show file tree
Hide file tree
Showing 101 changed files with 550 additions and 421 deletions.
15 changes: 12 additions & 3 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2113,10 +2113,19 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
None
},
self.tcx.generics_of(owner.to_def_id()),
hir.span(hir_id),
)
});

let span = match generics {
// This is to get around the trait identity obligation, that has a `DUMMY_SP` as signal
// for other diagnostics, so we need to recover it here.
Some((_, _, node)) if span.is_dummy() => node,
_ => span,
};

let type_param_span = match (generics, bound_kind) {
(Some((_, ref generics)), GenericKind::Param(ref param)) => {
(Some((_, ref generics, _)), GenericKind::Param(ref param)) => {
// Account for the case where `param` corresponds to `Self`,
// which doesn't have the expected type argument.
if !(generics.has_self && param.index == 0) {
Expand Down Expand Up @@ -2153,7 +2162,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
};
let new_lt = generics
.as_ref()
.and_then(|(parent_g, g)| {
.and_then(|(parent_g, g, _)| {
let mut possible = (b'a'..=b'z').map(|c| format!("'{}", c as char));
let mut lts_names = g
.params
Expand All @@ -2175,7 +2184,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
.unwrap_or("'lt".to_string());
let add_lt_sugg = generics
.as_ref()
.and_then(|(_, g)| g.params.first())
.and_then(|(_, g, _)| g.params.first())
.and_then(|param| param.def_id.as_local())
.map(|def_id| {
(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,14 +192,16 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
ObligationCauseCode::MatchImpl(parent, ..) => &parent.code,
_ => &cause.code,
};
if let ObligationCauseCode::ItemObligation(item_def_id) = *code {
if let (ObligationCauseCode::ItemObligation(item_def_id), None) =
(code, override_error_code)
{
// Same case of `impl Foo for dyn Bar { fn qux(&self) {} }` introducing a `'static`
// lifetime as above, but called using a fully-qualified path to the method:
// `Foo::qux(bar)`.
let mut v = TraitObjectVisitor(FxHashSet::default());
v.visit_ty(param.param_ty);
if let Some((ident, self_ty)) =
self.get_impl_ident_and_self_ty_from_trait(item_def_id, &v.0)
self.get_impl_ident_and_self_ty_from_trait(*item_def_id, &v.0)
{
if self.suggest_constrain_dyn_trait_in_impl(&mut err, &v.0, ident, self_ty) {
override_error_code = Some(ident);
Expand Down
19 changes: 13 additions & 6 deletions compiler/rustc_trait_selection/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ use rustc_middle::ty::subst::{GenericArg, Subst, SubstsRef};
use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness};

use super::{Normalized, Obligation, ObligationCause, PredicateObligation, SelectionContext};
pub use rustc_infer::traits::util::*;
pub use rustc_infer::traits::{self, util::*};

use std::iter;

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

generic_bounds.predicates.into_iter().map(move |predicate| Obligation {
cause: cause.clone(),
recursion_depth,
param_env,
predicate,
iter::zip(generic_bounds.predicates, generic_bounds.spans).map(move |(predicate, span)| {
let cause = match cause.code {
traits::ItemObligation(def_id) if !span.is_dummy() => traits::ObligationCause::new(
cause.span,
cause.body_id,
traits::BindingObligation(def_id, span),
),
_ => cause.clone(),
};
Obligation { cause, recursion_depth, param_env, predicate }
})
}

Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_trait_selection/src/traits/wf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,12 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {

iter::zip(iter::zip(predicates.predicates, predicates.spans), origins.into_iter().rev())
.map(|((pred, span), origin_def_id)| {
let cause = self.cause(traits::BindingObligation(origin_def_id, span));
let code = if span.is_dummy() {
traits::MiscObligation
} else {
traits::BindingObligation(origin_def_id, span)
};
let cause = self.cause(code);
traits::Obligation::with_depth(cause, self.recursion_depth, self.param_env, pred)
})
.filter(|pred| !pred.has_escaping_bound_vars())
Expand Down
13 changes: 7 additions & 6 deletions compiler/rustc_typeck/src/check/compare_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1385,12 +1385,13 @@ pub fn check_type_bounds<'tcx>(

let impl_ty_hir_id = tcx.hir().local_def_id_to_hir_id(impl_ty.def_id.expect_local());
let normalize_cause = traits::ObligationCause::misc(impl_ty_span, impl_ty_hir_id);
let mk_cause = |span| {
ObligationCause::new(
impl_ty_span,
impl_ty_hir_id,
ObligationCauseCode::BindingObligation(trait_ty.def_id, span),
)
let mk_cause = |span: Span| {
let code = if span.is_dummy() {
traits::MiscObligation
} else {
traits::BindingObligation(trait_ty.def_id, span)
};
ObligationCause::new(impl_ty_span, impl_ty_hir_id, code)
};

let obligations = tcx
Expand Down
46 changes: 4 additions & 42 deletions compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -586,38 +586,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

/// Given a fully substituted set of bounds (`generic_bounds`), and the values with which each
/// type/region parameter was instantiated (`substs`), creates and registers suitable
/// trait/region obligations.
///
/// For example, if there is a function:
///
/// ```
/// fn foo<'a,T:'a>(...)
/// ```
///
/// and a reference:
///
/// ```
/// let f = foo;
/// ```
///
/// Then we will create a fresh region variable `'$0` and a fresh type variable `$1` for `'a`
/// and `T`. This routine will add a region obligation `$1:'$0` and register it locally.
pub fn add_obligations_for_parameters(
&self,
cause: traits::ObligationCause<'tcx>,
predicates: ty::InstantiatedPredicates<'tcx>,
) {
assert!(!predicates.has_escaping_bound_vars());

debug!("add_obligations_for_parameters(predicates={:?})", predicates);

for obligation in traits::predicates_for_generics(cause, self.param_env, predicates) {
self.register_predicate(obligation);
}
}

// FIXME(arielb1): use this instead of field.ty everywhere
// Only for fields! Returns <none> for methods>
// Indifferent to privacy flags
Expand Down Expand Up @@ -1522,20 +1490,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

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

for (i, mut obligation) in traits::predicates_for_generics(
for obligation in traits::predicates_for_generics(
traits::ObligationCause::new(span, self.body_id, traits::ItemObligation(def_id)),
self.param_env,
bounds,
)
.enumerate()
{
// This makes the error point at the bound, but we want to point at the argument
if let Some(span) = spans.get(i) {
obligation.cause.make_mut().code = traits::BindingObligation(def_id, *span);
}
) {
self.register_predicate(obligation);
}
}
Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,10 +509,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.write_user_type_annotation_from_substs(hir_id, did, substs, None);

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

Some((variant, ty))
} else {
Expand Down
24 changes: 18 additions & 6 deletions compiler/rustc_typeck/src/check/method/confirm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,12 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
// We won't add these if we encountered an illegal sized bound, so that we can use
// a custom error in that case.
if illegal_sized_bound.is_none() {
self.add_obligations(self.tcx.mk_fn_ptr(method_sig), all_substs, method_predicates);
self.add_obligations(
self.tcx.mk_fn_ptr(method_sig),
all_substs,
method_predicates,
pick.item.def_id,
);
}

// Create the final `MethodCallee`.
Expand Down Expand Up @@ -471,16 +476,23 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
fty: Ty<'tcx>,
all_substs: SubstsRef<'tcx>,
method_predicates: ty::InstantiatedPredicates<'tcx>,
def_id: DefId,
) {
debug!(
"add_obligations: fty={:?} all_substs={:?} method_predicates={:?}",
fty, all_substs, method_predicates
"add_obligations: fty={:?} all_substs={:?} method_predicates={:?} def_id={:?}",
fty, all_substs, method_predicates, def_id
);

self.add_obligations_for_parameters(
traits::ObligationCause::misc(self.span, self.body_id),
// FIXME: could replace with the following, but we already calculated `method_predicates`,
// so we just call `predicates_for_generics` directly to avoid redoing work.
// `self.add_required_obligations(self.span, def_id, &all_substs);`
for obligation in traits::predicates_for_generics(
traits::ObligationCause::new(self.span, self.body_id, traits::ItemObligation(def_id)),
self.param_env,
method_predicates,
);
) {
self.register_predicate(obligation);
}

// this is a projection from a trait reference, so we have to
// make sure that the trait reference inputs are well-formed.
Expand Down
16 changes: 6 additions & 10 deletions compiler/rustc_typeck/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1990,16 +1990,12 @@ fn predicates_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::GenericPredicates<'_> {
// prove that the trait applies to the types that were
// used, and adding the predicate into this list ensures
// that this is done.
let mut span = tcx.def_span(def_id);
if tcx.sess.source_map().is_local_span(span) {
// `guess_head_span` reads the actual source file from
// disk to try to determine the 'head' snippet of the span.
// Don't do this for a span that comes from a file outside
// of our crate, since this would make our query output
// (and overall crate metadata) dependent on the
// *current* state of an external file.
span = tcx.sess.source_map().guess_head_span(span);
}
//
// We use a DUMMY_SP here as a way to signal trait bounds that come
// from the trait itself that *shouldn't* be shown as the source of
// an obligation and instead be skipped. Otherwise we'd use
// `tcx.def_span(def_id);`
let span = rustc_span::DUMMY_SP;
result.predicates =
tcx.arena.alloc_from_iter(result.predicates.iter().copied().chain(std::iter::once((
ty::TraitRef::identity(tcx, def_id).without_const().to_predicate(tcx),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,6 @@ LL | type C: Clone + Iterator<Item: Send + Iterator<Item: for<'a> Lam<&'a u8
| ^^^^ `<<Self as Case1>::C as Iterator>::Item` cannot be sent between threads safely
|
= help: the trait `Send` is not implemented for `<<Self as Case1>::C as Iterator>::Item`
note: required by a bound in `Send`
--> $SRC_DIR/core/src/marker.rs:LL:COL
|
LL | / pub unsafe auto trait Send {
LL | | // empty.
LL | | }
| |_^ required by this bound in `Send`
help: consider further restricting the associated type
|
LL | trait Case1 where <<Self as Case1>::C as Iterator>::Item: Send {
Expand All @@ -24,17 +17,6 @@ LL | type C: Clone + Iterator<Item: Send + Iterator<Item: for<'a> Lam<&'a u8
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `<<Self as Case1>::C as Iterator>::Item` is not an iterator
|
= help: the trait `Iterator` is not implemented for `<<Self as Case1>::C as Iterator>::Item`
note: required by a bound in `Iterator`
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
|
LL | / pub trait Iterator {
LL | | /// The type of the elements being iterated over.
LL | | #[stable(feature = "rust1", since = "1.0.0")]
LL | | type Item;
... |
LL | | }
LL | | }
| |_^ required by this bound in `Iterator`
help: consider further restricting the associated type
|
LL | trait Case1 where <<Self as Case1>::C as Iterator>::Item: Iterator {
Expand All @@ -47,17 +29,6 @@ LL | type C: Clone + Iterator<Item: Send + Iterator<Item: for<'a> Lam<&'a u8
| ^^^^ `<<Self as Case1>::C as Iterator>::Item` cannot be shared between threads safely
|
= help: the trait `Sync` is not implemented for `<<Self as Case1>::C as Iterator>::Item`
note: required by a bound in `Sync`
--> $SRC_DIR/core/src/marker.rs:LL:COL
|
LL | / pub unsafe auto trait Sync {
LL | | // FIXME(estebank): once support to add notes in `rustc_on_unimplemented`
LL | | // lands in beta, and it has been extended to check whether a closure is
LL | | // anywhere in the requirement chain, extend it as such (#48534):
... |
LL | | // Empty
LL | | }
| |_^ required by this bound in `Sync`
help: consider further restricting the associated type
|
LL | trait Case1 where <<Self as Case1>::C as Iterator>::Item: Sync {
Expand Down
22 changes: 0 additions & 22 deletions src/test/ui/associated-type-bounds/bounds-on-assoc-in-trait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,6 @@ LL | type A: Iterator<Item: Debug>;
| ^^^^^ `<<Self as Case1>::A as Iterator>::Item` cannot be formatted using `{:?}` because it doesn't implement `Debug`
|
= help: the trait `Debug` is not implemented for `<<Self as Case1>::A as Iterator>::Item`
note: required by a bound in `Debug`
--> $SRC_DIR/core/src/fmt/mod.rs:LL:COL
|
LL | / pub trait Debug {
LL | | /// Formats the value using the given formatter.
LL | | ///
LL | | /// # Examples
... |
LL | | fn fmt(&self, f: &mut Formatter<'_>) -> Result;
LL | | }
| |_^ required by this bound in `Debug`
help: consider further restricting the associated type
|
LL | trait Case1 where <<Self as Case1>::A as Iterator>::Item: Debug {
Expand All @@ -27,17 +16,6 @@ error[E0277]: the trait bound `<<Self as Foo>::Out as Baz>::Assoc: Default` is n
LL | pub trait Foo { type Out: Baz<Assoc: Default>; }
| ^^^^^^^ the trait `Default` is not implemented for `<<Self as Foo>::Out as Baz>::Assoc`
|
note: required by a bound in `Default`
--> $SRC_DIR/core/src/default.rs:LL:COL
|
LL | / pub trait Default: Sized {
LL | | /// Returns the "default value" for a type.
LL | | ///
LL | | /// Default values are often some kind of initial value, identity value, or anything else that
... |
LL | | fn default() -> Self;
LL | | }
| |_^ required by this bound in `Default`
help: consider further restricting the associated type
|
LL | pub trait Foo where <<Self as Foo>::Out as Baz>::Assoc: Default { type Out: Baz<Assoc: Default>; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@ error[E0277]: the trait bound `Self: Get` is not satisfied
LL | fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) {}
| ^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `Self`
|
note: required by a bound in `Get`
--> $DIR/associated-types-for-unimpl-trait.rs:4:1
|
LL | trait Get {
| ^^^^^^^^^ required by this bound in `Get`
help: consider further restricting `Self`
|
LL | fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) where Self: Get {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@ error[E0277]: the trait bound `T: Get` is not satisfied
LL | fn uhoh<T>(foo: <T as Get>::Value) {}
| ^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `T`
|
note: required by a bound in `Get`
--> $DIR/associated-types-no-suitable-bound.rs:1:1
|
LL | trait Get {
| ^^^^^^^^^ required by this bound in `Get`
help: consider restricting type parameter `T`
|
LL | fn uhoh<T: Get>(foo: <T as Get>::Value) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@ error[E0277]: the trait bound `Self: Get` is not satisfied
LL | fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) {}
| ^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `Self`
|
note: required by a bound in `Get`
--> $DIR/associated-types-no-suitable-supertrait-2.rs:12:1
|
LL | trait Get {
| ^^^^^^^^^ required by this bound in `Get`
help: consider further restricting `Self`
|
LL | fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) where Self: Get {}
Expand Down
Loading

0 comments on commit 6b9d910

Please sign in to comment.