Skip to content

Commit

Permalink
Auto merge of #106607 - compiler-errors:be-more-accurate-abt-method-s…
Browse files Browse the repository at this point in the history
…uggestions, r=oli-obk

Consider return type when giving various method suggestions

1. Fix a bug in method probe where we weren't normalizing `xform_ret_ty` for non-`impl` method candidates. This shouldn't affect happy-path code, since we only use `xform_ret_ty` when probing methods for diagnostics (I think).
2. Pass the return type expectation down to `lookup_probe`/`probe_for_name` usages in diagnostics. Added a few UI tests to gate against bad suggestions.
3. Make a `FnCtxt::lookup_probe_for_diagnostic` which properly passes down `IsSuggestion(true)`. Should help suppress other weird notes in some corner cases.
  • Loading branch information
bors committed Jan 10, 2023
2 parents deba5dd + 4f15034 commit 0442fba
Show file tree
Hide file tree
Showing 16 changed files with 329 additions and 107 deletions.
9 changes: 4 additions & 5 deletions compiler/rustc_hir_typeck/src/callee.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::method::probe::{IsSuggestion, Mode, ProbeScope};
use super::method::probe::ProbeScope;
use super::method::MethodCallee;
use super::{Expectation, FnCtxt, TupleArgumentsFlag};

