From bd2c65337465dff5551a26479412f7d88be4ff4a Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 30 Apr 2025 01:40:23 +0000 Subject: [PATCH 1/4] Rename lookup_method_in_trait and consolidate a Ident::with_dummy_span call --- compiler/rustc_hir_typeck/src/callee.rs | 6 +++--- compiler/rustc_hir_typeck/src/method/mod.rs | 17 ++++++++++++----- compiler/rustc_hir_typeck/src/op.rs | 5 ++--- compiler/rustc_hir_typeck/src/place_op.rs | 18 +++--------------- 4 files changed, 20 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/callee.rs b/compiler/rustc_hir_typeck/src/callee.rs index 5f5e9e45612a7..0c1d07e5e519f 100644 --- a/compiler/rustc_hir_typeck/src/callee.rs +++ b/compiler/rustc_hir_typeck/src/callee.rs @@ -14,7 +14,7 @@ use rustc_middle::ty::adjustment::{ use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypeVisitableExt}; use rustc_middle::{bug, span_bug}; use rustc_span::def_id::LocalDefId; -use rustc_span::{Ident, Span, sym}; +use rustc_span::{Span, sym}; use rustc_trait_selection::error_reporting::traits::DefIdOrName; use rustc_trait_selection::infer::InferCtxtExt as _; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _; @@ -296,9 +296,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Ty::new_tup_from_iter(self.tcx, arg_exprs.iter().map(|e| self.next_ty_var(e.span))) }); - if let Some(ok) = self.lookup_method_in_trait( + if let Some(ok) = self.lookup_method_for_operator( self.misc(call_expr.span), - Ident::with_dummy_span(method_name), + method_name, trait_def_id, adjusted_ty, opt_input_type, diff --git a/compiler/rustc_hir_typeck/src/method/mod.rs b/compiler/rustc_hir_typeck/src/method/mod.rs index 1b67e2306aa74..34bbb7d7c05e6 100644 --- a/compiler/rustc_hir_typeck/src/method/mod.rs +++ b/compiler/rustc_hir_typeck/src/method/mod.rs @@ -19,7 +19,7 @@ use rustc_middle::ty::{ self, GenericArgs, GenericArgsRef, GenericParamDefKind, Ty, TypeVisitableExt, }; use rustc_middle::{bug, span_bug}; -use rustc_span::{ErrorGuaranteed, Ident, Span}; +use rustc_span::{ErrorGuaranteed, Ident, Span, Symbol}; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; use rustc_trait_selection::traits::{self, NormalizeExt}; use tracing::{debug, instrument}; @@ -329,10 +329,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// an obligation for a particular trait with the given self type and checks /// whether that trait is implemented. #[instrument(level = "debug", skip(self))] - pub(super) fn lookup_method_in_trait( + pub(super) fn lookup_method_for_operator( &self, cause: ObligationCause<'tcx>, - m_name: Ident, + method_name: Symbol, trait_def_id: DefId, self_ty: Ty<'tcx>, opt_rhs_ty: Option>, @@ -374,13 +374,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Trait must have a method named `m_name` and it should not have // type parameters or early-bound regions. let tcx = self.tcx; - let Some(method_item) = self.associated_value(trait_def_id, m_name) else { + // We use `Ident::with_dummy_span` since no built-in operator methods have + // any macro-specific hygeine, so the span's context doesn't really matter. + let Some(method_item) = + self.associated_value(trait_def_id, Ident::with_dummy_span(method_name)) + else { bug!("expected associated item for operator trait") }; let def_id = method_item.def_id; if !method_item.is_fn() { - span_bug!(tcx.def_span(def_id), "expected `{m_name}` to be an associated function"); + span_bug!( + tcx.def_span(def_id), + "expected `{method_name}` to be an associated function" + ); } debug!("lookup_in_trait_adjusted: method_item={:?}", method_item); diff --git a/compiler/rustc_hir_typeck/src/op.rs b/compiler/rustc_hir_typeck/src/op.rs index b86991f81ad83..7f7921b66b572 100644 --- a/compiler/rustc_hir_typeck/src/op.rs +++ b/compiler/rustc_hir_typeck/src/op.rs @@ -12,7 +12,7 @@ use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::{self, IsSuggestable, Ty, TyCtxt, TypeVisitableExt}; use rustc_session::errors::ExprParenthesesNeeded; use rustc_span::source_map::Spanned; -use rustc_span::{Ident, Span, Symbol, sym}; +use rustc_span::{Span, Symbol, sym}; use rustc_trait_selection::infer::InferCtxtExt; use rustc_trait_selection::traits::{FulfillmentError, Obligation, ObligationCtxt}; use tracing::debug; @@ -975,7 +975,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { lhs_ty, opname, trait_did ); - let opname = Ident::with_dummy_span(opname); let (opt_rhs_expr, opt_rhs_ty) = opt_rhs.unzip(); let cause = self.cause( span, @@ -990,7 +989,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); let method = - self.lookup_method_in_trait(cause.clone(), opname, trait_did, lhs_ty, opt_rhs_ty); + self.lookup_method_for_operator(cause.clone(), opname, trait_did, lhs_ty, opt_rhs_ty); match method { Some(ok) => { let method = self.register_infer_ok_obligations(ok); diff --git a/compiler/rustc_hir_typeck/src/place_op.rs b/compiler/rustc_hir_typeck/src/place_op.rs index 4fc903cf68b88..fedc75abe4927 100644 --- a/compiler/rustc_hir_typeck/src/place_op.rs +++ b/compiler/rustc_hir_typeck/src/place_op.rs @@ -8,7 +8,7 @@ use rustc_middle::ty::adjustment::{ PointerCoercion, }; use rustc_middle::ty::{self, Ty}; -use rustc_span::{Ident, Span, sym}; +use rustc_span::{Span, sym}; use tracing::debug; use {rustc_ast as ast, rustc_hir as hir}; @@ -211,13 +211,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return None; }; - self.lookup_method_in_trait( - self.misc(span), - Ident::with_dummy_span(imm_op), - imm_tr, - base_ty, - opt_rhs_ty, - ) + self.lookup_method_for_operator(self.misc(span), imm_op, imm_tr, base_ty, opt_rhs_ty) } fn try_mutable_overloaded_place_op( @@ -237,13 +231,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return None; }; - self.lookup_method_in_trait( - self.misc(span), - Ident::with_dummy_span(mut_op), - mut_tr, - base_ty, - opt_rhs_ty, - ) + self.lookup_method_for_operator(self.misc(span), mut_op, mut_tr, base_ty, opt_rhs_ty) } /// Convert auto-derefs, indices, etc of an expression from `Deref` and `Index` From e650c1da46223d418f30b690495391a5848481a9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 30 Apr 2025 01:48:48 +0000 Subject: [PATCH 2/4] Move the error handling out of confirm_builtin_call --- compiler/rustc_hir_typeck/src/callee.rs | 56 +++++++++++-------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/callee.rs b/compiler/rustc_hir_typeck/src/callee.rs index 0c1d07e5e519f..fa096944c38a9 100644 --- a/compiler/rustc_hir_typeck/src/callee.rs +++ b/compiler/rustc_hir_typeck/src/callee.rs @@ -87,14 +87,29 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let output = match result { None => { - // this will report an error since original_callee_ty is not a fn - self.confirm_builtin_call( - call_expr, - callee_expr, - original_callee_ty, - arg_exprs, - expected, - ) + // Check all of the arg expressions, but with no expectations + // since we don't have a signature to compare them to. + for arg in arg_exprs { + self.check_expr(arg); + } + + if let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = &callee_expr.kind + && let [segment] = path.segments + { + self.dcx().try_steal_modify_and_emit_err( + segment.ident.span, + StashKey::CallIntoMethod, + |err| { + // Try suggesting `foo(a)` -> `a.foo()` if possible. + self.suggest_call_as_method( + err, segment, arg_exprs, call_expr, expected, + ); + }, + ); + } + + let guar = self.report_invalid_callee(call_expr, callee_expr, expr_ty, arg_exprs); + Ty::new_error(self.tcx, guar) } Some(CallStep::Builtin(callee_ty)) => { @@ -461,32 +476,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } (fn_sig, Some(def_id)) } + // FIXME(const_trait_impl): these arms should error because we can't enforce them ty::FnPtr(sig_tys, hdr) => (sig_tys.with(hdr), None), - _ => { - for arg in arg_exprs { - self.check_expr(arg); - } - if let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = &callee_expr.kind - && let [segment] = path.segments - { - self.dcx().try_steal_modify_and_emit_err( - segment.ident.span, - StashKey::CallIntoMethod, - |err| { - // Try suggesting `foo(a)` -> `a.foo()` if possible. - self.suggest_call_as_method( - err, segment, arg_exprs, call_expr, expected, - ); - }, - ); - } - - let err = self.report_invalid_callee(call_expr, callee_expr, callee_ty, arg_exprs); - - return Ty::new_error(self.tcx, err); - } + _ => unreachable!(), }; // Replace any late-bound regions that appear in the function From 6aa3dd19434ff244c6adc0bbc0cdd8e9ec14567d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 30 Apr 2025 02:57:05 +0000 Subject: [PATCH 3/4] Inline check_method_argument_types to get rid of TupleArgumentsFlag arg --- compiler/rustc_hir_typeck/src/callee.rs | 16 ++++++++++------ compiler/rustc_hir_typeck/src/expr.rs | 10 +--------- compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs | 10 ++-------- 3 files changed, 13 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/callee.rs b/compiler/rustc_hir_typeck/src/callee.rs index fa096944c38a9..f555d116c52db 100644 --- a/compiler/rustc_hir_typeck/src/callee.rs +++ b/compiler/rustc_hir_typeck/src/callee.rs @@ -902,19 +902,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { call_expr: &'tcx hir::Expr<'tcx>, arg_exprs: &'tcx [hir::Expr<'tcx>], expected: Expectation<'tcx>, - method_callee: MethodCallee<'tcx>, + method: MethodCallee<'tcx>, ) -> Ty<'tcx> { - let output_type = self.check_method_argument_types( + self.check_argument_types( call_expr.span, call_expr, - Ok(method_callee), + &method.sig.inputs()[1..], + method.sig.output(), + expected, arg_exprs, + method.sig.c_variadic, TupleArgumentsFlag::TupleArguments, - expected, + Some(method.def_id), ); - self.write_method_call_and_enforce_effects(call_expr.hir_id, call_expr.span, method_callee); - output_type + self.write_method_call_and_enforce_effects(call_expr.hir_id, call_expr.span, method); + + method.sig.output() } } diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 9c6d4ee096f1a..91077c976e461 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -40,7 +40,6 @@ use tracing::{debug, instrument, trace}; use {rustc_ast as ast, rustc_hir as hir}; use crate::Expectation::{self, ExpectCastableToType, ExpectHasType, NoExpectation}; -use crate::TupleArgumentsFlag::DontTupleArguments; use crate::coercion::{CoerceMany, DynamicCoerceMany}; use crate::errors::{ AddressOfTemporaryTaken, BaseExpressionDoubleDot, BaseExpressionDoubleDotAddExpr, @@ -1605,14 +1604,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }; // Call the generic checker. - self.check_method_argument_types( - segment.ident.span, - expr, - method, - args, - DontTupleArguments, - expected, - ) + self.check_method_argument_types(segment.ident.span, expr, method, args, expected) } /// Checks use `x.use`. diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index e70aa1a70645e..72588515f8aa1 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -133,7 +133,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expr: &'tcx hir::Expr<'tcx>, method: Result, ErrorGuaranteed>, args_no_rcvr: &'tcx [hir::Expr<'tcx>], - tuple_arguments: TupleArgumentsFlag, expected: Expectation<'tcx>, ) -> Ty<'tcx> { let has_error = match method { @@ -147,11 +146,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); let err_output = Ty::new_error(self.tcx, guar); - let err_inputs = match tuple_arguments { - DontTupleArguments => err_inputs, - TupleArguments => vec![Ty::new_tup(self.tcx, &err_inputs)], - }; - self.check_argument_types( sp, expr, @@ -160,7 +154,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { NoExpectation, args_no_rcvr, false, - tuple_arguments, + TupleArgumentsFlag::DontTupleArguments, method.ok().map(|method| method.def_id), ); return err_output; @@ -175,7 +169,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expected, args_no_rcvr, method.sig.c_variadic, - tuple_arguments, + TupleArgumentsFlag::DontTupleArguments, Some(method.def_id), ); From f986d124f134635995fe596b2670621df9c03a24 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 30 Apr 2025 03:13:59 +0000 Subject: [PATCH 4/4] Inline check_method_argument_types and remove error_reported special casing (unnecessary) --- compiler/rustc_hir_typeck/src/expr.rs | 48 +++++++++++++----- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 50 ------------------- 2 files changed, 36 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 91077c976e461..db2650ed357e4 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -50,8 +50,8 @@ use crate::errors::{ YieldExprOutsideOfCoroutine, }; use crate::{ - BreakableCtxt, CoroutineTypes, Diverges, FnCtxt, Needs, cast, fatally_break_rust, - report_unexpected_variant_res, type_error_struct, + BreakableCtxt, CoroutineTypes, Diverges, FnCtxt, Needs, TupleArgumentsFlag, cast, + fatally_break_rust, report_unexpected_variant_res, type_error_struct, }; impl<'a, 'tcx> FnCtxt<'a, 'tcx> { @@ -1590,21 +1590,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // no need to check for bot/err -- callee does that let rcvr_t = self.structurally_resolve_type(rcvr.span, rcvr_t); - let method = match self.lookup_method(rcvr_t, segment, segment.ident.span, expr, rcvr, args) - { + match self.lookup_method(rcvr_t, segment, segment.ident.span, expr, rcvr, args) { Ok(method) => { - // We could add a "consider `foo::`" suggestion here, but I wasn't able to - // trigger this codepath causing `structurally_resolve_type` to emit an error. self.write_method_call_and_enforce_effects(expr.hir_id, expr.span, method); - Ok(method) + + self.check_argument_types( + segment.ident.span, + expr, + &method.sig.inputs()[1..], + method.sig.output(), + expected, + args, + method.sig.c_variadic, + TupleArgumentsFlag::DontTupleArguments, + Some(method.def_id), + ); + + method.sig.output() } Err(error) => { - Err(self.report_method_error(expr.hir_id, rcvr_t, error, expected, false)) - } - }; + let guar = self.report_method_error(expr.hir_id, rcvr_t, error, expected, false); - // Call the generic checker. - self.check_method_argument_types(segment.ident.span, expr, method, args, expected) + let err_inputs = self.err_args(args.len(), guar); + let err_output = Ty::new_error(self.tcx, guar); + + self.check_argument_types( + segment.ident.span, + expr, + &err_inputs, + err_output, + NoExpectation, + args, + false, + TupleArgumentsFlag::DontTupleArguments, + None, + ); + + err_output + } + } } /// Checks use `x.use`. diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 72588515f8aa1..c804dc5e7fbad 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -33,7 +33,6 @@ use crate::fn_ctxt::arg_matrix::{ArgMatrix, Compatibility, Error, ExpectedIdx, P use crate::fn_ctxt::infer::FnCall; use crate::gather_locals::Declaration; use crate::inline_asm::InlineAsmCtxt; -use crate::method::MethodCallee; use crate::method::probe::IsSuggestion; use crate::method::probe::Mode::MethodCall; use crate::method::probe::ProbeScope::TraitsInScope; @@ -127,55 +126,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - pub(in super::super) fn check_method_argument_types( - &self, - sp: Span, - expr: &'tcx hir::Expr<'tcx>, - method: Result, ErrorGuaranteed>, - args_no_rcvr: &'tcx [hir::Expr<'tcx>], - expected: Expectation<'tcx>, - ) -> Ty<'tcx> { - let has_error = match method { - Ok(method) => method.args.error_reported().and(method.sig.error_reported()), - Err(guar) => Err(guar), - }; - if let Err(guar) = has_error { - let err_inputs = self.err_args( - method.map_or(args_no_rcvr.len(), |method| method.sig.inputs().len() - 1), - guar, - ); - let err_output = Ty::new_error(self.tcx, guar); - - self.check_argument_types( - sp, - expr, - &err_inputs, - err_output, - NoExpectation, - args_no_rcvr, - false, - TupleArgumentsFlag::DontTupleArguments, - method.ok().map(|method| method.def_id), - ); - return err_output; - } - - let method = method.unwrap(); - self.check_argument_types( - sp, - expr, - &method.sig.inputs()[1..], - method.sig.output(), - expected, - args_no_rcvr, - method.sig.c_variadic, - TupleArgumentsFlag::DontTupleArguments, - Some(method.def_id), - ); - - method.sig.output() - } - /// Generic function that factors out common logic from function calls, /// method calls and overloaded operators. pub(in super::super) fn check_argument_types(