Skip to content

minor changes to make method lookup diagnostic code easier to read #103937

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

Merged
merged 1 commit into from
Nov 4, 2022
Merged

minor changes to make method lookup diagnostic code easier to read #103937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 1 addition & 2 deletions compiler/rustc_hir_typeck/src/method/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ pub enum MethodError<'tcx> {
// not-in-scope traits which may work.
PrivateMatch(DefKind, DefId, Vec<DefId>),

// Found a `Self: Sized` bound where `Self` is a trait object, also the caller may have
// forgotten to import a trait.
// Found a `Self: Sized` bound where `Self` is a trait object.
IllegalSizedBound(Vec<DefId>, bool, Span),

// Found a match, but the return type is wrong
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_hir_typeck/src/method/probe.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1019,7 +1019,6 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {

let out_of_scope_traits = match self.pick_core() {
Some(Ok(p)) => vec![p.item.container_id(self.tcx)],
//Some(Ok(p)) => p.iter().map(|p| p.item.container().id()).collect(),
Some(Err(MethodError::Ambiguity(v))) => v
.into_iter()
.map(|source| match source {
Expand Down
136 changes: 39 additions & 97 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

match error {
MethodError::NoMatch(NoMatchData {
Copy link
Member

Choose a reason for hiding this comment

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

This block is tooo long, about 800 lines of code!
Is it worth to extract to a seperate sub-methods?
@compiler-errors

static_candidates: mut static_sources,
mut static_candidates,
unsatisfied_predicates,
out_of_scope_traits,
lev_candidate,
Expand Down Expand Up @@ -288,9 +288,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if generics.len() > 0 {
let mut autoderef = self.autoderef(span, actual);
let candidate_found = autoderef.any(|(ty, _)| {
if let ty::Adt(adt_deref, _) = ty.kind() {
if let ty::Adt(adt_def, _) = ty.kind() {
self.tcx
.inherent_impls(adt_deref.did())
.inherent_impls(adt_def.did())
.iter()
.filter_map(|def_id| self.associated_value(*def_id, item_name))
.count()
Expand Down Expand Up @@ -348,15 +348,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

let ty_span = match actual.kind() {
ty::Param(param_type) => {
let generics = self.tcx.generics_of(self.body_id.owner.to_def_id());
let type_param = generics.type_param(param_type, self.tcx);
Some(self.tcx.def_span(type_param.def_id))
}
ty::Param(param_type) => Some(
param_type.span_from_generics(self.tcx, self.body_id.owner.to_def_id()),
),
ty::Adt(def, _) if def.did().is_local() => Some(tcx.def_span(def.did())),
_ => None,
};

if let Some(span) = ty_span {
err.span_label(
span,
Expand Down Expand Up @@ -386,17 +383,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let mut custom_span_label = false;

if !static_sources.is_empty() {
if !static_candidates.is_empty() {
err.note(
"found the following associated functions; to be used as methods, \
functions must have a `self` parameter",
);
err.span_label(span, "this is an associated function, not a method");
custom_span_label = true;
}
if static_sources.len() == 1 {
if static_candidates.len() == 1 {
let ty_str =
if let Some(CandidateSource::Impl(impl_did)) = static_sources.get(0) {
if let Some(CandidateSource::Impl(impl_did)) = static_candidates.get(0) {
// When the "method" is resolved through dereferencing, we really want the
// original type that has the associated function for accurate suggestions.
// (#61411)
Expand All @@ -422,9 +419,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
err.help(&format!("try with `{}::{}`", ty_str, item_name,));
}

report_candidates(span, &mut err, &mut static_sources, sugg_span);
} else if static_sources.len() > 1 {
report_candidates(span, &mut err, &mut static_sources, sugg_span);
report_candidates(span, &mut err, &mut static_candidates, sugg_span);
} else if static_candidates.len() > 1 {
report_candidates(span, &mut err, &mut static_candidates, sugg_span);
}

let mut bound_spans = vec![];
Expand Down Expand Up @@ -496,24 +493,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if let (ty::Param(_), ty::PredicateKind::Trait(p)) =
(self_ty.kind(), parent_pred.kind().skip_binder())
{
let hir = self.tcx.hir();
let node = match p.trait_ref.self_ty().kind() {
ty::Param(_) => {
// Account for `fn` items like in `issue-35677.rs` to
// suggest restricting its type params.
let did = self.tcx.hir().body_owner_def_id(hir::BodyId {
hir_id: self.body_id,
});
Some(
self.tcx
.hir()
.get(self.tcx.hir().local_def_id_to_hir_id(did)),
)
let parent_body =
hir.body_owner(hir::BodyId { hir_id: self.body_id });
Some(hir.get(parent_body))
}
ty::Adt(def, _) => {
def.did().as_local().map(|def_id| hir.get_by_def_id(def_id))
}
ty::Adt(def, _) => def.did().as_local().map(|def_id| {
self.tcx
.hir()
.get(self.tcx.hir().local_def_id_to_hir_id(def_id))
}),
_ => None,
};
if let Some(hir::Node::Item(hir::Item { kind, .. })) = node {
Expand Down Expand Up @@ -605,7 +596,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.iter()
.filter_map(|(p, parent, c)| c.as_ref().map(|c| (p, parent, c)))
.filter_map(|(p, parent, c)| match c.code() {
ObligationCauseCode::ImplDerivedObligation(ref data) => {
ObligationCauseCode::ImplDerivedObligation(data) => {
Some((&data.derived, p, parent, data.impl_def_id, data))
}
_ => None,
Expand All @@ -620,22 +611,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
match self.tcx.hir().get_if_local(impl_def_id) {
// Unmet obligation comes from a `derive` macro, point at it once to
// avoid multiple span labels pointing at the same place.
Some(Node::Item(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'm pretty convinced that this code will never be triggered, but if it is, someone will report it as an ICE 😅

Copy link
Member

Choose a reason for hiding this comment

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

For this to be triggered, we need to have a nested obligation of a trait be reported as an unsatisfied predicate for a method to be applicable. We only construct nested trait obligations with ImplDerivedObligation causes for auto traits, which have no methods.

Copy link
Member

Choose a reason for hiding this comment

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

#105732 is here,
maybe we should add an delay_span_bug here for match with Some(node)?
@compiler-errors

kind: hir::ItemKind::Trait(..),
ident,
..
})) if matches!(
ident.span.ctxt().outer_expn_data().kind,
ExpnKind::Macro(MacroKind::Derive, _)
) =>
{
let span = ident.span.ctxt().outer_expn_data().call_site;
let mut spans: MultiSpan = span.into();
spans.push_span_label(span, derive_msg);
let entry = spanned_predicates.entry(spans);
entry.or_insert_with(|| (path, tr_self_ty, Vec::new())).2.push(p);
}

Some(Node::Item(hir::Item {
kind: hir::ItemKind::Impl(hir::Impl { of_trait, self_ty, .. }),
..
Expand All @@ -659,34 +634,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
entry.or_insert_with(|| (path, tr_self_ty, Vec::new())).2.push(p);
}

// Unmet obligation coming from a `trait`.
Some(Node::Item(hir::Item {
kind: hir::ItemKind::Trait(..),
ident,
span: item_span,
..
})) if !matches!(
ident.span.ctxt().outer_expn_data().kind,
ExpnKind::Macro(MacroKind::Derive, _)
) =>
{
if let Some(pred) = parent_p {
// Done to add the "doesn't satisfy" `span_label`.
let _ = format_pred(*pred);
}
skip_list.insert(p);
let mut spans = if cause.span != *item_span {
let mut spans: MultiSpan = cause.span.into();
spans.push_span_label(cause.span, unsatisfied_msg);
spans
} else {
ident.span.into()
};
spans.push_span_label(ident.span, "in this trait");
let entry = spanned_predicates.entry(spans);
entry.or_insert_with(|| (path, tr_self_ty, Vec::new())).2.push(p);
}

// Unmet obligation coming from an `impl`.
Some(Node::Item(hir::Item {
kind:
Expand All @@ -695,19 +642,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}),
span: item_span,
..
})) if !matches!(
self_ty.span.ctxt().outer_expn_data().kind,
ExpnKind::Macro(MacroKind::Derive, _)
) && !matches!(
of_trait.as_ref().map(|t| t
.path
.span
.ctxt()
.outer_expn_data()
.kind),
Some(ExpnKind::Macro(MacroKind::Derive, _))
) =>
{
})) => {
let sized_pred =
unsatisfied_predicates.iter().any(|(pred, _, _)| {
match pred.kind().skip_binder() {
Expand Down Expand Up @@ -759,7 +694,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let entry = spanned_predicates.entry(spans);
entry.or_insert_with(|| (path, tr_self_ty, Vec::new())).2.push(p);
}
_ => {}
Some(_) => unreachable!(),
None => (),
}
}
let mut spanned_predicates: Vec<_> = spanned_predicates.into_iter().collect();
Expand Down Expand Up @@ -863,7 +799,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.on_unimplemented_note(trait_ref, &obligation);
(message, label)
})
.unwrap_or((None, None))
.unwrap()
} else {
(None, None)
};
Expand Down Expand Up @@ -972,7 +908,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// If the method name is the name of a field with a function or closure type,
// give a helping note that it has to be called as `(x.f)(...)`.
if let SelfSource::MethodCall(expr) = source {
if !self.suggest_field_call(span, rcvr_ty, expr, item_name, &mut err)
if !self.suggest_calling_field_as_fn(span, rcvr_ty, expr, item_name, &mut err)
&& lev_candidate.is_none()
&& !custom_span_label
{
Expand All @@ -982,10 +918,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
label_span_not_found(&mut err);
}

// Don't suggest (for example) `expr.field.method()` if `expr.method()`
// doesn't exist due to unsatisfied predicates.
// Don't suggest (for example) `expr.field.clone()` if `expr.clone()`
// can't be called due to `typeof(expr): Clone` not holding.
if unsatisfied_predicates.is_empty() {
self.check_for_field_method(&mut err, source, span, actual, item_name);
self.suggest_calling_method_on_field(&mut err, source, span, actual, item_name);
}

self.check_for_inner_self(&mut err, source, span, actual, item_name);
Expand All @@ -1007,7 +943,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
source,
out_of_scope_traits,
&unsatisfied_predicates,
&static_sources,
&static_candidates,
unsatisfied_bounds,
);
}
Expand Down Expand Up @@ -1146,7 +1082,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
None
}

fn suggest_field_call(
/// Suggest calling a field with a type that implements the `Fn*` traits instead of a method with
/// the same name as the field i.e. `(a.my_fn_ptr)(10)` instead of `a.my_fn_ptr(10)`.
fn suggest_calling_field_as_fn(
&self,
span: Span,
rcvr_ty: Ty<'tcx>,
Expand Down Expand Up @@ -1408,7 +1346,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
false
}

fn check_for_field_method(
/// Suggest calling a method on a field i.e. `a.field.bar()` instead of `a.bar()`
fn suggest_calling_method_on_field(
&self,
err: &mut Diagnostic,
source: SelfSource<'tcx>,
Expand Down Expand Up @@ -2021,7 +1960,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) {
let mut alt_rcvr_sugg = false;
if let (SelfSource::MethodCall(rcvr), false) = (source, unsatisfied_bounds) {
debug!(?span, ?item_name, ?rcvr_ty, ?rcvr);
debug!(
"suggest_traits_to_import: span={:?}, item_name={:?}, rcvr_ty={:?}, rcvr={:?}",
span, item_name, rcvr_ty, rcvr
);
let skippable = [
self.tcx.lang_items().clone_trait(),
self.tcx.lang_items().deref_trait(),
Expand Down Expand Up @@ -2060,7 +2002,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// suggestions are generally misleading (see #94218).
break;
}
_ => {}
Err(_) => (),
}

for (rcvr_ty, pre) in &[
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_middle/src/traits/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,20 @@ pub struct UnifyReceiverContext<'tcx> {
pub substs: SubstsRef<'tcx>,
}

#[derive(Clone, Debug, PartialEq, Eq, Hash, Lift, Default)]
#[derive(Clone, PartialEq, Eq, Hash, Lift, Default)]
pub struct InternedObligationCauseCode<'tcx> {
/// `None` for `ObligationCauseCode::MiscObligation` (a common case, occurs ~60% of
/// the time). `Some` otherwise.
code: Option<Lrc<ObligationCauseCode<'tcx>>>,
}

impl<'tcx> std::fmt::Debug for InternedObligationCauseCode<'tcx> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let cause: &ObligationCauseCode<'_> = self;
cause.fmt(f)
}
}

impl<'tcx> ObligationCauseCode<'tcx> {
#[inline(always)]
fn into(self) -> InternedObligationCauseCode<'tcx> {
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_middle/src/ty/sty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use rustc_hir::def_id::DefId;
use rustc_index::vec::Idx;
use rustc_macros::HashStable;
use rustc_span::symbol::{kw, sym, Symbol};
use rustc_span::Span;
use rustc_target::abi::VariantIdx;
use rustc_target::spec::abi;
use std::borrow::Cow;
Expand Down Expand Up @@ -1282,6 +1283,12 @@ impl<'tcx> ParamTy {
pub fn to_ty(self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> {
tcx.mk_ty_param(self.index, self.name)
}

pub fn span_from_generics(&self, tcx: TyCtxt<'tcx>, item_with_generics: DefId) -> Span {
let generics = tcx.generics_of(item_with_generics);
let type_param = generics.type_param(self, tcx);
tcx.def_span(type_param.def_id)
}
}

#[derive(Copy, Clone, Hash, TyEncodable, TyDecodable, Eq, PartialEq, Ord, PartialOrd)]
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_trait_selection/src/traits/on_unimplemented.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub struct OnUnimplementedDirective {
}

#[derive(Default)]
/// For the `#[rustc_on_unimplemented]` attribute
pub struct OnUnimplementedNote {
pub message: Option<String>,
pub label: Option<String>,
Expand Down