Skip to content

Improve location reporting of trait placeholder error #124977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/region_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl<'tcx> ConstraintDescription for ConstraintCategory<'tcx> {
ConstraintCategory::OpaqueType => "opaque type ",
ConstraintCategory::ClosureUpvar(_) => "closure capture ",
ConstraintCategory::Usage => "this usage ",
ConstraintCategory::Predicate(_)
ConstraintCategory::Predicate(..)
| ConstraintCategory::Boring
| ConstraintCategory::BoringNoLocation
| ConstraintCategory::Internal
Expand Down
12 changes: 7 additions & 5 deletions compiler/rustc_borrowck/src/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1989,7 +1989,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
continue;
};
debug!(?constraint, ?p);
let ConstraintCategory::Predicate(span) = constraint.category else {
let ConstraintCategory::Predicate(_, span) = constraint.category else {
continue;
};
extra_info.push(ExtraConstraintInfo::PlaceholderFromPredicate(span));
Expand All @@ -2004,11 +2004,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
let cause_code = path
.iter()
.find_map(|constraint| {
if let ConstraintCategory::Predicate(predicate_span) = constraint.category {
if let ConstraintCategory::Predicate(source_def_id, predicate_span) =
constraint.category
{
// We currently do not store the `DefId` in the `ConstraintCategory`
// for performances reasons. The error reporting code used by NLL only
// uses the span, so this doesn't cause any problems at the moment.
Comment on lines 2010 to 2012
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// We currently do not store the `DefId` in the `ConstraintCategory`
// for performances reasons. The error reporting code used by NLL only
// uses the span, so this doesn't cause any problems at the moment.

or adjust accordingly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL

Some(ObligationCauseCode::WhereClause(CRATE_DEF_ID.to_def_id(), predicate_span))
Some(ObligationCauseCode::WhereClause(source_def_id, predicate_span))
} else {
None
}
Expand Down Expand Up @@ -2102,7 +2104,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
| ConstraintCategory::Boring
| ConstraintCategory::BoringNoLocation
| ConstraintCategory::Internal
| ConstraintCategory::Predicate(_) => false,
| ConstraintCategory::Predicate(..) => false,
ConstraintCategory::TypeAnnotation
| ConstraintCategory::Return(_)
| ConstraintCategory::Yield => true,
Expand All @@ -2115,7 +2117,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
| ConstraintCategory::Boring
| ConstraintCategory::BoringNoLocation
| ConstraintCategory::Internal
| ConstraintCategory::Predicate(_)
| ConstraintCategory::Predicate(..)
)
}
};
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/type_check/canonical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,13 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
&mut self,
// Keep this parameter for now, in case we start using
// it in `ConstraintCategory` at some point.
Comment on lines 102 to 103
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Keep this parameter for now, in case we start using
// it in `ConstraintCategory` at some point.

_def_id: DefId,
def_id: DefId,
instantiated_predicates: ty::InstantiatedPredicates<'tcx>,
locations: Locations,
) {
for (predicate, span) in instantiated_predicates {
debug!(?span, ?predicate);
let category = ConstraintCategory::Predicate(span);
let category = ConstraintCategory::Predicate(def_id, span);
let predicate = self.normalize_with_category(predicate, locations, category);
self.prove_predicate(predicate, locations, category);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::traits::{ObligationCause, ObligationCauseCode};
use rustc_data_structures::intern::Interned;
use rustc_errors::{Diag, IntoDiagArg};
use rustc_hir::def::Namespace;
use rustc_hir::def_id::{DefId, CRATE_DEF_ID};
use rustc_hir::def_id::DefId;
use rustc_middle::bug;
use rustc_middle::ty::error::ExpectedFound;
use rustc_middle::ty::print::{FmtPrinter, Print, PrintTraitRefExt as _, RegionHighlightMode};
Expand Down Expand Up @@ -240,21 +240,17 @@ impl<'tcx> NiceRegionError<'_, 'tcx> {
) -> Diag<'tcx> {
let span = cause.span();

let (leading_ellipsis, satisfy_span, where_span, dup_span, def_id) =
if let ObligationCauseCode::WhereClause(def_id, span)
| ObligationCauseCode::WhereClauseInExpr(def_id, span, ..) = *cause.code()
&& def_id != CRATE_DEF_ID.to_def_id()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this check removed? process_registered_region_obligations still introduces a dummy crate def-id. Won't it cause issues?

{
(
true,
Some(span),
Some(self.tcx().def_span(def_id)),
None,
self.tcx().def_path_str(def_id),
)
} else {
(false, None, None, Some(span), String::new())
};
let (leading_ellipsis, satisfy_span, where_span, dup_span, def_id) = match *cause.code() {
ObligationCauseCode::WhereClause(def_id, pred_span)
| ObligationCauseCode::WhereClauseInExpr(def_id, pred_span, ..) => (
true,
Some(span),
if pred_span.is_dummy() { Some(span) } else { Some(pred_span) },
None,
self.tcx().def_path_str(def_id),
),
_ => (false, None, None, Some(span), String::new()),
};

let expected_trait_ref = self.cx.resolve_vars_if_possible(ty::TraitRef::new_from_args(
self.cx.tcx,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_infer/src/infer/error_reporting/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
infer::CheckAssociatedTypeBounds { ref parent, .. } => {
self.note_region_origin(err, parent);
}
infer::AscribeUserTypeProvePredicate(span) => {
infer::AscribeUserTypeProvePredicate(_, span) => {
RegionOriginNote::Plain {
span,
msg: fluent::infer_ascribe_user_type_prove_predicate,
Expand Down Expand Up @@ -483,7 +483,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
);
err
}
infer::AscribeUserTypeProvePredicate(span) => {
infer::AscribeUserTypeProvePredicate(_, span) => {
let instantiated = note_and_explain::RegionExplanation::new(
self.tcx,
generic_param_scope,
Expand Down
12 changes: 7 additions & 5 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ pub enum SubregionOrigin<'tcx> {
trait_item_def_id: DefId,
},

AscribeUserTypeProvePredicate(Span),
AscribeUserTypeProvePredicate(DefId, Span),
}

// `SubregionOrigin` is used a lot. Make sure it doesn't unintentionally get bigger.
Expand All @@ -424,7 +424,9 @@ impl<'tcx> SubregionOrigin<'tcx> {
pub fn to_constraint_category(&self) -> ConstraintCategory<'tcx> {
match self {
Self::Subtype(type_trace) => type_trace.cause.to_constraint_category(),
Self::AscribeUserTypeProvePredicate(span) => ConstraintCategory::Predicate(*span),
Self::AscribeUserTypeProvePredicate(def_id, span) => {
ConstraintCategory::Predicate(*def_id, *span)
}
_ => ConstraintCategory::BoringNoLocation,
}
}
Expand Down Expand Up @@ -1766,7 +1768,7 @@ impl<'tcx> SubregionOrigin<'tcx> {
Reborrow(a) => a,
ReferenceOutlivesReferent(_, a) => a,
CompareImplItemObligation { span, .. } => span,
AscribeUserTypeProvePredicate(span) => span,
AscribeUserTypeProvePredicate(_, span) => span,
CheckAssociatedTypeBounds { ref parent, .. } => parent.span(),
}
}
Expand Down Expand Up @@ -1799,8 +1801,8 @@ impl<'tcx> SubregionOrigin<'tcx> {
parent: Box::new(default()),
},

traits::ObligationCauseCode::AscribeUserTypeProvePredicate(span) => {
SubregionOrigin::AscribeUserTypeProvePredicate(span)
traits::ObligationCauseCode::AscribeUserTypeProvePredicate(def_id, span) => {
SubregionOrigin::AscribeUserTypeProvePredicate(def_id, span)
}

_ => default(),
Expand Down
14 changes: 12 additions & 2 deletions compiler/rustc_infer/src/infer/outlives/obligations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ use crate::infer::snapshot::undo_log::UndoLog;
use crate::infer::{self, GenericKind, InferCtxt, RegionObligation, SubregionOrigin, VerifyBound};
use crate::traits::{ObligationCause, ObligationCauseCode};
use rustc_data_structures::undo_log::UndoLogs;
use rustc_hir::def_id::CRATE_DEF_ID;
use rustc_middle::bug;
use rustc_middle::mir::ConstraintCategory;
use rustc_middle::traits::query::NoSolution;
Expand Down Expand Up @@ -153,11 +154,20 @@ impl<'tcx> InferCtxt<'tcx> {
Some(
deeply_normalize_ty(
outlives,
SubregionOrigin::AscribeUserTypeProvePredicate(DUMMY_SP),
SubregionOrigin::AscribeUserTypeProvePredicate(
CRATE_DEF_ID.to_def_id(),
DUMMY_SP,
),
)
// FIXME(-Znext-solver): How do we accurately report an error span here :(
.map_err(|NoSolution| {
(outlives, SubregionOrigin::AscribeUserTypeProvePredicate(DUMMY_SP))
(
outlives,
SubregionOrigin::AscribeUserTypeProvePredicate(
CRATE_DEF_ID.to_def_id(),
DUMMY_SP,
),
)
}),
)
})
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/mir/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::mir;
use crate::ty::{self, CoroutineArgsExt, OpaqueHiddenType, Ty, TyCtxt};
use rustc_data_structures::fx::FxIndexMap;
use rustc_errors::ErrorGuaranteed;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_index::bit_set::BitMatrix;
use rustc_index::{Idx, IndexVec};
use rustc_macros::{HashStable, TyDecodable, TyEncodable, TypeFoldable, TypeVisitable};
Expand Down Expand Up @@ -263,7 +263,7 @@ pub enum ConstraintCategory<'tcx> {
/// A constraint from a user-written predicate
/// with the provided span, written on the item
/// with the given `DefId`
Predicate(Span),
Predicate(#[derivative(PartialOrd = "ignore", Ord = "ignore")] DefId, Span),

/// A "boring" constraint (caused by the given location) is one that
/// the user probably doesn't want to see described in diagnostics,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ impl<'tcx> ObligationCause<'tcx> {
pub fn to_constraint_category(&self) -> ConstraintCategory<'tcx> {
match self.code() {
ObligationCauseCode::MatchImpl(cause, _) => cause.to_constraint_category(),
ObligationCauseCode::AscribeUserTypeProvePredicate(predicate_span) => {
ConstraintCategory::Predicate(*predicate_span)
ObligationCauseCode::AscribeUserTypeProvePredicate(def_id, predicate_span) => {
ConstraintCategory::Predicate(*def_id, *predicate_span)
}
_ => ConstraintCategory::BoringNoLocation,
}
Expand Down Expand Up @@ -388,7 +388,7 @@ pub enum ObligationCauseCode<'tcx> {
output_ty: Option<Ty<'tcx>>,
},

AscribeUserTypeProvePredicate(Span),
AscribeUserTypeProvePredicate(DefId, Span),

RustCall,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ fn relate_mir_and_user_args<'tcx>(
let span = if span == DUMMY_SP { predicate_span } else { span };
let cause = ObligationCause::new(
span,
CRATE_DEF_ID,
ObligationCauseCode::AscribeUserTypeProvePredicate(predicate_span),
def_id.as_local().unwrap_or(CRATE_DEF_ID),
ObligationCauseCode::AscribeUserTypeProvePredicate(def_id, predicate_span),
);
let instantiated_predicate =
ocx.normalize(&cause.clone(), param_env, instantiated_predicate);
Expand All @@ -118,7 +118,7 @@ fn relate_mir_and_user_args<'tcx>(

// Now prove the well-formedness of `def_id` with `args`.
// Note for some items, proving the WF of `ty` is not sufficient because the
// well-formedness of an item may depend on the WF of gneneric args not present in the
// well-formedness of an item may depend on the WF of generic args not present in the
// item's type. Currently this is true for associated consts, e.g.:
// ```rust
// impl<T> MyTy<T> {
Expand Down
7 changes: 5 additions & 2 deletions tests/ui/coroutine/resume-arg-late-bound.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
error: implementation of `Coroutine` is not general enough
--> $DIR/resume-arg-late-bound.rs:15:5
|
LL | fn test(a: impl for<'a> Coroutine<&'a mut bool>) {}
| ------------------------------- due to a where-clause on `test`...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The terminology may be a bit surprising here.

...
LL | test(gen);
| ^^^^^^^^^ implementation of `Coroutine` is not general enough
| ^^^^^^^^^ doesn't satisfy where-clause
|
= note: `{coroutine@$DIR/resume-arg-late-bound.rs:11:28: 11:44}` must implement `Coroutine<&'1 mut bool>`, for any lifetime `'1`...
= note: ...`{coroutine@$DIR/resume-arg-late-bound.rs:11:28: 11:44}` must implement `Coroutine<&'1 mut bool>`, for any lifetime `'1`...
= note: ...but it actually implements `Coroutine<&'2 mut bool>`, for some specific lifetime `'2`

error: aborting due to 1 previous error
Expand Down
28 changes: 10 additions & 18 deletions tests/ui/higher-ranked/trait-bounds/issue-59311.stderr
Original file line number Diff line number Diff line change
@@ -1,31 +1,23 @@
error: implementation of `Trait` is not general enough
--> $DIR/issue-59311.rs:17:5
|
LL | / pub fn crash<V>(v: &V)
LL | | where
LL | | for<'a> &'a V: Trait + 'static,
| |____________________-----__________- due to a where-clause on `crash`...
| |
| doesn't satisfy where-clause
LL | {
LL | v.t(|| {});
| ^^^^^^^^^^
LL | for<'a> &'a V: Trait + 'static,
| ----- due to a where-clause on `crash`...
LL | {
LL | v.t(|| {});
| ^^^^^^^^^^ doesn't satisfy where-clause
|
= note: ...`Trait` would have to be implemented for the type `&'a V`
= note: ...but `Trait` is actually implemented for the type `&'0 V`, for some specific lifetime `'0`

error: implementation of `Trait` is not general enough
--> $DIR/issue-59311.rs:17:5
|
LL | / pub fn crash<V>(v: &V)
LL | | where
LL | | for<'a> &'a V: Trait + 'static,
| |____________________-----__________- due to a where-clause on `crash`...
| |
| doesn't satisfy where-clause
LL | {
LL | v.t(|| {});
| ^^^^^^^^^^
LL | for<'a> &'a V: Trait + 'static,
| ----- due to a where-clause on `crash`...
LL | {
LL | v.t(|| {});
| ^^^^^^^^^^ doesn't satisfy where-clause
|
= note: ...`Trait` would have to be implemented for the type `&'a V`
= note: ...but `Trait` is actually implemented for the type `&'0 V`, for some specific lifetime `'0`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,40 +1,50 @@
error: implementation of `FnOnce` is not general enough
--> $DIR/issue-71955.rs:45:5
|
LL | F2: FnOnce(&<F1 as Parser>::Output) -> bool
| --------------------------------------- due to a where-clause on `foo`...
...
LL | foo(bar, "string", |s| s.len() == 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't satisfy where-clause
|
= note: closure with signature `for<'a> fn(&'a &'2 str) -> bool` must implement `FnOnce<(&&'1 str,)>`, for any lifetime `'1`...
= note: ...closure with signature `for<'a> fn(&'a &'2 str) -> bool` must implement `FnOnce<(&&'1 str,)>`, for any lifetime `'1`...
= note: ...but it actually implements `FnOnce<(&&'2 str,)>`, for some specific lifetime `'2`

error: implementation of `FnOnce` is not general enough
--> $DIR/issue-71955.rs:45:5
|
LL | F2: FnOnce(&<F1 as Parser>::Output) -> bool
| ---- due to a where-clause on `foo`...
...
LL | foo(bar, "string", |s| s.len() == 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't satisfy where-clause
|
= note: closure with signature `for<'a> fn(&'a &'2 str) -> bool` must implement `FnOnce<(&&'1 str,)>`, for any lifetime `'1`...
= note: ...closure with signature `for<'a> fn(&'a &'2 str) -> bool` must implement `FnOnce<(&&'1 str,)>`, for any lifetime `'1`...
= note: ...but it actually implements `FnOnce<(&&'2 str,)>`, for some specific lifetime `'2`
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: implementation of `FnOnce` is not general enough
--> $DIR/issue-71955.rs:48:5
|
LL | F2: FnOnce(&<F1 as Parser>::Output) -> bool
| --------------------------------------- due to a where-clause on `foo`...
...
LL | foo(baz, "string", |s| s.0.len() == 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't satisfy where-clause
|
= note: closure with signature `for<'a> fn(&'a Wrapper<'2>) -> bool` must implement `FnOnce<(&Wrapper<'1>,)>`, for any lifetime `'1`...
= note: ...closure with signature `for<'a> fn(&'a Wrapper<'2>) -> bool` must implement `FnOnce<(&Wrapper<'1>,)>`, for any lifetime `'1`...
= note: ...but it actually implements `FnOnce<(&Wrapper<'2>,)>`, for some specific lifetime `'2`

error: implementation of `FnOnce` is not general enough
--> $DIR/issue-71955.rs:48:5
|
LL | F2: FnOnce(&<F1 as Parser>::Output) -> bool
| ---- due to a where-clause on `foo`...
...
LL | foo(baz, "string", |s| s.0.len() == 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't satisfy where-clause
|
= note: closure with signature `for<'a> fn(&'a Wrapper<'2>) -> bool` must implement `FnOnce<(&Wrapper<'1>,)>`, for any lifetime `'1`...
= note: ...closure with signature `for<'a> fn(&'a Wrapper<'2>) -> bool` must implement `FnOnce<(&Wrapper<'1>,)>`, for any lifetime `'1`...
= note: ...but it actually implements `FnOnce<(&Wrapper<'2>,)>`, for some specific lifetime `'2`
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: aborting due to 4 previous errors

Loading
Loading