Skip to content

Commit

Permalink
Auto merge of rust-lang#89323 - estebank:derive-binop, r=petrochenkov
Browse files Browse the repository at this point in the history
Consider unfulfilled obligations in binop errors

When encountering a binop where the types would have been accepted, if
all the predicates had been fulfilled, include information about the
predicates and suggest appropriate `#[derive]`s if possible.

Fix rust-lang#84515.
  • Loading branch information
bors committed Oct 6, 2021
2 parents 98a5a98 + e8fc076 commit d7539a6
Show file tree
Hide file tree
Showing 27 changed files with 667 additions and 108 deletions.
21 changes: 21 additions & 0 deletions compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2171,10 +2171,26 @@ impl fmt::Debug for TraitRefPrintOnlyTraitPath<'tcx> {
}
}

/// Wrapper type for `ty::TraitRef` which opts-in to pretty printing only
/// the trait name. That is, it will print `Trait` instead of
/// `<T as Trait<U>>`.
#[derive(Copy, Clone, TypeFoldable, Lift)]
pub struct TraitRefPrintOnlyTraitName<'tcx>(ty::TraitRef<'tcx>);

impl fmt::Debug for TraitRefPrintOnlyTraitName<'tcx> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(self, f)
}
}

impl ty::TraitRef<'tcx> {
pub fn print_only_trait_path(self) -> TraitRefPrintOnlyTraitPath<'tcx> {
TraitRefPrintOnlyTraitPath(self)
}

pub fn print_only_trait_name(self) -> TraitRefPrintOnlyTraitName<'tcx> {
TraitRefPrintOnlyTraitName(self)
}
}

impl ty::Binder<'tcx, ty::TraitRef<'tcx>> {
Expand All @@ -2193,6 +2209,7 @@ forward_display_to_print! {
ty::Binder<'tcx, ty::ExistentialPredicate<'tcx>>,
ty::Binder<'tcx, ty::TraitRef<'tcx>>,
ty::Binder<'tcx, TraitRefPrintOnlyTraitPath<'tcx>>,
ty::Binder<'tcx, TraitRefPrintOnlyTraitName<'tcx>>,
ty::Binder<'tcx, ty::FnSig<'tcx>>,
ty::Binder<'tcx, ty::TraitPredicate<'tcx>>,
ty::Binder<'tcx, ty::SubtypePredicate<'tcx>>,
Expand Down Expand Up @@ -2255,6 +2272,10 @@ define_print_and_forward_display! {
p!(print_def_path(self.0.def_id, self.0.substs));
}

TraitRefPrintOnlyTraitName<'tcx> {
p!(print_def_path(self.0.def_id, &[]));
}
ty::ParamTy {
p!(write("{}", self.name))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
"this error might have been caused by changes to \
Rust's type-inference algorithm (see issue #48950 \
<https://github.com/rust-lang/rust/issues/48950> \
for more information).",
for more information)",
);
err.help("did you intend to use the type `()` here instead?");
}
Expand Down
66 changes: 41 additions & 25 deletions compiler/rustc_typeck/src/check/method/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,29 +290,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)
}

