From 75337775f776250a3a29c951344186e698c11c75 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 28 Jun 2022 00:12:49 -0700 Subject: [PATCH 1/2] Remove `final_arg_types`, improve tuple wrapping suggestion --- .../src/infer/error_reporting/mod.rs | 4 +- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 310 ++++++++---------- .../suggestions/args-instead-of-tuple.stderr | 10 +- .../ui/tuple/add-tuple-within-arguments.rs | 10 + .../tuple/add-tuple-within-arguments.stderr | 40 +++ src/test/ui/tuple/wrong_argument_ice-2.stderr | 2 +- src/test/ui/tuple/wrong_argument_ice.stderr | 2 +- 7 files changed, 199 insertions(+), 179 deletions(-) create mode 100644 src/test/ui/tuple/add-tuple-within-arguments.rs create mode 100644 src/test/ui/tuple/add-tuple-within-arguments.stderr diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index b94d205488d01..e319f17b0e60d 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -342,8 +342,8 @@ pub fn same_type_modulo_infer<'tcx>(a: Ty<'tcx>, b: Ty<'tcx>) -> bool { ) | (&ty::Infer(ty::InferTy::TyVar(_)), _) | (_, &ty::Infer(ty::InferTy::TyVar(_))) => true, - (&ty::Ref(reg_a, ty_a, mut_a), &ty::Ref(reg_b, ty_b, mut_b)) => { - reg_a == reg_b && mut_a == mut_b && same_type_modulo_infer(*ty_a, *ty_b) + (&ty::Ref(_, ty_a, mut_a), &ty::Ref(_, ty_b, mut_b)) => { + mut_a == mut_b && same_type_modulo_infer(*ty_a, *ty_b) } _ => a == b, } diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index 08df01c0c1a1a..ede2ed5169f80 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -24,7 +24,6 @@ use rustc_infer::infer::error_reporting::{FailureCode, ObligationCauseExt}; use rustc_infer::infer::InferOk; use rustc_infer::infer::TypeTrace; use rustc_middle::ty::adjustment::AllowTwoPhase; -use rustc_middle::ty::error::TypeError; use rustc_middle::ty::fold::TypeFoldable; use rustc_middle::ty::{self, IsSuggestable, Ty, TyCtxt}; use rustc_session::Session; @@ -35,12 +34,6 @@ use rustc_trait_selection::traits::{self, ObligationCauseCode, StatementAsExpres use std::iter; use std::slice; -enum TupleMatchFound { - None, - Single, - /// Beginning and end Span - Multiple(Span, Span), -} impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub(in super::super) fn check_casts(&self) { let mut deferred_cast_checks = self.deferred_cast_checks.borrow_mut(); @@ -216,14 +209,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let minimum_input_count = expected_input_tys.len(); let provided_arg_count = provided_args.len(); - // We'll also want to keep track of the fully coerced argument types, for an awkward hack near the end - // FIXME(compiler-errors): Get rid of this, actually. - let mut final_arg_types: Vec, Ty<'_>)>> = vec![None; provided_arg_count]; - // We introduce a helper function to demand that a given argument satisfy a given input // This is more complicated than just checking type equality, as arguments could be coerced // This version writes those types back so further type checking uses the narrowed types - let demand_compatible = |idx, final_arg_types: &mut Vec, Ty<'tcx>)>>| { + let demand_compatible = |idx| { let formal_input_ty: Ty<'tcx> = formal_input_tys[idx]; let expected_input_ty: Ty<'tcx> = expected_input_tys[idx]; let provided_arg = &provided_args[idx]; @@ -242,9 +231,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // `ExpectHasType(expected_ty)`, or the `formal_ty` otherwise. let coerced_ty = expectation.only_has_type(self).unwrap_or(formal_input_ty); - // Keep track of these for below - final_arg_types[idx] = Some((checked_ty, coerced_ty)); - // Cause selection errors caused by resolving a single argument to point at the // argument and not the call. This lets us customize the span pointed to in the // fulfillment error to be more accurate. @@ -253,16 +239,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.point_at_type_arg_instead_of_call_if_possible(errors, call_expr); self.point_at_arg_instead_of_call_if_possible( errors, - &final_arg_types, call_expr, call_span, provided_args, + &expected_input_tys, ); }); - // Make sure we store the resolved type - final_arg_types[idx] = Some((checked_ty, coerced_ty)); - let coerce_error = self .try_coerce(provided_arg, checked_ty, coerced_ty, AllowTwoPhase::Yes, None) .err(); @@ -320,10 +303,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.point_at_type_arg_instead_of_call_if_possible(errors, call_expr); self.point_at_arg_instead_of_call_if_possible( errors, - &final_arg_types, call_expr, call_span, &provided_args, + &expected_input_tys, ); }) } @@ -352,7 +335,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { continue; } - let compatible = demand_compatible(idx, &mut final_arg_types); + let compatible = demand_compatible(idx); let is_compatible = matches!(compatible, Compatibility::Compatible); compatibility_diagonal[idx] = compatible; @@ -445,72 +428,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { None => "function", }; - let try_tuple_wrap_args = || { - // The case where we expect a single tuple and wrapping all the args - // in parentheses (or adding a comma to already existing parentheses) - // will result in a tuple that satisfies the call. - // This isn't super ideal code, because we copy code from elsewhere - // and somewhat duplicate this. We also delegate to the general type - // mismatch suggestions for the single arg case. - match self.suggested_tuple_wrap(&expected_input_tys, provided_args) { - TupleMatchFound::Single => { - let expected_ty = expected_input_tys[0]; - let provided_ty = final_arg_types[0].map(|ty| ty.0).unwrap(); - let expected_ty = self.resolve_vars_if_possible(expected_ty); - let provided_ty = self.resolve_vars_if_possible(provided_ty); - let cause = &self.misc(provided_args[0].span); - let compatibility = demand_compatible(0, &mut final_arg_types); - let type_error = match compatibility { - Compatibility::Incompatible(Some(error)) => error, - _ => TypeError::Mismatch, - }; - let trace = TypeTrace::types(cause, true, expected_ty, provided_ty); - let mut err = self.report_and_explain_type_error(trace, &type_error); - self.emit_coerce_suggestions( - &mut err, - &provided_args[0], - final_arg_types[0].map(|ty| ty.0).unwrap(), - final_arg_types[0].map(|ty| ty.1).unwrap(), - None, - None, - ); - err.span_label( - full_call_span, - format!("arguments to this {} are incorrect", call_name), - ); - // Call out where the function is defined - label_fn_like(tcx, &mut err, fn_def_id); - err.emit(); - return true; - } - TupleMatchFound::Multiple(start, end) => { - let mut err = tcx.sess.struct_span_err_with_code( - full_call_span, - &format!( - "this {} takes {}{} but {} {} supplied", - call_name, - if c_variadic { "at least " } else { "" }, - potentially_plural_count(minimum_input_count, "argument"), - potentially_plural_count(provided_arg_count, "argument"), - if provided_arg_count == 1 { "was" } else { "were" } - ), - DiagnosticId::Error(err_code.to_owned()), - ); - // Call out where the function is defined - label_fn_like(tcx, &mut err, fn_def_id); - err.multipart_suggestion( - "use parentheses to construct a tuple", - vec![(start, '('.to_string()), (end, ')'.to_string())], - Applicability::MachineApplicable, - ); - err.emit(); - return true; - } - TupleMatchFound::None => {} - } - false - }; - let compatibility_diagonal = IndexVec::from_raw(compatibility_diagonal); let provided_args = IndexVec::from_iter(provided_args.iter().take(if c_variadic { minimum_input_count @@ -541,7 +458,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { c_variadic, err_code, fn_def_id, - try_tuple_wrap_args, ); } } @@ -558,7 +474,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { c_variadic: bool, err_code: &str, fn_def_id: Option, - try_tuple_wrap_args: impl FnOnce() -> bool, ) { // Don't print if it has error types or is just plain `_` fn has_error_or_infer<'tcx>(tys: impl IntoIterator>) -> bool { @@ -578,7 +493,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let (formal_input_ty, expected_input_ty) = formal_and_expected_inputs[expected_idx]; // If either is an error type, we defy the usual convention and consider them to *not* be - // coercible. This prevents our error message heuristic from trying to pass errors into + // coercible. This prevents our error message heuristic from trying to pass errors into // every argument. if (formal_input_ty, expected_input_ty).references_error() { return Compatibility::Incompatible(None); @@ -599,16 +514,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return Compatibility::Incompatible(None); } - let subtyping_result = self - .at(&self.misc(provided_arg.span), self.param_env) - .sup(formal_input_ty, coerced_ty); + // Using probe here, since we don't want this subtyping to affect inference. + let subtyping_error = self.probe(|_| { + self.at(&self.misc(provided_arg.span), self.param_env) + .sup(formal_input_ty, coerced_ty) + .err() + }); // Same as above: if either the coerce type or the checked type is an error type, // consider them *not* compatible. let references_error = (coerced_ty, checked_ty).references_error(); - match (references_error, &subtyping_result) { - (false, Ok(_)) => Compatibility::Compatible, - _ => Compatibility::Incompatible(subtyping_result.err()), + match (references_error, subtyping_error) { + (false, None) => Compatibility::Compatible, + (_, subtyping_error) => Compatibility::Incompatible(subtyping_error), } }; @@ -629,9 +547,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .iter() .map(|expr| { let ty = self - .in_progress_typeck_results - .as_ref() - .unwrap() + .typeck_results .borrow() .expr_ty_adjusted_opt(*expr) .unwrap_or_else(|| tcx.ty_error()); @@ -639,6 +555,97 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }) .collect(); + // First, check if we just need to wrap some arguments in a tuple. + if let Some((mismatch_idx, terr)) = + compatibility_diagonal.iter().enumerate().find_map(|(i, c)| { + if let Compatibility::Incompatible(Some(terr)) = c { Some((i, terr)) } else { None } + }) + { + // Is the first bad expected argument a tuple? + // Do we have as many extra provided arguments as the tuple's length? + // If so, we might have just forgotten to wrap some args in a tuple. + if let Some(ty::Tuple(tys)) = + formal_and_expected_inputs.get(mismatch_idx.into()).map(|tys| tys.1.kind()) + && provided_arg_tys.len() == formal_and_expected_inputs.len() - 1 + tys.len() + { + // Wrap up the N provided arguments starting at this position in a tuple. + let provided_as_tuple = tcx.mk_tup( + provided_arg_tys.iter().map(|(ty, _)| *ty).skip(mismatch_idx).take(tys.len()), + ); + + let mut satisfied = true; + // Check if the newly wrapped tuple + rest of the arguments are compatible. + for ((_, expected_ty), provided_ty) in std::iter::zip( + formal_and_expected_inputs.iter().skip(mismatch_idx), + [provided_as_tuple].into_iter().chain( + provided_arg_tys.iter().map(|(ty, _)| *ty).skip(mismatch_idx + tys.len()), + ), + ) { + if !self.can_coerce(provided_ty, *expected_ty) { + satisfied = false; + break; + } + } + + // If they're compatible, suggest wrapping in an arg, and we're done! + // Take some care with spans, so we don't suggest wrapping a macro's + // innards in parenthesis, for example. + if satisfied + && let Some(lo) = + provided_args[mismatch_idx.into()].span.find_ancestor_inside(error_span) + && let Some(hi) = provided_args[(mismatch_idx + tys.len() - 1).into()] + .span + .find_ancestor_inside(error_span) + { + let mut err; + if tys.len() == 1 { + // A tuple wrap suggestion actually occurs within, + // so don't do anything special here. + err = self.report_and_explain_type_error( + TypeTrace::types( + &self.misc(lo), + true, + formal_and_expected_inputs[mismatch_idx.into()].1, + provided_arg_tys[mismatch_idx.into()].0, + ), + terr, + ); + err.span_label( + full_call_span, + format!("arguments to this {} are incorrect", call_name), + ); + } else { + err = tcx.sess.struct_span_err_with_code( + full_call_span, + &format!( + "this {} takes {}{} but {} {} supplied", + call_name, + if c_variadic { "at least " } else { "" }, + potentially_plural_count( + formal_and_expected_inputs.len(), + "argument" + ), + potentially_plural_count(provided_args.len(), "argument"), + if provided_args.len() == 1 { "was" } else { "were" } + ), + DiagnosticId::Error(err_code.to_owned()), + ); + err.multipart_suggestion_verbose( + "wrap these arguments in parentheses to construct a tuple", + vec![ + (lo.shrink_to_lo(), "(".to_string()), + (hi.shrink_to_hi(), ")".to_string()), + ], + Applicability::MachineApplicable, + ); + }; + label_fn_like(tcx, &mut err, fn_def_id); + err.emit(); + return; + } + } + } + // Okay, so here's where it gets complicated in regards to what errors // we emit and how. // There are 3 different "types" of errors we might encounter. @@ -666,7 +673,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) .note( "we would appreciate a bug report: \ - https://github.com/rust-lang/rust-clippy/issues/new", + https://github.com/rust-lang/rust/issues/new", ) .emit(); } @@ -727,13 +734,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return; } - // Second, let's try tuple wrapping the args. - // FIXME(compiler-errors): This is currently in its own closure because - // I didn't want to factor it out. - if try_tuple_wrap_args() { - return; - } - let mut err = if formal_and_expected_inputs.len() == provided_args.len() { struct_span_err!( tcx.sess, @@ -989,13 +989,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } else { String::new() }; - // FIXME(compiler-errors): Why do we get permutations with the same type? - if expected_ty != provided_ty { - labels.push(( - provided_span, - format!("expected `{}`{}", expected_ty, provided_ty_name), - )); - } + labels.push(( + provided_span, + format!("expected `{}`{}", expected_ty, provided_ty_name), + )); } suggestion_text = match suggestion_text { @@ -1043,10 +1040,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } else { needs_comma = true; } - let suggestion_text = - if let Some(provided_idx) = provided_idx + let suggestion_text = if let Some(provided_idx) = provided_idx && let (_, provided_span) = provided_arg_tys[*provided_idx] - && let Ok(arg_text) = source_map.span_to_snippet(provided_span.source_callsite()) { + && let Ok(arg_text) = + source_map.span_to_snippet(provided_span.source_callsite()) + { arg_text } else { // Propose a placeholder of the correct type @@ -1073,38 +1071,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err.emit(); } - fn suggested_tuple_wrap( - &self, - expected_input_tys: &[Ty<'tcx>], - provided_args: &'tcx [hir::Expr<'tcx>], - ) -> TupleMatchFound { - // Only handle the case where we expect only one tuple arg - let [expected_arg_type] = expected_input_tys[..] else { return TupleMatchFound::None }; - let &ty::Tuple(expected_types) = self.resolve_vars_if_possible(expected_arg_type).kind() - else { return TupleMatchFound::None }; - - // First check that there are the same number of types. - if expected_types.len() != provided_args.len() { - return TupleMatchFound::None; - } - - let supplied_types: Vec<_> = provided_args.iter().map(|arg| self.check_expr(arg)).collect(); - - let all_match = iter::zip(expected_types, supplied_types) - .all(|(expected, supplied)| self.can_eq(self.param_env, expected, supplied).is_ok()); - - if !all_match { - return TupleMatchFound::None; - } - match provided_args { - [] => TupleMatchFound::None, - [_] => TupleMatchFound::Single, - [first, .., last] => { - TupleMatchFound::Multiple(first.span.shrink_to_lo(), last.span.shrink_to_hi()) - } - } - } - // AST fragment checking pub(in super::super) fn check_lit( &self, @@ -1652,10 +1618,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn point_at_arg_instead_of_call_if_possible( &self, errors: &mut Vec>, - final_arg_types: &[Option<(Ty<'tcx>, Ty<'tcx>)>], expr: &'tcx hir::Expr<'tcx>, call_sp: Span, args: &'tcx [hir::Expr<'tcx>], + expected_tys: &[Ty<'tcx>], ) { // We *do not* do this for desugared call spans to keep good diagnostics when involving // the `?` operator. @@ -1688,39 +1654,43 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { (result_code, code) = (code, parent); } } - let self_: ty::subst::GenericArg<'_> = match unpeel_to_top(error.obligation.cause.code()) { - ObligationCauseCode::BuiltinDerivedObligation(code) | - ObligationCauseCode::DerivedObligation(code) => { - code.parent_trait_pred.self_ty().skip_binder().into() - } - ObligationCauseCode::ImplDerivedObligation(code) => { - code.derived.parent_trait_pred.self_ty().skip_binder().into() - } - _ if let ty::PredicateKind::Trait(predicate) = - error.obligation.predicate.kind().skip_binder() => { + let self_: ty::subst::GenericArg<'_> = + match unpeel_to_top(error.obligation.cause.code()) { + ObligationCauseCode::BuiltinDerivedObligation(code) + | ObligationCauseCode::DerivedObligation(code) => { + code.parent_trait_pred.self_ty().skip_binder().into() + } + ObligationCauseCode::ImplDerivedObligation(code) => { + code.derived.parent_trait_pred.self_ty().skip_binder().into() + } + _ if let ty::PredicateKind::Trait(predicate) = + error.obligation.predicate.kind().skip_binder() => + { predicate.self_ty().into() } - _ => continue, - }; + _ => continue, + }; let self_ = self.resolve_vars_if_possible(self_); // Collect the argument position for all arguments that could have caused this // `FulfillmentError`. - let mut referenced_in = final_arg_types - .iter() + let typeck_results = self.typeck_results.borrow(); + let mut referenced_in: Vec<_> = std::iter::zip(expected_tys, args) .enumerate() - .filter_map(|(i, arg)| match arg { - Some((checked_ty, coerce_ty)) => Some([(i, *checked_ty), (i, *coerce_ty)]), - _ => None, + .flat_map(|(idx, (expected_ty, arg))| { + if let Some(arg_ty) = typeck_results.expr_ty_opt(arg) { + vec![(idx, arg_ty), (idx, *expected_ty)] + } else { + vec![] + } }) - .flatten() - .flat_map(|(i, ty)| { + .filter_map(|(i, ty)| { let ty = self.resolve_vars_if_possible(ty); // We walk the argument type because the argument's type could have // been `Option`, but the `FulfillmentError` references `T`. if ty.walk().any(|arg| arg == self_) { Some(i) } else { None } }) - .collect::>(); + .collect(); // Both checked and coerced types could have matched, thus we need to remove // duplicates. diff --git a/src/test/ui/suggestions/args-instead-of-tuple.stderr b/src/test/ui/suggestions/args-instead-of-tuple.stderr index f6d158782dad2..2448a5149654d 100644 --- a/src/test/ui/suggestions/args-instead-of-tuple.stderr +++ b/src/test/ui/suggestions/args-instead-of-tuple.stderr @@ -9,7 +9,7 @@ note: tuple variant defined here | LL | Ok(#[stable(feature = "rust1", since = "1.0.0")] T), | ^^ -help: use parentheses to construct a tuple +help: wrap these arguments in parentheses to construct a tuple | LL | let _: Result<(i32, i8), ()> = Ok((1, 2)); | + + @@ -25,7 +25,7 @@ note: tuple variant defined here | LL | Some(#[stable(feature = "rust1", since = "1.0.0")] T), | ^^^^ -help: use parentheses to construct a tuple +help: wrap these arguments in parentheses to construct a tuple | LL | let _: Option<(i32, i8, &'static str)> = Some((1, 2, "hi")); | + + @@ -97,7 +97,7 @@ note: function defined here | LL | fn two_ints(_: (i32, i32)) { | ^^^^^^^^ ------------- -help: use parentheses to construct a tuple +help: wrap these arguments in parentheses to construct a tuple | LL | two_ints((1, 2)); | + + @@ -113,7 +113,7 @@ note: function defined here | LL | fn with_generic((a, b): (i32, T)) { | ^^^^^^^^^^^^ ---------------- -help: use parentheses to construct a tuple +help: wrap these arguments in parentheses to construct a tuple | LL | with_generic((3, 4)); | + + @@ -129,7 +129,7 @@ note: function defined here | LL | fn with_generic((a, b): (i32, T)) { | ^^^^^^^^^^^^ ---------------- -help: use parentheses to construct a tuple +help: wrap these arguments in parentheses to construct a tuple | LL | with_generic((a, b)); | + + diff --git a/src/test/ui/tuple/add-tuple-within-arguments.rs b/src/test/ui/tuple/add-tuple-within-arguments.rs new file mode 100644 index 0000000000000..089c703fda5c7 --- /dev/null +++ b/src/test/ui/tuple/add-tuple-within-arguments.rs @@ -0,0 +1,10 @@ +fn foo(s: &str, a: (i32, i32), s2: &str) {} + +fn bar(s: &str, a: (&str,), s2: &str) {} + +fn main() { + foo("hi", 1, 2, "hi"); + //~^ ERROR this function takes 3 arguments but 4 arguments were supplied + bar("hi", "hi", "hi"); + //~^ ERROR mismatched types +} diff --git a/src/test/ui/tuple/add-tuple-within-arguments.stderr b/src/test/ui/tuple/add-tuple-within-arguments.stderr new file mode 100644 index 0000000000000..95df96ca0dd4f --- /dev/null +++ b/src/test/ui/tuple/add-tuple-within-arguments.stderr @@ -0,0 +1,40 @@ +error[E0061]: this function takes 3 arguments but 4 arguments were supplied + --> $DIR/add-tuple-within-arguments.rs:6:5 + | +LL | foo("hi", 1, 2, "hi"); + | ^^^ + | +note: function defined here + --> $DIR/add-tuple-within-arguments.rs:1:4 + | +LL | fn foo(s: &str, a: (i32, i32), s2: &str) {} + | ^^^ ------- ------------- -------- +help: wrap these arguments in parentheses to construct a tuple + | +LL | foo("hi", (1, 2), "hi"); + | + + + +error[E0308]: mismatched types + --> $DIR/add-tuple-within-arguments.rs:8:15 + | +LL | bar("hi", "hi", "hi"); + | --- ^^^^ expected tuple, found `&str` + | | + | arguments to this function are incorrect + | + = note: expected tuple `(&str,)` + found reference `&'static str` +note: function defined here + --> $DIR/add-tuple-within-arguments.rs:3:4 + | +LL | fn bar(s: &str, a: (&str,), s2: &str) {} + | ^^^ ------- ---------- -------- +help: use a trailing comma to create a tuple with one element + | +LL | bar("hi", ("hi",), "hi"); + | + ++ + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0061, E0308. +For more information about an error, try `rustc --explain E0061`. diff --git a/src/test/ui/tuple/wrong_argument_ice-2.stderr b/src/test/ui/tuple/wrong_argument_ice-2.stderr index c704ae9934b1c..0c2a4c41461fc 100644 --- a/src/test/ui/tuple/wrong_argument_ice-2.stderr +++ b/src/test/ui/tuple/wrong_argument_ice-2.stderr @@ -9,7 +9,7 @@ note: function defined here | LL | fn test(t: (i32, i32)) {} | ^^^^ ------------- -help: use parentheses to construct a tuple +help: wrap these arguments in parentheses to construct a tuple | LL | test((x.qux(), x.qux())); | + + diff --git a/src/test/ui/tuple/wrong_argument_ice.stderr b/src/test/ui/tuple/wrong_argument_ice.stderr index 2b4cb669f5c7d..ec07f1e70cff6 100644 --- a/src/test/ui/tuple/wrong_argument_ice.stderr +++ b/src/test/ui/tuple/wrong_argument_ice.stderr @@ -9,7 +9,7 @@ note: associated function defined here | LL | pub fn push_back(&mut self, value: T) { | ^^^^^^^^^ -help: use parentheses to construct a tuple +help: wrap these arguments in parentheses to construct a tuple | LL | self.acc.push_back((self.current_provides, self.current_requires)); | + + From 23f3b0dfd04812b03da0df4d5dfad0fef39c6862 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 28 Jun 2022 19:32:12 -0700 Subject: [PATCH 2/2] Don't point at another arg if we're already pointing at one --- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 38 +++++++++++++++---- src/test/ui/proc-macro/signature.stderr | 5 +-- src/test/ui/unsized/unsized-fn-param.stderr | 16 ++++++-- src/test/ui/unsized/unsized3.rs | 1 + src/test/ui/unsized/unsized3.stderr | 35 ++++++++++++++++- 5 files changed, 77 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index ede2ed5169f80..dc49ff90f34b0 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -1629,7 +1629,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return; } - for error in errors { + 'outer: for error in errors { // Only if the cause is somewhere inside the expression we want try to point at arg. // Otherwise, it means that the cause is somewhere else and we should not change // anything because we can break the correct span. @@ -1671,10 +1671,32 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { _ => continue, }; let self_ = self.resolve_vars_if_possible(self_); + let ty_matches_self = |ty: Ty<'tcx>| ty.walk().any(|arg| arg == self_); + + let typeck_results = self.typeck_results.borrow(); + + for (idx, arg) in args.iter().enumerate() { + // Don't adjust the span if we already have a more precise span + // within one of the args. + if arg.span.contains(error.obligation.cause.span) { + let references_arg = + typeck_results.expr_ty_opt(arg).map_or(false, &ty_matches_self) + || expected_tys.get(idx).copied().map_or(false, &ty_matches_self); + if references_arg && !arg.span.from_expansion() { + error.obligation.cause.map_code(|parent_code| { + ObligationCauseCode::FunctionArgumentObligation { + arg_hir_id: args[idx].hir_id, + call_hir_id: expr.hir_id, + parent_code, + } + }) + } + continue 'outer; + } + } // Collect the argument position for all arguments that could have caused this // `FulfillmentError`. - let typeck_results = self.typeck_results.borrow(); let mut referenced_in: Vec<_> = std::iter::zip(expected_tys, args) .enumerate() .flat_map(|(idx, (expected_ty, arg))| { @@ -1688,7 +1710,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let ty = self.resolve_vars_if_possible(ty); // We walk the argument type because the argument's type could have // been `Option`, but the `FulfillmentError` references `T`. - if ty.walk().any(|arg| arg == self_) { Some(i) } else { None } + if ty_matches_self(ty) { Some(i) } else { None } }) .collect(); @@ -1699,18 +1721,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { referenced_in.sort_unstable(); referenced_in.dedup(); - if let (Some(ref_in), None) = (referenced_in.pop(), referenced_in.pop()) { + if let &[idx] = &referenced_in[..] { // Do not point at the inside of a macro. // That would often result in poor error messages. - if args[ref_in].span.from_expansion() { - return; + if args[idx].span.from_expansion() { + continue; } // We make sure that only *one* argument matches the obligation failure // and we assign the obligation's span to its expression's. - error.obligation.cause.span = args[ref_in].span; + error.obligation.cause.span = args[idx].span; error.obligation.cause.map_code(|parent_code| { ObligationCauseCode::FunctionArgumentObligation { - arg_hir_id: args[ref_in].hir_id, + arg_hir_id: args[idx].hir_id, call_hir_id: expr.hir_id, parent_code, } diff --git a/src/test/ui/proc-macro/signature.stderr b/src/test/ui/proc-macro/signature.stderr index 78b0beff0da39..a6bd98ddb1977 100644 --- a/src/test/ui/proc-macro/signature.stderr +++ b/src/test/ui/proc-macro/signature.stderr @@ -5,10 +5,7 @@ LL | / pub unsafe extern "C" fn foo(a: i32, b: u32) -> u32 { LL | | LL | | loop {} LL | | } - | | ^ - | | | - | |_call the function in a closure: `|| unsafe { /* code */ }` - | required by a bound introduced by this call + | |_^ call the function in a closure: `|| unsafe { /* code */ }` | = help: the trait `Fn<(proc_macro::TokenStream,)>` is not implemented for `unsafe extern "C" fn(i32, u32) -> u32 {foo}` = note: unsafe function cannot be called generically without an unsafe block diff --git a/src/test/ui/unsized/unsized-fn-param.stderr b/src/test/ui/unsized/unsized-fn-param.stderr index 3eecca0fa09d9..edf0b89596536 100644 --- a/src/test/ui/unsized/unsized-fn-param.stderr +++ b/src/test/ui/unsized/unsized-fn-param.stderr @@ -2,7 +2,9 @@ error[E0277]: the size for values of type `str` cannot be known at compilation t --> $DIR/unsized-fn-param.rs:11:11 | LL | foo11("bar", &"baz"); - | ^^^^^ doesn't have a size known at compile-time + | ----- ^^^^^ doesn't have a size known at compile-time + | | + | required by a bound introduced by this call | = help: the trait `Sized` is not implemented for `str` = note: required for the cast to the object type `dyn AsRef` @@ -15,7 +17,9 @@ error[E0277]: the size for values of type `str` cannot be known at compilation t --> $DIR/unsized-fn-param.rs:13:19 | LL | foo12(&"bar", "baz"); - | ^^^^^ doesn't have a size known at compile-time + | ----- ^^^^^ doesn't have a size known at compile-time + | | + | required by a bound introduced by this call | = help: the trait `Sized` is not implemented for `str` = note: required for the cast to the object type `dyn AsRef` @@ -28,7 +32,9 @@ error[E0277]: the size for values of type `str` cannot be known at compilation t --> $DIR/unsized-fn-param.rs:16:11 | LL | foo21("bar", &"baz"); - | ^^^^^ doesn't have a size known at compile-time + | ----- ^^^^^ doesn't have a size known at compile-time + | | + | required by a bound introduced by this call | = help: the trait `Sized` is not implemented for `str` = note: required for the cast to the object type `dyn AsRef` @@ -41,7 +47,9 @@ error[E0277]: the size for values of type `str` cannot be known at compilation t --> $DIR/unsized-fn-param.rs:18:19 | LL | foo22(&"bar", "baz"); - | ^^^^^ doesn't have a size known at compile-time + | ----- ^^^^^ doesn't have a size known at compile-time + | | + | required by a bound introduced by this call | = help: the trait `Sized` is not implemented for `str` = note: required for the cast to the object type `dyn AsRef` diff --git a/src/test/ui/unsized/unsized3.rs b/src/test/ui/unsized/unsized3.rs index 39b6583bc4ec4..af76aca2c2958 100644 --- a/src/test/ui/unsized/unsized3.rs +++ b/src/test/ui/unsized/unsized3.rs @@ -44,6 +44,7 @@ fn f9(x1: Box>) { fn f10(x1: Box>) { f5(&(32, *x1)); //~^ ERROR the size for values of type + //~| ERROR the size for values of type } pub fn main() {} diff --git a/src/test/ui/unsized/unsized3.stderr b/src/test/ui/unsized/unsized3.stderr index 65bdc4c2ea352..d64091b15eb1f 100644 --- a/src/test/ui/unsized/unsized3.stderr +++ b/src/test/ui/unsized/unsized3.stderr @@ -100,6 +100,29 @@ LL - fn f9(x1: Box>) { LL + fn f9(x1: Box>) { | +error[E0277]: the size for values of type `X` cannot be known at compilation time + --> $DIR/unsized3.rs:45:9 + | +LL | fn f10(x1: Box>) { + | - this type parameter needs to be `std::marker::Sized` +LL | f5(&(32, *x1)); + | -- ^^^^^^^^^ doesn't have a size known at compile-time + | | + | required by a bound introduced by this call + | +note: required because it appears within the type `S` + --> $DIR/unsized3.rs:28:8 + | +LL | struct S { + | ^ + = note: required because it appears within the type `({integer}, S)` + = note: tuples must have a statically known size to be initialized +help: consider removing the `?Sized` bound to make the type parameter `Sized` + | +LL - fn f10(x1: Box>) { +LL + fn f10(x1: Box>) { + | + error[E0277]: the size for values of type `X` cannot be known at compilation time --> $DIR/unsized3.rs:45:8 | @@ -116,13 +139,21 @@ note: required because it appears within the type `S` LL | struct S { | ^ = note: required because it appears within the type `({integer}, S)` - = note: tuples must have a statically known size to be initialized +note: required by a bound in `f5` + --> $DIR/unsized3.rs:24:7 + | +LL | fn f5(x: &Y) {} + | ^ required by this bound in `f5` help: consider removing the `?Sized` bound to make the type parameter `Sized` | LL - fn f10(x1: Box>) { LL + fn f10(x1: Box>) { | +help: consider relaxing the implicit `Sized` restriction + | +LL | fn f5(x: &Y) {} + | ++++++++ -error: aborting due to 5 previous errors +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0277`.