Expand Down Expand Up @@ -496,15 +496,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// any strange errors. If it's successful, then we'll do a true
// method lookup.
let Ok(pick) = self
.probe_for_name(
Mode::MethodCall,
.lookup_probe_for_diagnostic(
segment.ident,
IsSuggestion(true),
callee_ty,
call_expr.hir_id,
call_expr,
// We didn't record the in scope traits during late resolution
// so we need to probe AllTraits unfortunately
ProbeScope::AllTraits,
expected.only_has_type(self),
) else {
return None;
};
Expand Down
25 changes: 15 additions & 10 deletions compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,11 +303,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Get the evaluated type *after* calling the method call, so that the influence
// of the arguments can be reflected in the receiver type. The receiver
// expression has the type *before* theis analysis is done.
let ty = match self.lookup_probe(
let ty = match self.lookup_probe_for_diagnostic(
segment.ident,
rcvr_ty,
expr,
probe::ProbeScope::TraitsInScope,
None,
) {
Ok(pick) => pick.self_ty,
Err(_) => rcvr_ty,
Expand Down Expand Up @@ -557,19 +558,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let Some(self_ty) = self.typeck_results.borrow().expr_ty_adjusted_opt(base) else { return; };

let Ok(pick) = self
.probe_for_name(
probe::Mode::MethodCall,
.lookup_probe_for_diagnostic(
path.ident,
probe::IsSuggestion(true),
self_ty,
deref.hir_id,
deref,
probe::ProbeScope::TraitsInScope,
None,
) else {
return;
};
let in_scope_methods = self.probe_for_name_many(
probe::Mode::MethodCall,
path.ident,
Some(expected),
probe::IsSuggestion(true),
self_ty,
deref.hir_id,
Expand All @@ -581,6 +582,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let all_methods = self.probe_for_name_many(
probe::Mode::MethodCall,
path.ident,
Some(expected),
probe::IsSuggestion(true),
self_ty,
deref.hir_id,
Expand Down Expand Up @@ -1832,7 +1834,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub fn check_for_range_as_method_call(
&self,
err: &mut Diagnostic,
expr: &hir::Expr<'_>,
expr: &hir::Expr<'tcx>,
checked_ty: Ty<'tcx>,
expected_ty: Ty<'tcx>,
) {
Expand All @@ -1850,10 +1852,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return;
}
let mut expr = end.expr;
let mut expectation = Some(expected_ty);
while let hir::ExprKind::MethodCall(_, rcvr, ..) = expr.kind {
// Getting to the root receiver and asserting it is a fn call let's us ignore cases in
// `src/test/ui/methods/issues/issue-90315.stderr`.
expr = rcvr;
// If we have more than one layer of calls, then the expected ty
// cannot guide the method probe.
expectation = None;
}
let hir::ExprKind::Call(method_name, _) = expr.kind else { return; };
let ty::Adt(adt, _) = checked_ty.kind() else { return; };
Expand All @@ -1869,13 +1875,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let hir::ExprKind::Path(hir::QPath::Resolved(None, p)) = method_name.kind else { return; };
let [hir::PathSegment { ident, .. }] = p.segments else { return; };
let self_ty = self.typeck_results.borrow().expr_ty(start.expr);
let Ok(_pick) = self.probe_for_name(
probe::Mode::MethodCall,
let Ok(_pick) = self.lookup_probe_for_diagnostic(
*ident,
probe::IsSuggestion(true),
self_ty,
expr.hir_id,
expr,
probe::ProbeScope::AllTraits,
expectation,
) else { return; };
let mut sugg = ".";
let mut span = start.expr.span.between(end.expr.span);
Expand Down
22 changes: 16 additions & 6 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ExprKind::Struct(qpath, fields, ref base_expr) => {
self.check_expr_struct(expr, expected, qpath, fields, base_expr)
}
ExprKind::Field(base, field) => self.check_field(expr, &base, field),
ExprKind::Field(base, field) => self.check_field(expr, &base, field, expected),
ExprKind::Index(base, idx) => self.check_expr_index(base, idx, expr),
ExprKind::Yield(value, ref src) => self.check_expr_yield(value, expr, src),
hir::ExprKind::Err => tcx.ty_error(),
Expand Down Expand Up @@ -1244,6 +1244,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
SelfSource::MethodCall(rcvr),
error,
Some((rcvr, args)),
expected,
) {
err.emit();
}
Expand Down Expand Up @@ -2186,6 +2187,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expr: &'tcx hir::Expr<'tcx>,
base: &'tcx hir::Expr<'tcx>,
field: Ident,
expected: Expectation<'tcx>,
) -> Ty<'tcx> {
debug!("check_field(expr: {:?}, base: {:?}, field: {:?})", expr, base, field);
let base_ty = self.check_expr(base);
Expand Down Expand Up @@ -2244,12 +2246,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// (#90483) apply adjustments to avoid ExprUseVisitor from
// creating erroneous projection.
self.apply_adjustments(base, adjustments);
self.ban_private_field_access(expr, base_ty, field, did);
self.ban_private_field_access(expr, base_ty, field, did, expected.only_has_type(self));
return self.tcx().ty_error();
}

if field.name == kw::Empty {
} else if self.method_exists(field, base_ty, expr.hir_id, true) {
} else if self.method_exists(
field,
base_ty,
expr.hir_id,
true,
expected.only_has_type(self),
) {
self.ban_take_value_of_method(expr, base_ty, field);
} else if !base_ty.is_primitive_ty() {
self.ban_nonexisting_field(field, base, expr, base_ty);
Expand Down Expand Up @@ -2423,10 +2431,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

fn ban_private_field_access(
&self,
expr: &hir::Expr<'_>,
expr: &hir::Expr<'tcx>,
expr_t: Ty<'tcx>,
field: Ident,
base_did: DefId,
return_ty: Option<Ty<'tcx>>,
) {
let struct_path = self.tcx().def_path_str(base_did);
let kind_name = self.tcx().def_kind(base_did).descr(base_did);
Expand All @@ -2438,7 +2447,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
err.span_label(field.span, "private field");
// Also check if an accessible method exists, which is often what is meant.
if self.method_exists(field, expr_t, expr.hir_id, false) && !self.expr_in_place(expr.hir_id)
if self.method_exists(field, expr_t, expr.hir_id, false, return_ty)
&& !self.expr_in_place(expr.hir_id)
{
self.suggest_method_call(
&mut err,
Expand All @@ -2452,7 +2462,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
err.emit();
}

fn ban_take_value_of_method(&self, expr: &hir::Expr<'_>, expr_t: Ty<'tcx>, field: Ident) {
fn ban_take_value_of_method(&self, expr: &hir::Expr<'tcx>, expr_t: Ty<'tcx>, field: Ident) {
let mut err = type_error_struct!(
self.tcx().sess,
field.span,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
SelfSource::QPath(qself),
error,
None,
Expectation::NoExpectation,
) {
e.emit();
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1343,6 +1343,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if let Ok(pick) = self.probe_for_name(
Mode::Path,
Ident::new(capitalized_name, segment.ident.span),
Some(expected_ty),
IsSuggestion(true),
self_ty,
expr.hir_id,
Expand Down
80 changes: 54 additions & 26 deletions compiler/rustc_hir_typeck/src/method/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self_ty: Ty<'tcx>,
call_expr_id: hir::HirId,
allow_private: bool,
return_type: Option<Ty<'tcx>>,
) -> bool {
match self.probe_for_name(
probe::Mode::MethodCall,
method_name,
return_type,
IsSuggestion(false),
self_ty,
call_expr_id,
Expand All @@ -118,7 +120,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Err(Ambiguity(..)) => true,
Err(PrivateMatch(..)) => allow_private,
Err(IllegalSizedBound { .. }) => true,
Err(BadReturnType) => bug!("no return type expectations but got BadReturnType"),
Err(BadReturnType) => false,
}
}

Expand All @@ -130,17 +132,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
msg: &str,
method_name: Ident,
self_ty: Ty<'tcx>,
call_expr: &hir::Expr<'_>,
call_expr: &hir::Expr<'tcx>,
span: Option<Span>,
) {
let params = self
.probe_for_name(
probe::Mode::MethodCall,
.lookup_probe_for_diagnostic(
method_name,
IsSuggestion(true),
self_ty,
call_expr.hir_id,
call_expr,
ProbeScope::TraitsInScope,
None,
)
.map(|pick| {
let sig = self.tcx.fn_sig(pick.item.def_id);
Expand Down Expand Up @@ -221,25 +222,30 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

// We probe again, taking all traits into account (not only those in scope).
let candidates =
match self.lookup_probe(segment.ident, self_ty, call_expr, ProbeScope::AllTraits) {
// If we find a different result the caller probably forgot to import a trait.
Ok(ref new_pick) if pick.differs_from(new_pick) => {
vec![new_pick.item.container_id(self.tcx)]
}
Err(Ambiguity(ref sources)) => sources
.iter()
.filter_map(|source| {
match *source {
// Note: this cannot come from an inherent impl,
// because the first probing succeeded.
CandidateSource::Impl(def) => self.tcx.trait_id_of_impl(def),
CandidateSource::Trait(_) => None,
}
})
.collect(),
_ => Vec::new(),
};
let candidates = match self.lookup_probe_for_diagnostic(
segment.ident,
self_ty,
call_expr,
ProbeScope::AllTraits,
None,
) {
// If we find a different result the caller probably forgot to import a trait.
Ok(ref new_pick) if pick.differs_from(new_pick) => {
vec![new_pick.item.container_id(self.tcx)]
}
Err(Ambiguity(ref sources)) => sources
.iter()
.filter_map(|source| {
match *source {
// Note: this cannot come from an inherent impl,
// because the first probing succeeded.
CandidateSource::Impl(def) => self.tcx.trait_id_of_impl(def),
CandidateSource::Trait(_) => None,
}
})
.collect(),
_ => Vec::new(),
};

return Err(IllegalSizedBound { candidates, needs_mut, bound_span: span, self_expr });
}
Expand All @@ -252,12 +258,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
method_name: Ident,
self_ty: Ty<'tcx>,
call_expr: &'tcx hir::Expr<'tcx>,
call_expr: &hir::Expr<'_>,
scope: ProbeScope,
) -> probe::PickResult<'tcx> {
let pick = self.probe_for_name(
probe::Mode::MethodCall,
method_name,
None,
IsSuggestion(false),
self_ty,
call_expr.hir_id,
Expand All @@ -267,6 +274,26 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Ok(pick)
}

pub fn lookup_probe_for_diagnostic(
&self,
method_name: Ident,
self_ty: Ty<'tcx>,
call_expr: &hir::Expr<'_>,
scope: ProbeScope,
return_type: Option<Ty<'tcx>>,
) -> probe::PickResult<'tcx> {
let pick = self.probe_for_name(
probe::Mode::MethodCall,
method_name,
return_type,
IsSuggestion(true),
self_ty,
call_expr.hir_id,
scope,
)?;
Ok(pick)
}

pub(super) fn obligation_for_method(
&self,
cause: ObligationCause<'tcx>,
Expand Down Expand Up @@ -484,6 +511,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let pick = self.probe_for_name(
probe::Mode::Path,
method_name,
None,
IsSuggestion(false),
self_ty,
expr_id,
Expand Down
Loading

0 comments on commit 0442fba

Please sign in to comment.