/// `lookup_method_in_trait` is used for overloaded operators.
/// It does a very narrow slice of what the normal probe/confirm path does.
/// In particular, it doesn't really do any probing: it simply constructs
/// an obligation for a particular trait with the given self type and checks
/// whether that trait is implemented.
//
// FIXME(#18741): it seems likely that we can consolidate some of this
// code with the other method-lookup code. In particular, the second half
// of this method is basically the same as confirmation.
#[instrument(level = "debug", skip(self, span, opt_input_types))]
pub fn lookup_method_in_trait(
pub(super) fn obligation_for_method(
&self,
span: Span,
m_name: Ident,
trait_def_id: DefId,
self_ty: Ty<'tcx>,
opt_input_types: Option<&[Ty<'tcx>]>,
) -> Option<InferOk<'tcx, MethodCallee<'tcx>>> {
debug!(
"lookup_in_trait_adjusted(self_ty={:?}, m_name={}, trait_def_id={:?}, opt_input_types={:?})",
self_ty, m_name, trait_def_id, opt_input_types
);

) -> (traits::Obligation<'tcx, ty::Predicate<'tcx>>, &'tcx ty::List<ty::subst::GenericArg<'tcx>>)
{
// Construct a trait-reference `self_ty : Trait<input_tys>`
let substs = InternalSubsts::for_item(self.tcx, trait_def_id, |param, _| {
match param.kind {
Expand All @@ -332,17 +317,48 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

// Construct an obligation
let poly_trait_ref = ty::Binder::dummy(trait_ref);
let obligation = traits::Obligation::misc(
span,
self.body_id,
self.param_env,
poly_trait_ref.without_const().to_predicate(self.tcx),
(
traits::Obligation::misc(
span,
self.body_id,
self.param_env,
poly_trait_ref.without_const().to_predicate(self.tcx),
),
substs,
)
}

/// `lookup_method_in_trait` is used for overloaded operators.
/// It does a very narrow slice of what the normal probe/confirm path does.
/// In particular, it doesn't really do any probing: it simply constructs
/// an obligation for a particular trait with the given self type and checks
/// whether that trait is implemented.
//
// FIXME(#18741): it seems likely that we can consolidate some of this
// code with the other method-lookup code. In particular, the second half
// of this method is basically the same as confirmation.
#[instrument(level = "debug", skip(self, span, opt_input_types))]
pub(super) fn lookup_method_in_trait(
&self,
span: Span,
m_name: Ident,
trait_def_id: DefId,
self_ty: Ty<'tcx>,
opt_input_types: Option<&[Ty<'tcx>]>,
) -> Option<InferOk<'tcx, MethodCallee<'tcx>>> {
debug!(
"lookup_in_trait_adjusted(self_ty={:?}, m_name={}, trait_def_id={:?}, opt_input_types={:?})",
self_ty, m_name, trait_def_id, opt_input_types
);

let (obligation, substs) =
self.obligation_for_method(span, trait_def_id, self_ty, opt_input_types);

// Now we want to know if this can be matched
if !self.predicate_may_hold(&obligation) {
debug!("--> Cannot match obligation");
return None; // Cannot be matched, no such method resolution is possible.
// Cannot be matched, no such method resolution is possible.
return None;
}

// Trait must have a method named `m_name` and it should not have
Expand Down Expand Up @@ -416,7 +432,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ty::Binder::dummy(ty::PredicateKind::WellFormed(method_ty.into())).to_predicate(tcx),
));

let callee = MethodCallee { def_id, substs: trait_ref.substs, sig: fn_sig };
let callee = MethodCallee { def_id, substs, sig: fn_sig };

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

Expand Down
121 changes: 118 additions & 3 deletions compiler/rustc_typeck/src/check/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ use rustc_middle::ty::print::with_crate_prefix;
use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness};
use rustc_span::lev_distance;
use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::{source_map, FileName, Span};
use rustc_span::{source_map, FileName, MultiSpan, Span};
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
use rustc_trait_selection::traits::Obligation;
use rustc_trait_selection::traits::{FulfillmentError, Obligation};

use std::cmp::Ordering;
use std::iter;
Expand Down Expand Up @@ -969,6 +969,79 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
None
}

crate fn note_unmet_impls_on_type(
&self,
err: &mut rustc_errors::DiagnosticBuilder<'_>,
errors: Vec<FulfillmentError<'tcx>>,
) {
let all_local_types_needing_impls =
errors.iter().all(|e| match e.obligation.predicate.kind().skip_binder() {
ty::PredicateKind::Trait(pred) => match pred.self_ty().kind() {
ty::Adt(def, _) => def.did.is_local(),
_ => false,
},
_ => false,
});
let mut preds: Vec<_> = errors
.iter()
.filter_map(|e| match e.obligation.predicate.kind().skip_binder() {
ty::PredicateKind::Trait(pred) => Some(pred),
_ => None,
})
.collect();
preds.sort_by_key(|pred| (pred.def_id(), pred.self_ty()));
let def_ids = preds
.iter()
.filter_map(|pred| match pred.self_ty().kind() {
ty::Adt(def, _) => Some(def.did),
_ => None,
})
.collect::<FxHashSet<_>>();
let sm = self.tcx.sess.source_map();
let mut spans: MultiSpan = def_ids
.iter()
.filter_map(|def_id| {
let span = self.tcx.def_span(*def_id);
if span.is_dummy() { None } else { Some(sm.guess_head_span(span)) }
})
.collect::<Vec<_>>()
.into();

for pred in &preds {
match pred.self_ty().kind() {
ty::Adt(def, _) => {
spans.push_span_label(
sm.guess_head_span(self.tcx.def_span(def.did)),
format!("must implement `{}`", pred.trait_ref.print_only_trait_path()),
);
}
_ => {}
}
}

if all_local_types_needing_impls && spans.primary_span().is_some() {
let msg = if preds.len() == 1 {
format!(
"an implementation of `{}` might be missing for `{}`",
preds[0].trait_ref.print_only_trait_path(),
preds[0].self_ty()
)
} else {
format!(
"the following type{} would have to `impl` {} required trait{} for this \
operation to be valid",
pluralize!(def_ids.len()),
if def_ids.len() == 1 { "its" } else { "their" },
pluralize!(preds.len()),
)
};
err.span_note(spans, &msg);
}

let preds: Vec<_> = errors.iter().map(|e| (e.obligation.predicate, None)).collect();
self.suggest_derive(err, &preds);
}

fn suggest_derive(
&self,
err: &mut DiagnosticBuilder<'_>,
Expand Down Expand Up @@ -1010,7 +1083,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return Some((
format!("{}", trait_ref.self_ty()),
self.tcx.def_span(adt_def.did),
format!("{}", trait_ref.print_only_trait_path()),
format!("{}", trait_ref.print_only_trait_name()),
));
}
None
Expand All @@ -1033,6 +1106,48 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
acc
},
);
let mut traits: Vec<_> = unsatisfied_predicates
.iter()
.filter_map(|(pred, _)| {
let trait_pred =
if let ty::PredicateKind::Trait(trait_pred) = pred.kind().skip_binder() {
trait_pred
} else {
return None;
};
if let ty::Adt(adt_def, _) = trait_pred.trait_ref.self_ty().kind() {
if !adt_def.did.is_local() {
return None;
}
} else {
return None;
};

let did = trait_pred.def_id();
let diagnostic_items = self.tcx.diagnostic_items(did.krate);

if !derivables
.iter()
.any(|trait_derivable| diagnostic_items.get(trait_derivable) == Some(&did))
{
Some(self.tcx.def_span(did))
} else {
None
}
})
.collect();
traits.sort();
traits.dedup();

let len = traits.len();
if len > 0 {
let span: MultiSpan = traits.into();
err.span_note(
span,
&format!("the following trait{} must be implemented", pluralize!(len),),
);
}

for (self_name, self_span, traits) in &derives_grouped {
err.span_suggestion_verbose(
self_span.shrink_to_lo(),
Expand Down
Loading

0 comments on commit d7539a6

Please sign in to comment.