Skip to content
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.

Sign up for GitHub

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

Point at associated type for some obligations #65288

Merged
merged 7 commits into from
Oct 27, 2019
Merged

Point at associated type for some obligations #65288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
58c6591
Point at associated type for some obligations
estebank Oct 10, 2019
88e4e2a
fix compile-fail test
estebank Oct 11, 2019
5b58095
Handle projection obligation errors
estebank Oct 11, 2019
0118278
Drive-by formatting
estebank Oct 11, 2019
669a403
review comments: move code, fix indentation and change span
estebank Oct 22, 2019
580a93e
Fix rebase
estebank Oct 22, 2019
3a4cacd
Add comments
estebank Oct 22, 2019
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
52 changes: 37 additions & 15 deletions src/librustc/traits/error_reporting.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
obligation: &PredicateObligation<'tcx>,
error: &MismatchedProjectionTypes<'tcx>,
) {
let predicate =
self.resolve_vars_if_possible(&obligation.predicate);
let predicate = self.resolve_vars_if_possible(&obligation.predicate);

if predicate.references_error() {
return
Expand Down Expand Up @@ -228,7 +227,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
&mut obligations
);
if let Err(error) = self.at(&obligation.cause, obligation.param_env)
.eq(normalized_ty, data.ty) {
.eq(normalized_ty, data.ty)
{
values = Some(infer::ValuePairs::Types(ExpectedFound {
expected: normalized_ty,
found: data.ty,
Expand All @@ -239,13 +239,19 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}

let msg = format!("type mismatch resolving `{}`", predicate);
let error_id = (DiagnosticMessageId::ErrorId(271),
Some(obligation.cause.span), msg);
let error_id = (
DiagnosticMessageId::ErrorId(271),
Some(obligation.cause.span),
msg,
);
let fresh = self.tcx.sess.one_time_diagnostics.borrow_mut().insert(error_id);
if fresh {
let mut diag = struct_span_err!(
self.tcx.sess, obligation.cause.span, E0271,
"type mismatch resolving `{}`", predicate
self.tcx.sess,
obligation.cause.span,
E0271,
"type mismatch resolving `{}`",
predicate
);
self.note_type_err(&mut diag, &obligation.cause, None, values, err);
self.note_obligation_cause(&mut diag, obligation);
Expand Down Expand Up @@ -532,23 +538,33 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
/// whose result could not be truly determined and thus we can't say
/// if the program type checks or not -- and they are unusual
/// occurrences in any case.
pub fn report_overflow_error<T>(&self,
obligation: &Obligation<'tcx, T>,
suggest_increasing_limit: bool) -> !
pub fn report_overflow_error<T>(
&self,
obligation: &Obligation<'tcx, T>,
suggest_increasing_limit: bool,
) -> !
where T: fmt::Display + TypeFoldable<'tcx>
{
let predicate =
self.resolve_vars_if_possible(&obligation.predicate);
let mut err = struct_span_err!(self.tcx.sess, obligation.cause.span, E0275,
"overflow evaluating the requirement `{}`",
predicate);
let mut err = struct_span_err!(
self.tcx.sess,
obligation.cause.span,
E0275,
"overflow evaluating the requirement `{}`",
predicate
);

if suggest_increasing_limit {
self.suggest_new_overflow_limit(&mut err);
}

self.note_obligation_cause_code(&mut err, &obligation.predicate, &obligation.cause.code,
&mut vec![]);
self.note_obligation_cause_code(
&mut err,
&obligation.predicate,
&obligation.cause.code,
&mut vec![],
);

err.emit();
self.tcx.sess.abort_if_errors();
Expand Down Expand Up @@ -2199,6 +2215,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
);
}
}
ObligationCauseCode::AssocTypeBound(impl_span, orig) => {
err.span_label(orig, "associated type defined here");
if let Some(sp) = impl_span {
err.span_label(sp, "in this `impl` item");
}
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/librustc/traits/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,8 @@ pub enum ObligationCauseCode<'tcx> {

/// #[feature(trivial_bounds)] is not enabled
TrivialBound,

AssocTypeBound(/*impl*/ Option<Span>, /*original*/ Span),
}

// `ObligationCauseCode` is used a lot. Make sure it doesn't unintentionally get bigger.
Expand Down
1 change: 1 addition & 0 deletions src/librustc/traits/structural_impls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,7 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
super::MethodReceiver => Some(super::MethodReceiver),
super::BlockTailExpression(id) => Some(super::BlockTailExpression(id)),
super::TrivialBound => Some(super::TrivialBound),
super::AssocTypeBound(impl_sp, sp) => Some(super::AssocTypeBound(impl_sp, sp)),
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc/ty/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3139,6 +3139,7 @@ impl<'tcx> TyCtxt<'tcx> {
}
}

#[derive(Clone)]
pub struct AssocItemsIterator<'tcx> {
tcx: TyCtxt<'tcx>,
def_ids: &'tcx [DefId],
Expand Down
172 changes: 153 additions & 19 deletions src/librustc/ty/wf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@ pub fn obligations<'a, 'tcx>(
ty: Ty<'tcx>,
span: Span,
) -> Option<Vec<traits::PredicateObligation<'tcx>>> {
let mut wf = WfPredicates { infcx,
param_env,
body_id,
span,
out: vec![] };
let mut wf = WfPredicates {
infcx,
param_env,
body_id,
span,
out: vec![],
item: None,
};
if wf.compute(ty) {
debug!("wf::obligations({:?}, body_id={:?}) = {:?}", ty, body_id, wf.out);
let result = wf.normalize();
Expand All @@ -47,8 +50,9 @@ pub fn trait_obligations<'a, 'tcx>(
body_id: hir::HirId,
trait_ref: &ty::TraitRef<'tcx>,
span: Span,
item: Option<&'tcx hir::Item>,
Copy link
Member

Choose a reason for hiding this comment

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

I don't think passing around references to the HIR fits ty::wf, this can just be an Option<DefId> and should probably be named something more suggestive.

) -> Vec<traits::PredicateObligation<'tcx>> {
let mut wf = WfPredicates { infcx, param_env, body_id, span, out: vec![] };
let mut wf = WfPredicates { infcx, param_env, body_id, span, out: vec![], item };
wf.compute_trait_ref(trait_ref, Elaborate::All);
wf.normalize()
}
Expand All @@ -60,7 +64,7 @@ pub fn predicate_obligations<'a, 'tcx>(
predicate: &ty::Predicate<'tcx>,
span: Span,
) -> Vec<traits::PredicateObligation<'tcx>> {
let mut wf = WfPredicates { infcx, param_env, body_id, span, out: vec![] };
let mut wf = WfPredicates { infcx, param_env, body_id, span, out: vec![], item: None };

// (*) ok to skip binders, because wf code is prepared for it
match *predicate {
Expand Down Expand Up @@ -107,6 +111,7 @@ struct WfPredicates<'a, 'tcx> {
body_id: hir::HirId,
span: Span,
out: Vec<traits::PredicateObligation<'tcx>>,
item: Option<&'tcx hir::Item>,
}

/// Controls whether we "elaborate" supertraits and so forth on the WF
Expand Down Expand Up @@ -157,33 +162,162 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
.collect()
}

/// Pushes the obligations required for `trait_ref` to be WF into
/// `self.out`.
/// Pushes the obligations required for `trait_ref` to be WF into `self.out`.
fn compute_trait_ref(&mut self, trait_ref: &ty::TraitRef<'tcx>, elaborate: Elaborate) {
let tcx = self.infcx.tcx;
let obligations = self.nominal_obligations(trait_ref.def_id, trait_ref.substs);

let cause = self.cause(traits::MiscObligation);
let param_env = self.param_env;

let item = &self.item;
Copy link
Member

Choose a reason for hiding this comment

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

No need to borrow a Copy field.

let extend_cause_with_original_assoc_item_obligation = |
Copy link
Contributor

Choose a reason for hiding this comment

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

Heh. Personally I'd have preferred to see this be a separate method, but I can accept this =) I do like the comments!

cause: &mut traits::ObligationCause<'_>,
pred: &ty::Predicate<'_>,
trait_assoc_items: ty::AssocItemsIterator<'_>,
| {
let item_span = item.map(|i| tcx.sess.source_map().def_span(i.span));
match pred {
ty::Predicate::Projection(proj) => {
// The obligation comes not from the current `impl` nor the `trait` being
// implemented, but rather from a "second order" obligation, like in
// `src/test/ui/associated-types/point-at-type-on-obligation-failure.rs`:
//
// error[E0271]: type mismatch resolving `<Foo2 as Bar2>::Ok == ()`
// --> $DIR/point-at-type-on-obligation-failure.rs:13:5
// |
// LL | type Ok;
// | -- associated type defined here
// ...
// LL | impl Bar for Foo {
// | ---------------- in this `impl` item
// LL | type Ok = ();
// | ^^^^^^^^^^^^^ expected u32, found ()
// |
// = note: expected type `u32`
// found type `()`
//
// FIXME: we would want to point a span to all places that contributed to this
// obligation. In the case above, it should be closer to:
//
// error[E0271]: type mismatch resolving `<Foo2 as Bar2>::Ok == ()`
// --> $DIR/point-at-type-on-obligation-failure.rs:13:5
// |
// LL | type Ok;
// | -- associated type defined here
// LL | type Sibling: Bar2<Ok=Self::Ok>;
// | -------------------------------- obligation set here
// ...
// LL | impl Bar for Foo {
// | ---------------- in this `impl` item
// LL | type Ok = ();
// | ^^^^^^^^^^^^^ expected u32, found ()
// ...
// LL | impl Bar2 for Foo2 {
// | ---------------- in this `impl` item
// LL | type Ok = u32;
// | -------------- obligation set here
// |
// = note: expected type `u32`
// found type `()`
if let Some(hir::ItemKind::Impl(.., impl_items)) = item.map(|i| &i.kind) {
let trait_assoc_item = tcx.associated_item(proj.projection_def_id());
if let Some(impl_item) = impl_items.iter().filter(|item| {
item.ident == trait_assoc_item.ident
}).next() {
cause.span = impl_item.span;
cause.code = traits::AssocTypeBound(
item_span,
trait_assoc_item.ident.span,
);
}
}
}
ty::Predicate::Trait(proj) => {
// An associated item obligation born out of the `trait` failed to be met.
// Point at the `impl` that failed the obligation, the associated item that
// needed to meet the obligation, and the definition of that associated item,
// which should hold the obligation in most cases. An example can be seen in
// `src/test/ui/associated-types/point-at-type-on-obligation-failure-2.rs`:
//
// error[E0277]: the trait bound `bool: Bar` is not satisfied
// --> $DIR/point-at-type-on-obligation-failure-2.rs:8:5
// |
// LL | type Assoc: Bar;
// | ----- associated type defined here
// ...
// LL | impl Foo for () {
// | --------------- in this `impl` item
// LL | type Assoc = bool;
// | ^^^^^^^^^^^^^^^^^^ the trait `Bar` is not implemented for `bool`
//
// FIXME: if the obligation comes from the where clause in the `trait`, we
// should point at it:
//
// error[E0277]: the trait bound `bool: Bar` is not satisfied
// --> $DIR/point-at-type-on-obligation-failure-2.rs:8:5
// |
// | trait Foo where <Self as Foo>>::Assoc: Bar {
// | -------------------------- obligation set here
// LL | type Assoc;
// | ----- associated type defined here
// ...
// LL | impl Foo for () {
// | --------------- in this `impl` item
// LL | type Assoc = bool;
// | ^^^^^^^^^^^^^^^^^^ the trait `Bar` is not implemented for `bool`
if let (
ty::Projection(ty::ProjectionTy { item_def_id, .. }),
Some(hir::ItemKind::Impl(.., impl_items)),
) = (&proj.skip_binder().self_ty().kind, item.map(|i| &i.kind)) {
if let Some((impl_item, trait_assoc_item)) = trait_assoc_items
.filter(|i| i.def_id == *item_def_id)
.next()
.and_then(|trait_assoc_item| impl_items.iter()
.filter(|i| i.ident == trait_assoc_item.ident)
.next()
.map(|impl_item| (impl_item, trait_assoc_item)))
{
cause.span = impl_item.span;
cause.code = traits::AssocTypeBound(
item_span,
trait_assoc_item.ident.span,
);
}
}
}
_ => {}
}
};

if let Elaborate::All = elaborate {
let trait_assoc_items = tcx.associated_items(trait_ref.def_id);

let predicates = obligations.iter()
.map(|obligation| obligation.predicate.clone())
.collect();
let implied_obligations = traits::elaborate_predicates(self.infcx.tcx, predicates);
.map(|obligation| obligation.predicate.clone())
.collect();
let implied_obligations = traits::elaborate_predicates(tcx, predicates);
let implied_obligations = implied_obligations.map(|pred| {
traits::Obligation::new(cause.clone(), param_env, pred)
let mut cause = cause.clone();
extend_cause_with_original_assoc_item_obligation(
&mut cause,
&pred,
trait_assoc_items.clone(),
);
traits::Obligation::new(cause, param_env, pred)
});
self.out.extend(implied_obligations);
}

self.out.extend(obligations);

self.out.extend(
trait_ref.substs.types()
.filter(|ty| !ty.has_escaping_bound_vars())
.map(|ty| traits::Obligation::new(cause.clone(),
param_env,
ty::Predicate::WellFormed(ty))));
self.out.extend(trait_ref.substs.types()
.filter(|ty| !ty.has_escaping_bound_vars())
.map(|ty| traits::Obligation::new(
cause.clone(),
param_env,
ty::Predicate::WellFormed(ty),
)));
}

/// Pushes the obligations required for `trait_ref::Item` to be WF
Expand Down
23 changes: 13 additions & 10 deletions src/librustc_typeck/check/wfcheck.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ fn check_item_type(

fn check_impl<'tcx>(
tcx: TyCtxt<'tcx>,
item: &hir::Item,
item: &'tcx hir::Item,
ast_self_ty: &hir::Ty,
ast_trait_ref: &Option<hir::TraitRef>,
) {
Expand All @@ -445,15 +445,18 @@ fn check_impl<'tcx>(
// therefore don't need to be WF (the trait's `Self: Trait` predicate
// won't hold).
let trait_ref = fcx.tcx.impl_trait_ref(item_def_id).unwrap();
let trait_ref =
fcx.normalize_associated_types_in(
ast_trait_ref.path.span, &trait_ref);
let obligations =
ty::wf::trait_obligations(fcx,
fcx.param_env,
fcx.body_id,
&trait_ref,
ast_trait_ref.path.span);
let trait_ref = fcx.normalize_associated_types_in(
ast_trait_ref.path.span,
&trait_ref,
);
let obligations = ty::wf::trait_obligations(
fcx,
fcx.param_env,
fcx.body_id,
&trait_ref,
ast_trait_ref.path.span,
Some(item),
);
for obligation in obligations {
fcx.register_predicate(obligation);
}
Expand Down
1 change: 1 addition & 0 deletions src/test/compile-fail/chalkify/impl_wf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ impl<T> Bar for Option<T> {
impl Bar for f32 {
//~^ ERROR the trait bound `f32: Foo` is not satisfied
type Item = f32;
//~^ ERROR the trait bound `f32: Foo` is not satisfied
}

trait Baz<U: ?Sized> where U: Foo { }
Expand Down
Loading