Skip to content

Commit

Permalink
Auto merge of #100654 - compiler-errors:rework-point-at-arg, r=estebank
Browse files Browse the repository at this point in the history
Rework "point at arg" suggestions to be more accurate

Fixes #100560

Introduce a new set of `ObligationCauseCode`s which have additional bookeeping for what expression caused the obligation, and which predicate caused the obligation. This allows us to look at the _unsubstituted_ signature to find out which parameter or generic type argument caused an obligaton to fail.

This means that (in most cases) we significantly improve the likelihood of pointing out the right argument that causes a fulfillment error. Also, since this logic isn't happening in just the `select_where_possible_and_mutate_fulfillment()` calls in the argument checking code, but instead during all trait selection in `FnCtxt`, we are also able to point out the correct argument even if inference means that we don't know whether an obligation has failed until well after a call expression has been checked.

r? `@ghost`
  • Loading branch information
bors committed Aug 21, 2022
2 parents c0941df + d577eb0 commit 0b71ffc
Show file tree
Hide file tree
Showing 161 changed files with 1,369 additions and 780 deletions.
10 changes: 5 additions & 5 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -740,12 +740,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
err.help("...or use `match` instead of `let...else`");
}
_ => {
if let ObligationCauseCode::BindingObligation(_, binding_span) =
cause.code().peel_derives()
if let ObligationCauseCode::BindingObligation(_, span)
| ObligationCauseCode::ExprBindingObligation(_, span, ..)
= cause.code().peel_derives()
&& let TypeError::RegionsPlaceholderMismatch = terr
{
if matches!(terr, TypeError::RegionsPlaceholderMismatch) {
err.span_note(*binding_span, "the lifetime requirement is introduced here");
}
err.span_note(*span, "the lifetime requirement is introduced here");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
let ObligationCauseCode::MatchImpl(parent, impl_def_id) = code else {
return None;
};
let ObligationCauseCode::BindingObligation(_def_id, binding_span) = *parent.code() else {
let (ObligationCauseCode::BindingObligation(_, binding_span) | ObligationCauseCode::ExprBindingObligation(_, binding_span, ..))
= *parent.code() else {
return None;
};
let mut err = self.tcx().sess.struct_span_err(cause.span, "incompatible lifetime on type");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,10 @@ impl<'tcx> NiceRegionError<'_, 'tcx> {
);
let mut err = self.tcx().sess.struct_span_err(span, &msg);

let leading_ellipsis = if let ObligationCauseCode::ItemObligation(def_id) = *cause.code() {
let leading_ellipsis = if let ObligationCauseCode::ItemObligation(def_id)
| ObligationCauseCode::ExprItemObligation(def_id, ..) =
*cause.code()
{
err.span_label(span, "doesn't satisfy where-clause");
err.span_label(
self.tcx().def_span(def_id),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
ObligationCauseCode::MatchImpl(parent, ..) => parent.code(),
_ => cause.code(),
}
&& let (&ObligationCauseCode::ItemObligation(item_def_id), None) = (code, override_error_code)
&& let (&ObligationCauseCode::ItemObligation(item_def_id) | &ObligationCauseCode::ExprItemObligation(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:
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_infer/src/infer/error_reporting/note.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,10 +390,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
if matches!(
&trace.cause.code().peel_derives(),
ObligationCauseCode::BindingObligation(..)
| ObligationCauseCode::ExprBindingObligation(..)
) =>
{
// Hack to get around the borrow checker because trace.cause has an `Rc`.
if let ObligationCauseCode::BindingObligation(_, span) =
if let ObligationCauseCode::BindingObligation(_, span)
| ObligationCauseCode::ExprBindingObligation(_, span, ..) =
&trace.cause.code().peel_derives()
{
let span = *span;
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_infer/src/infer/outlives/obligations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
cause.span,
sup_type,
match cause.code().peel_derives() {
ObligationCauseCode::BindingObligation(_, span) => Some(*span),
ObligationCauseCode::BindingObligation(_, span)
| ObligationCauseCode::ExprBindingObligation(_, span, ..) => Some(*span),
_ => None,
},
)
Expand Down
16 changes: 13 additions & 3 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,13 +234,23 @@ pub enum ObligationCauseCode<'tcx> {
/// This is the trait reference from the given projection.
ProjectionWf(ty::ProjectionTy<'tcx>),

/// In an impl of trait `X` for type `Y`, type `Y` must
/// also implement all supertraits of `X`.
/// Must satisfy all of the where-clause predicates of the
/// given item.
ItemObligation(DefId),

/// Like `ItemObligation`, but with extra detail on the source of the obligation.
/// Like `ItemObligation`, but carries the span of the
/// predicate when it can be identified.
BindingObligation(DefId, Span),

/// Like `ItemObligation`, but carries the `HirId` of the
/// expression that caused the obligation, and the `usize`
/// indicates exactly which predicate it is in the list of
/// instantiated predicates.
ExprItemObligation(DefId, rustc_hir::HirId, usize),

/// Combines `ExprItemObligation` and `BindingObligation`.
ExprBindingObligation(DefId, Span, rustc_hir::HirId, usize),

/// A type like `&'a T` is WF only if `T: 'a`.
ReferenceOutlivesReferent(Ty<'tcx>),

Expand Down
15 changes: 15 additions & 0 deletions compiler/rustc_middle/src/ty/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,21 @@ pub struct Generics {
}

impl<'tcx> Generics {
/// Looks through the generics and all parents to find the index of the
/// given param def-id. This is in comparison to the `param_def_id_to_index`
/// struct member, which only stores information about this item's own
/// generics.
pub fn param_def_id_to_index(&self, tcx: TyCtxt<'tcx>, def_id: DefId) -> Option<u32> {
if let Some(idx) = self.param_def_id_to_index.get(&def_id) {
Some(*idx)
} else if let Some(parent) = self.parent {
let parent = tcx.generics_of(parent);
parent.param_def_id_to_index(tcx, def_id)
} else {
None
}
}

#[inline]
pub fn count(&self) -> usize {
self.parent_count + self.params.len()
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_span/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,16 @@ impl Span {
Some(self)
}

/// Like `find_ancestor_inside`, but specifically for when spans might not
/// overlaps. Take care when using this, and prefer `find_ancestor_inside`
/// when you know that the spans are nested (modulo macro expansion).
pub fn find_ancestor_in_same_ctxt(mut self, other: Span) -> Option<Span> {
while !Span::eq_ctxt(self, other) {
self = self.parent_callsite()?;
}
Some(self)
}

/// Edition of the crate from which this span came.
pub fn edition(self) -> edition::Edition {
self.ctxt().edition()
Expand Down
28 changes: 11 additions & 17 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -860,8 +860,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}
}

err.emit();
return;
err
}

ty::PredicateKind::WellFormed(ty) => {
Expand Down Expand Up @@ -1564,6 +1563,8 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
obligation.cause.code().peel_derives(),
ObligationCauseCode::ItemObligation(_)
| ObligationCauseCode::BindingObligation(_, _)
| ObligationCauseCode::ExprItemObligation(..)
| ObligationCauseCode::ExprBindingObligation(..)
| ObligationCauseCode::ObjectCastObligation(..)
| ObligationCauseCode::OpaqueType
);
Expand Down Expand Up @@ -2091,13 +2092,11 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
}
}

if let ObligationCauseCode::ItemObligation(def_id) = *obligation.cause.code() {
if let ObligationCauseCode::ItemObligation(def_id) | ObligationCauseCode::ExprItemObligation(def_id, ..) = *obligation.cause.code() {
self.suggest_fully_qualified_path(&mut err, def_id, span, trait_ref.def_id());
} else if let (
Ok(ref snippet),
&ObligationCauseCode::BindingObligation(def_id, _),
) =
(self.tcx.sess.source_map().span_to_snippet(span), obligation.cause.code())
} else if let Ok(snippet) = &self.tcx.sess.source_map().span_to_snippet(span)
&& let ObligationCauseCode::BindingObligation(def_id, _) | ObligationCauseCode::ExprBindingObligation(def_id, ..)
= *obligation.cause.code()
{
let generics = self.tcx.generics_of(def_id);
if generics.params.iter().any(|p| p.name != kw::SelfUpper)
Expand Down Expand Up @@ -2520,15 +2519,10 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
err: &mut Diagnostic,
obligation: &PredicateObligation<'tcx>,
) {
let (
ty::PredicateKind::Trait(pred),
&ObligationCauseCode::BindingObligation(item_def_id, span),
) = (
obligation.predicate.kind().skip_binder(),
obligation.cause.code().peel_derives(),
) else {
return;
};
let ty::PredicateKind::Trait(pred) = obligation.predicate.kind().skip_binder() else { return; };
let (ObligationCauseCode::BindingObligation(item_def_id, span)
| ObligationCauseCode::ExprBindingObligation(item_def_id, span, ..))
= *obligation.cause.code().peel_derives() else { return; };
debug!(?pred, ?item_def_id, ?span);

let (Some(node), true) = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}

if let ObligationCauseCode::ItemObligation(item)
| ObligationCauseCode::BindingObligation(item, _) = *obligation.cause.code()
| ObligationCauseCode::BindingObligation(item, _)
| ObligationCauseCode::ExprItemObligation(item, ..)
| ObligationCauseCode::ExprBindingObligation(item, ..) = *obligation.cause.code()
{
// FIXME: maybe also have some way of handling methods
// from other traits? That would require name resolution,
Expand Down
Loading

0 comments on commit 0b71ffc

Please sign in to comment.