Skip to content
Closed
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
4 changes: 3 additions & 1 deletion src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
@@ -138,7 +138,7 @@ pub enum ObligationCauseCode<'tcx> {

/// In an impl of trait `X` for type `Y`, type `Y` must
/// also implement all supertraits of `X`.
Comment on lines 139 to 140
Copy link
Member

Choose a reason for hiding this comment

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

Oh wow this comment is incredibly outdated (/me pokes @nikomatsakis).

ItemObligation(DefId),
ItemObligation(DefId, Option<Span>),

/// Like `ItemObligation`, but with extra detail on the source of the obligation.
BindingObligation(DefId, Span),
Comment on lines 143 to 144
Copy link
Member

Choose a reason for hiding this comment

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

Wait, looks like a version with a Span already exists, or is it too restricted to work for you?

Or I guess BindingObligation itself could be replaced with ItemObligation now.

Copy link
Member

Choose a reason for hiding this comment

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

At the very least seeing this tells me that having a Span in ObligationCauseCode is probably fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#69745 acknowledges this and uses BindingObligation instead. Some of the output is slightly worse, but can be iterated on.

@@ -193,6 +193,8 @@ pub enum ObligationCauseCode<'tcx> {

ImplDerivedObligation(DerivedObligationCause<'tcx>),

DerivedCauseCode(Box<ObligationCauseCode<'tcx>>),
Copy link
Member

Choose a reason for hiding this comment

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

Don't we have a system for this already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The closest is what is above but that requires a predicates to be available always, and that wasn't feasible for the cases I was handling.

Copy link
Member

@eddyb eddyb Mar 5, 2020

Choose a reason for hiding this comment

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

WF definitely has WF predicates, so at least that should be doable.

I'll take a second look at the diff and note the places DerivedCauseCode is created in.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I see, DerivedObligationCause is a bit limited, you want to replace the TraitRef with a Predicate (which would let you put a WF predicate in there).

But still, I'd prefer if this side of the PR was done separately from the ItemObligation side.


/// Error derived when matching traits/impls; see ObligationCause for more details
CompareImplMethodObligation {
item_name: ast::Name,
3 changes: 2 additions & 1 deletion src/librustc/traits/structural_impls.rs
Original file line number Diff line number Diff line change
@@ -410,7 +410,7 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
super::SliceOrArrayElem => Some(super::SliceOrArrayElem),
super::TupleElem => Some(super::TupleElem),
super::ProjectionWf(proj) => tcx.lift(&proj).map(super::ProjectionWf),
super::ItemObligation(def_id) => Some(super::ItemObligation(def_id)),
super::ItemObligation(def_id, span) => Some(super::ItemObligation(def_id, span)),
super::BindingObligation(def_id, span) => Some(super::BindingObligation(def_id, span)),
super::ReferenceOutlivesReferent(ty) => {
tcx.lift(&ty).map(super::ReferenceOutlivesReferent)
@@ -442,6 +442,7 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
super::ImplDerivedObligation(ref cause) => {
tcx.lift(cause).map(super::ImplDerivedObligation)
}
super::DerivedCauseCode(ref x) => tcx.lift(&**x).map(|x| x),
Copy link
Member

Choose a reason for hiding this comment

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

This is stripping DerivedCauseCode, please follow other match arms.

super::CompareImplMethodObligation {
item_name,
impl_item_def_id,
8 changes: 8 additions & 0 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
@@ -1932,6 +1932,14 @@ impl<'tcx> TyS<'tcx> {
}
}

#[inline]
pub fn is_some_param(&self) -> bool {
match self.kind {
ty::Param(_) => true,
_ => false,
}
}

#[inline]
pub fn is_slice(&self) -> bool {
match self.kind {
Original file line number Diff line number Diff line change
@@ -222,7 +222,7 @@ impl NiceRegionError<'me, 'tcx> {
format!("trait `{}` defined here", self.tcx().def_path_str(trait_def_id)),
);

let leading_ellipsis = if let ObligationCauseCode::ItemObligation(def_id) = cause.code {
let leading_ellipsis = if let ObligationCauseCode::ItemObligation(def_id, _) = cause.code {
err.span_label(span, "doesn't satisfy where-clause");
err.span_label(
self.tcx().def_span(def_id),
4 changes: 2 additions & 2 deletions src/librustc_infer/infer/opaque_types/mod.rs
Original file line number Diff line number Diff line change
@@ -1295,8 +1295,8 @@ crate fn required_region_bounds(
assert!(!erased_self_ty.has_escaping_bound_vars());

traits::elaborate_predicates(tcx, predicates)
.filter_map(|predicate| {
match predicate {
.filter_map(|obligation| {
match obligation.predicate {
ty::Predicate::Projection(..)
| ty::Predicate::Trait(..)
| ty::Predicate::Subtype(..)
5 changes: 4 additions & 1 deletion src/librustc_infer/infer/outlives/verify.rs
Original file line number Diff line number Diff line change
@@ -290,7 +290,10 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
let identity_proj = tcx.mk_projection(assoc_item_def_id, identity_substs);
self.collect_outlives_from_predicate_list(
move |ty| ty == identity_proj,
traits::elaborate_predicates(tcx, trait_predicates),
traits::elaborate_predicates(tcx, trait_predicates)
.into_iter()
.map(|o| o.predicate)
.collect::<Vec<_>>(),
)
.map(|b| b.1)
}
3 changes: 2 additions & 1 deletion src/librustc_infer/traits/auto_trait.rs
Original file line number Diff line number Diff line change
@@ -368,7 +368,8 @@ impl AutoTraitFinder<'tcx> {

computed_preds.extend(user_computed_preds.iter().cloned());
let normalized_preds =
elaborate_predicates(tcx, computed_preds.iter().cloned().collect());
elaborate_predicates(tcx, computed_preds.iter().cloned().collect())
.map(|o| o.predicate);
new_env =
ty::ParamEnv::new(tcx.mk_predicates(normalized_preds), param_env.reveal, None);
}
46 changes: 29 additions & 17 deletions src/librustc_infer/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
@@ -133,8 +133,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
};

for implication in super::elaborate_predicates(self.tcx, vec![*cond]) {
if let ty::Predicate::Trait(implication, _) = implication {
for obligation in super::elaborate_predicates(self.tcx, vec![*cond]) {
if let ty::Predicate::Trait(implication, _) = obligation.predicate {
let error = error.to_poly_trait_ref();
let implication = implication.to_poly_trait_ref();
// FIXME: I'm just not taking associated types at all here.
@@ -233,7 +233,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
);

let is_normalized_ty_expected = match &obligation.cause.code {
ObligationCauseCode::ItemObligation(_)
ObligationCauseCode::ItemObligation(..)
| ObligationCauseCode::BindingObligation(_, _)
| ObligationCauseCode::ObjectCastObligation(_) => false,
_ => true,
@@ -423,7 +423,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {

self.note_obligation_cause_code(
&mut err,
&obligation.predicate,
Some(&obligation.predicate),
&obligation.cause.code,
&mut vec![],
);
@@ -584,17 +584,29 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
))
);

let explanation =
if obligation.cause.code == ObligationCauseCode::MainFunctionType {
"consider using `()`, or a `Result`".to_owned()
} else {
format!(
"{}the trait `{}` is not implemented for `{}`",
pre_message,
trait_ref.print_only_trait_path(),
trait_ref.self_ty(),
)
};
let explanation = if obligation.cause.code
== ObligationCauseCode::MainFunctionType
{
"consider using `()`, or a `Result`".to_owned()
} else if self.tcx.lang_items().sized_trait().map_or(false, |sized_id| {
let self_ty = trait_ref.self_ty();
sized_id == trait_ref.def_id()
&& self_ty.is_some_param()
&& self_ty != self.tcx.types.self_param
}) {
// Detect type parameters with an implied `Sized` bound and explain
// it instead of giving a generic message. This will be displayed
// as a `help`.
"type parameters have an implicit `Sized` requirement by default"
.to_string()
} else {
format!(
"{}the trait `{}` is not implemented for `{}`",
pre_message,
trait_ref.print_only_trait_path(),
trait_ref.self_ty(),
)
};

if self.suggest_add_reference_to_arg(
&obligation,
@@ -1164,7 +1176,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
let mut err = self.need_type_info_err(body_id, span, self_ty, ErrorCode::E0283);
err.note(&format!("cannot resolve `{}`", predicate));
if let ObligationCauseCode::ItemObligation(def_id) = obligation.cause.code {
if let ObligationCauseCode::ItemObligation(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),
@@ -1333,7 +1345,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
if !self.maybe_note_obligation_cause_for_async_await(err, obligation) {
self.note_obligation_cause_code(
err,
&obligation.predicate,
Some(&obligation.predicate),
&obligation.cause.code,
&mut vec![],
);
Original file line number Diff line number Diff line change
@@ -126,7 +126,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
}

if let ObligationCauseCode::ItemObligation(item) = obligation.cause.code {
if let ObligationCauseCode::ItemObligation(item, _) = obligation.cause.code {
// FIXME: maybe also have some way of handling methods
// from other traits? That would require name resolution,
// which we might want to be some sort of hygienic.
53 changes: 36 additions & 17 deletions src/librustc_infer/traits/error_reporting/suggestions.rs
Original file line number Diff line number Diff line change
@@ -28,9 +28,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
body_id: hir::HirId,
) {
let self_ty = trait_ref.self_ty();
let (param_ty, projection) = match &self_ty.kind {
ty::Param(_) => (true, None),
ty::Projection(projection) => (false, Some(projection)),
let projection = match &self_ty.kind {
ty::Param(_) => None,
ty::Projection(projection) => Some(projection),
_ => return,
};

@@ -64,7 +64,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
generics,
kind: hir::TraitItemKind::Method(..),
..
}) if param_ty && self_ty == self.tcx.types.self_param => {
}) if self_ty.is_some_param() && self_ty == self.tcx.types.self_param => {
// Restricting `Self` for a single method.
suggest_restriction(&generics, "`Self`", err);
return;
@@ -138,7 +138,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
})
| hir::Node::TraitItem(hir::TraitItem { generics, span, .. })
| hir::Node::ImplItem(hir::ImplItem { generics, span, .. })
if param_ty =>
if self_ty.is_some_param() =>
{
// Missing generic type parameter bound.
let param_name = self_ty.to_string();
@@ -1421,7 +1421,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
debug!("note_obligation_cause_for_async_await: next_code={:?}", next_code);
self.note_obligation_cause_code(
err,
&obligation.predicate,
Some(&obligation.predicate),
next_code.unwrap(),
&mut Vec::new(),
);
@@ -1430,7 +1430,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
crate fn note_obligation_cause_code<T>(
&self,
err: &mut DiagnosticBuilder<'_>,
predicate: &T,
predicate: Option<&T>,
cause_code: &ObligationCauseCode<'tcx>,
obligated_types: &mut Vec<&ty::TyS<'tcx>>,
Copy link
Member

Choose a reason for hiding this comment

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

This is a weird name and type. I assume this was meant to be &mut Vec<Ty<'tcx>>? How is this not tripping the lint against types like TyS?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

This should indeed trip the internal lint. I want to revisit this lint anyway, to improve its suggestions. I'll look into this then.

) where
@@ -1470,15 +1470,30 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
region, object_ty,
));
}
ObligationCauseCode::ItemObligation(item_def_id) => {
ObligationCauseCode::ItemObligation(item_def_id, bound_span) => {
let item_name = tcx.def_path_str(item_def_id);
let msg = format!("required by `{}`", item_name);

if let Some(sp) = tcx.hir().span_if_local(item_def_id) {
let sp = tcx.sess.source_map().def_span(sp);
err.span_label(sp, &msg);
} else {
err.note(&msg);
match (
tcx.hir().span_if_local(item_def_id).map(|s| tcx.sess.source_map().def_span(s)),
bound_span,
) {
(Some(item_span), Some(bound_span)) if item_span.overlaps(bound_span) => {
err.span_label(bound_span, &format!("{} here", msg));
}
(Some(item_span), Some(bound_span)) => {
err.span_label(item_span, &msg);
err.span_label(bound_span, &format!("{} here", msg));
}
(None, Some(bound_span)) => {
err.span_label(bound_span, &format!("{} here", msg));
}
(Some(item_span), None) => {
err.span_label(item_span, &msg);
}
_ => {
err.note(&msg);
}
}
}
ObligationCauseCode::BindingObligation(item_def_id, span) => {
@@ -1576,6 +1591,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
ObligationCauseCode::SharedStatic => {
err.note("shared static variables must have a type that implements `Sync`");
}
ObligationCauseCode::DerivedCauseCode(ref data) => {
let mut obligated_types = vec![];
self.note_obligation_cause_code(err, None::<&T>, &**data, &mut obligated_types);
}
ObligationCauseCode::BuiltinDerivedObligation(ref data) => {
let parent_trait_ref = self.resolve_vars_if_possible(&data.parent_trait_ref);
let ty = parent_trait_ref.skip_binder().self_ty();
@@ -1586,7 +1605,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
if !self.is_recursive_obligation(obligated_types, &data.parent_code) {
self.note_obligation_cause_code(
err,
&parent_predicate,
Some(&parent_predicate),
&data.parent_code,
obligated_types,
);
@@ -1602,7 +1621,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let parent_predicate = parent_trait_ref.without_const().to_predicate();
self.note_obligation_cause_code(
err,
&parent_predicate,
Some(&parent_predicate),
&data.parent_code,
obligated_types,
);
@@ -1611,14 +1630,14 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
err.note(&format!(
"the requirement `{}` appears on the impl method \
but not on the corresponding trait method",
predicate
predicate.unwrap()
));
}
ObligationCauseCode::CompareImplTypeObligation { .. } => {
err.note(&format!(
"the requirement `{}` appears on the associated impl type \
but not on the corresponding associated trait type",
predicate
predicate.unwrap()
));
}
ObligationCauseCode::ReturnType
8 changes: 6 additions & 2 deletions src/librustc_infer/traits/mod.rs
Original file line number Diff line number Diff line change
@@ -64,7 +64,9 @@ pub use self::specialize::{specialization_graph, translate_substs, OverlapError}
pub use self::structural_match::search_for_structural_match_violation;
pub use self::structural_match::type_marked_structural;
pub use self::structural_match::NonStructuralMatchTy;
pub use self::util::{elaborate_predicates, elaborate_trait_ref, elaborate_trait_refs};
pub use self::util::{
elaborate_obligations, elaborate_predicates, elaborate_trait_ref, elaborate_trait_refs,
};
pub use self::util::{expand_trait_aliases, TraitAliasExpander};
pub use self::util::{
get_vtable_index_of_object_method, impl_is_default, impl_item_is_final,
@@ -361,7 +363,9 @@ pub fn normalize_param_env_or_error<'tcx>(
);

let mut predicates: Vec<_> =
util::elaborate_predicates(tcx, unnormalized_env.caller_bounds.to_vec()).collect();
util::elaborate_predicates(tcx, unnormalized_env.caller_bounds.to_vec())
.map(|o| o.predicate)
.collect();

debug!("normalize_param_env_or_error: elaborated-predicates={:?}", predicates);

2 changes: 1 addition & 1 deletion src/librustc_infer/traits/object_safety.rs
Original file line number Diff line number Diff line change
@@ -302,7 +302,7 @@ fn generics_require_sized_self(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
// Search for a predicate like `Self : Sized` amongst the trait bounds.
let predicates = tcx.predicates_of(def_id);
let predicates = predicates.instantiate_identity(tcx).predicates;
elaborate_predicates(tcx, predicates).any(|predicate| match predicate {
elaborate_predicates(tcx, predicates).any(|obligation| match obligation.predicate {
ty::Predicate::Trait(ref trait_pred, _) => {
trait_pred.def_id() == sized_def_id && trait_pred.skip_binder().self_ty().is_param(0)
}
6 changes: 3 additions & 3 deletions src/librustc_infer/traits/project.rs
Original file line number Diff line number Diff line change
@@ -893,7 +893,7 @@ fn assemble_candidates_from_param_env<'cx, 'tcx>(
///
/// ```
/// trait Foo {
/// type FooT : Bar<BarT=i32>
/// type FooT: Bar<BarT=i32>
/// }
/// ```
///
@@ -923,7 +923,7 @@ fn assemble_candidates_from_trait_def<'cx, 'tcx>(
// If so, extract what we know from the trait and try to come up with a good answer.
let trait_predicates = tcx.predicates_of(def_id);
let bounds = trait_predicates.instantiate(tcx, substs);
let bounds = elaborate_predicates(tcx, bounds.predicates);
let bounds = elaborate_predicates(tcx, bounds.predicates).map(|o| o.predicate);
assemble_candidates_from_predicates(
selcx,
obligation,
@@ -1212,7 +1212,7 @@ fn confirm_object_candidate<'cx, 'tcx>(

// select only those projections that are actually projecting an
// item with the correct name
let env_predicates = env_predicates.filter_map(|p| match p {
let env_predicates = env_predicates.filter_map(|o| match o.predicate {
ty::Predicate::Projection(data) => {
if data.projection_def_id() == obligation.predicate.item_def_id {
Some(data)
Loading