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.

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

Remove a bunch of redundant args from report_method_error #125906

Merged
merged 2 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
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
39 changes: 18 additions & 21 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use crate::errors::{
YieldExprOutsideOfCoroutine,
};
use crate::fatally_break_rust;
use crate::method::SelfSource;
use crate::type_error_struct;
use crate::CoroutineTypes;
use crate::Expectation::{self, ExpectCastableToType, ExpectHasType, NoExpectation};
Expand Down Expand Up @@ -223,7 +222,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let ty = ensure_sufficient_stack(|| match &expr.kind {
hir::ExprKind::Path(
qpath @ (hir::QPath::Resolved(..) | hir::QPath::TypeRelative(..)),
) => self.check_expr_path(qpath, expr, args, call),
) => self.check_expr_path(qpath, expr, Some(args), call),
_ => self.check_expr_kind(expr, expected),
});
let ty = self.resolve_vars_if_possible(ty);
Expand Down Expand Up @@ -290,7 +289,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ExprKind::Path(QPath::LangItem(lang_item, _)) => {
self.check_lang_item_path(lang_item, expr)
}
ExprKind::Path(ref qpath) => self.check_expr_path(qpath, expr, &[], None),
ExprKind::Path(ref qpath) => self.check_expr_path(qpath, expr, None, None),
ExprKind::InlineAsm(asm) => {
// We defer some asm checks as we may not have resolved the input and output types yet (they may still be infer vars).
self.deferred_asm_checks.borrow_mut().push((asm, expr.hir_id));
Expand Down Expand Up @@ -502,12 +501,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
qpath: &'tcx hir::QPath<'tcx>,
expr: &'tcx hir::Expr<'tcx>,
args: &'tcx [hir::Expr<'tcx>],
args: Option<&'tcx [hir::Expr<'tcx>]>,
call: Option<&'tcx hir::Expr<'tcx>>,
) -> Ty<'tcx> {
let tcx = self.tcx;
let (res, opt_ty, segs) =
self.resolve_ty_and_res_fully_qualified_call(qpath, expr.hir_id, expr.span, Some(args));
self.resolve_ty_and_res_fully_qualified_call(qpath, expr.hir_id, expr.span);
let ty = match res {
Res::Err => {
self.suggest_assoc_method_call(segs);
Expand Down Expand Up @@ -564,7 +563,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// We just want to check sizedness, so instead of introducing
// placeholder lifetimes with probing, we just replace higher lifetimes
// with fresh vars.
let span = args.get(i).map(|a| a.span).unwrap_or(expr.span);
let span = args.and_then(|args| args.get(i)).map_or(expr.span, |arg| arg.span);
let input = self.instantiate_binder_with_fresh_vars(
span,
infer::BoundRegionConversionTime::FnCall,
Expand Down Expand Up @@ -1331,9 +1330,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let rcvr_t = self.check_expr(rcvr);
// no need to check for bot/err -- callee does that
let rcvr_t = self.structurally_resolve_type(rcvr.span, rcvr_t);
let span = segment.ident.span;

let method = match self.lookup_method(rcvr_t, segment, span, expr, rcvr, args) {
let method = match self.lookup_method(rcvr_t, segment, segment.ident.span, expr, rcvr, args)
{
Ok(method) => {
// We could add a "consider `foo::<params>`" suggestion here, but I wasn't able to
// trigger this codepath causing `structurally_resolve_type` to emit an error.
Expand All @@ -1342,18 +1341,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
Err(error) => {
if segment.ident.name != kw::Empty {
if let Some(err) = self.report_method_error(
span,
Some(rcvr),
rcvr_t,
segment.ident,
expr.hir_id,
SelfSource::MethodCall(rcvr),
error,
Some(args),
expected,
false,
) {
if let Some(err) =
self.report_method_error(expr.hir_id, rcvr_t, error, expected, false)
{
err.emit();
}
}
Expand All @@ -1362,7 +1352,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};

// Call the generic checker.
self.check_method_argument_types(span, expr, method, args, DontTupleArguments, expected)
self.check_method_argument_types(
segment.ident.span,
expr,
method,
args,
DontTupleArguments,
expected,
)
}

fn check_expr_cast(
Expand Down
10 changes: 2 additions & 8 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::callee::{self, DeferredCallResolution};
use crate::errors::{self, CtorIsPrivate};
use crate::method::{self, MethodCallee, SelfSource};
use crate::method::{self, MethodCallee};
use crate::rvalue_scopes;
use crate::{BreakableCtxt, Diverges, Expectation, FnCtxt, LoweredTy};
use rustc_data_structures::fx::FxHashSet;
Expand Down Expand Up @@ -735,7 +735,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
qpath: &'tcx QPath<'tcx>,
hir_id: HirId,
span: Span,
args: Option<&'tcx [hir::Expr<'tcx>]>,
) -> (Res, Option<LoweredTy<'tcx>>, &'tcx [hir::PathSegment<'tcx>]) {
debug!(
"resolve_ty_and_res_fully_qualified_call: qpath={:?} hir_id={:?} span={:?}",
Expand Down Expand Up @@ -828,14 +827,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

if item_name.name != kw::Empty {
if let Some(e) = self.report_method_error(
span,
None,
ty.normalized,
item_name,
hir_id,
SelfSource::QPath(qself),
ty.normalized,
error,
args,
Expectation::NoExpectation,
trait_missing_method && span.edition().at_least_rust_2021(), // emits missing method for trait only after edition 2021
) {
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_hir_typeck/src/method/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ mod prelude_edition_lints;
pub mod probe;
mod suggest;

pub use self::suggest::SelfSource;
pub use self::MethodError::*;

use crate::FnCtxt;
Expand Down
58 changes: 38 additions & 20 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ use rustc_data_structures::unord::UnordSet;
use rustc_errors::{
codes::*, pluralize, struct_span_code_err, Applicability, Diag, MultiSpan, StashKey,
};
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
use rustc_hir::lang_items::LangItem;
use rustc_hir::PathSegment;
use rustc_hir::{self as hir, HirId};
use rustc_hir::{ExprKind, Node, QPath};
use rustc_infer::infer::{self, RegionVariableOrigin};
use rustc_middle::bug;
Expand Down Expand Up @@ -187,37 +187,56 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
#[instrument(level = "debug", skip(self))]
pub fn report_method_error(
&self,
span: Span,
rcvr_opt: Option<&'tcx hir::Expr<'tcx>>,
call_id: HirId,
rcvr_ty: Ty<'tcx>,
item_name: Ident,
expr_id: hir::HirId,
source: SelfSource<'tcx>,
error: MethodError<'tcx>,
args: Option<&'tcx [hir::Expr<'tcx>]>,
expected: Expectation<'tcx>,
trait_missing_method: bool,
) -> Option<Diag<'_>> {
let (span, sugg_span, source, item_name, args) = match self.tcx.hir_node(call_id) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Don't ask me what the difference is between span and sugg_span, because it's incredibly nasty how both of those spans are being passed around below and I don't particularly want to simplify it.

hir::Node::Expr(&hir::Expr {
kind: hir::ExprKind::MethodCall(segment, rcvr, args, _),
span,
..
}) => {
(segment.ident.span, span, SelfSource::MethodCall(rcvr), segment.ident, Some(args))
}
hir::Node::Expr(&hir::Expr {
kind: hir::ExprKind::Path(QPath::TypeRelative(rcvr, segment)),
span,
..
})
| hir::Node::Pat(&hir::Pat {
kind:
hir::PatKind::Path(QPath::TypeRelative(rcvr, segment))
| hir::PatKind::Struct(QPath::TypeRelative(rcvr, segment), ..)
| hir::PatKind::TupleStruct(QPath::TypeRelative(rcvr, segment), ..),
span,
..
}) => {
let args = match self.tcx.parent_hir_node(call_id) {
hir::Node::Expr(&hir::Expr {
kind: hir::ExprKind::Call(callee, args), ..
}) if callee.hir_id == call_id => Some(args),
_ => None,
};
(segment.ident.span, span, SelfSource::QPath(rcvr), segment.ident, args)
}
node => unreachable!("{node:?}"),
};

// Avoid suggestions when we don't know what's going on.
if rcvr_ty.references_error() {
return None;
}

let sugg_span = if let SelfSource::MethodCall(expr) = source {
// Given `foo.bar(baz)`, `expr` is `bar`, but we want to point to the whole thing.
self.tcx.hir().expect_expr(self.tcx.parent_hir_id(expr.hir_id)).span
} else {
span
};

match error {
MethodError::NoMatch(mut no_match_data) => {
return self.report_no_match_method_error(
span,
rcvr_opt,
rcvr_ty,
item_name,
expr_id,
call_id,
source,
args,
sugg_span,
Expand Down Expand Up @@ -362,7 +381,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

pub fn suggest_use_shadowed_binding_with_method(
&self,
rcvr_opt: Option<&'tcx hir::Expr<'tcx>>,
self_source: SelfSource<'tcx>,
method_name: Ident,
ty_str_reported: &str,
err: &mut Diag<'_>,
Expand Down Expand Up @@ -502,7 +521,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

if let Some(rcvr) = rcvr_opt
if let SelfSource::MethodCall(rcvr) = self_source
&& let hir::ExprKind::Path(QPath::Resolved(_, path)) = rcvr.kind
&& let hir::def::Res::Local(recv_id) = path.res
&& let Some(segment) = path.segments.first()
Expand Down Expand Up @@ -548,7 +567,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub fn report_no_match_method_error(
&self,
mut span: Span,
rcvr_opt: Option<&'tcx hir::Expr<'tcx>>,
rcvr_ty: Ty<'tcx>,
item_name: Ident,
expr_id: hir::HirId,
Expand Down Expand Up @@ -658,7 +676,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

if is_method {
self.suggest_use_shadowed_binding_with_method(
rcvr_opt,
source,
item_name,
&ty_str_reported,
&mut err,
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let PatInfo { binding_mode, max_ref_mutbl, top_info: ti, current_depth, .. } = pat_info;

let path_res = match &pat.kind {
PatKind::Path(qpath) => Some(
self.resolve_ty_and_res_fully_qualified_call(qpath, pat.hir_id, pat.span, None),
),
PatKind::Path(qpath) => {
Some(self.resolve_ty_and_res_fully_qualified_call(qpath, pat.hir_id, pat.span))
}
_ => None,
};
let adjust_mode = self.calc_adjust_mode(pat, path_res.map(|(res, ..)| res));
Expand Down Expand Up @@ -1184,7 +1184,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

// Resolve the path and check the definition for errors.
let (res, opt_ty, segments) =
self.resolve_ty_and_res_fully_qualified_call(qpath, pat.hir_id, pat.span, None);
self.resolve_ty_and_res_fully_qualified_call(qpath, pat.hir_id, pat.span);
if res == Res::Err {
let e = tcx.dcx().span_delayed_bug(pat.span, "`Res::Err` but no error emitted");
self.set_tainted_by_errors(e);
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/issues/issue-28344.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ error[E0599]: no function or associated item named `bitor` found for trait objec
LL | let g = BitXor::bitor;
| ^^^^^ function or associated item not found in `dyn BitXor<_>`
|
help: there is a method `bitxor` with a similar name, but with different arguments
help: there is a method `bitxor` with a similar name
Copy link
Member Author

Choose a reason for hiding this comment

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

We were previously passing Some([]) for the args for all raw path exprs, I believe. This lead to the diagnostics code to treat it as if it had no methods.

This expression has no arguments, so "but with different arguments" was spurious.

--> $SRC_DIR/core/src/ops/bit.rs:LL:COL

error: aborting due to 4 previous errors; 2 warnings emitted
Expand Down
7 changes: 0 additions & 7 deletions tests/ui/resolve/issue-82865.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,6 @@ LL | Box::z
LL | mac!();
| ------ in this macro invocation
|
note: if you're trying to build a new `Box<_, _>` consider using one of the following associated functions:
Copy link
Member Author

Choose a reason for hiding this comment

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

See above. We don't suggest builder methods for non-calls.

Box::<T>::new
Box::<T>::new_uninit
Box::<T>::new_zeroed
Box::<T>::try_new
and 18 others
--> $SRC_DIR/alloc/src/boxed.rs:LL:COL
= note: this error originates in the macro `mac` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 2 previous errors
Expand Down
9 changes: 5 additions & 4 deletions tests/ui/traits/item-privacy.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,15 @@ LL | S::B;
| ^ associated item not found in `S`
|
= help: items from traits can only be used if the trait is in scope
help: there is a method `b` with a similar name
--> $DIR/item-privacy.rs:11:9
|
LL | fn b(&self) { }
| ^^^^^^^^^^^
help: trait `B` which provides `B` is implemented but not in scope; perhaps you want to import it
|
LL + use assoc_const::B;
|
help: there is a method `b` with a similar name
|
LL | S::b;
Copy link
Member Author

Choose a reason for hiding this comment

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

See above, we don't do an identifier suggestion if it's not a method.

| ~

error[E0624]: associated constant `A` is private
--> $DIR/item-privacy.rs:101:14
Expand Down
Loading