Skip to content

Commit

Permalink
Rollup merge of #94438 - compiler-errors:check-method-inputs-once, r=…
Browse files Browse the repository at this point in the history
…davidtwco

Check method input expressions once

If the user mistakenly forgets to wrap their method args in a tuple, then the compiler tries to check that  types within the tuple match the expression args. This means we call `check_expr` once within this diagnostic code, so when we check the expr once again in `demand_compatible`, we attempt to apply expr adjustments twice, leading to ICEs.

This PR attempts to fix this by skipping the expression type check in `demand_compatible` if we have detected an method arg mismatch at all.

This does lead to a single UI test regressing slightly, due to a diagnostic disappearing, though I don't know if it is generally meaningful to even raise an type error after noting that the argument count is incorrect in a function call, since the user might be providing (in-context) meaningless expressions to the wrong method.

I can adjust this to be a bit more targeted (to just skip checking exprs in `demand_compatible` in the tuple case) if this UI test regression is a problem.

fixes #94334
cc #94291

Also drive-by fixup of `.node_type(expr.hir_id)` to `.expr_ty(expr)`, since that method exists.
  • Loading branch information
matthiaskrgr authored Mar 1, 2022
2 parents 8d6f527 + 7543067 commit 2353e83
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 24 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return None;
};

let self_ty = self.typeck_results.borrow().node_type(method_expr[0].hir_id);
let self_ty = self.typeck_results.borrow().expr_ty(&method_expr[0]);
let self_ty = format!("{:?}", self_ty);
let name = method_path.ident.name;
let is_as_ref_able = (self_ty.starts_with("&std::option::Option")
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1517,7 +1517,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
} else {
self.check_expr_has_type_or_error(base_expr, adt_ty, |_| {
let base_ty = self.typeck_results.borrow().node_type(base_expr.hir_id);
let base_ty = self.typeck_results.borrow().expr_ty(*base_expr);
let same_adt = match (adt_ty.kind(), base_ty.kind()) {
(ty::Adt(adt, _), ty::Adt(base_adt, _)) if adt == base_adt => true,
_ => false,
Expand Down
9 changes: 0 additions & 9 deletions compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,15 +313,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) => {
// A reborrow has no effect before a dereference.
}
// Catch cases which have Deref(None)
// having them slip to bug! causes ICE
// see #94291 for more info
(&[Adjustment { kind: Adjust::Deref(None), .. }], _) => {
self.tcx.sess.delay_span_bug(
DUMMY_SP,
&format!("Can't compose Deref(None) expressions"),
)
}
// FIXME: currently we never try to compose autoderefs
// and ReifyFnPointer/UnsafeFnPointer, but we could.
_ => bug!(
Expand Down
19 changes: 14 additions & 5 deletions compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let expected_arg_count = formal_input_tys.len();

// expected_count, arg_count, error_code, sugg_unit, sugg_tuple_wrap_args
let mut error: Option<(usize, usize, &str, bool, Option<FnArgsAsTuple<'_>>)> = None;
let mut arg_count_error: Option<(usize, usize, &str, bool, Option<FnArgsAsTuple<'_>>)> =
None;

// If the arguments should be wrapped in a tuple (ex: closures), unwrap them here
let (formal_input_tys, expected_input_tys) = if tuple_arguments == TupleArguments {
Expand All @@ -143,7 +144,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ty::Tuple(arg_types) => {
// Argument length differs
if arg_types.len() != provided_args.len() {
error = Some((arg_types.len(), provided_args.len(), "E0057", false, None));
arg_count_error =
Some((arg_types.len(), provided_args.len(), "E0057", false, None));
}
let expected_input_tys = match expected_input_tys.get(0) {
Some(&ty) => match ty.kind() {
Expand Down Expand Up @@ -174,7 +176,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if supplied_arg_count >= expected_arg_count {
(formal_input_tys.to_vec(), expected_input_tys)
} else {
error = Some((expected_arg_count, supplied_arg_count, "E0060", false, None));
arg_count_error =
Some((expected_arg_count, supplied_arg_count, "E0060", false, None));
(self.err_args(supplied_arg_count), vec![])
}
} else {
Expand All @@ -198,7 +201,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let sugg_tuple_wrap_args = self.suggested_tuple_wrap(expected_input_tys, provided_args);

error = Some((
arg_count_error = Some((
expected_arg_count,
supplied_arg_count,
"E0061",
Expand Down Expand Up @@ -231,6 +234,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// 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<Option<(Ty<'tcx>, Ty<'tcx>)>>| {
// Do not check argument compatibility if the number of args do not match
if arg_count_error.is_some() {
return;
}

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];
Expand Down Expand Up @@ -328,7 +336,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

// If there was an error in parameter count, emit that here
if let Some((expected_count, arg_count, err_code, sugg_unit, sugg_tuple_wrap_args)) = error
if let Some((expected_count, arg_count, err_code, sugg_unit, sugg_tuple_wrap_args)) =
arg_count_error
{
let (span, start_span, args, ctor_of) = match &call_expr.kind {
hir::ExprKind::Call(
Expand Down
1 change: 0 additions & 1 deletion src/test/ui/mismatched_types/overloaded-calls-bad.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,4 @@ fn main() {
//~^ ERROR this function takes 1 argument but 0 arguments were supplied
let ans = s("burma", "shave");
//~^ ERROR this function takes 1 argument but 2 arguments were supplied
//~| ERROR mismatched types
}
8 changes: 1 addition & 7 deletions src/test/ui/mismatched_types/overloaded-calls-bad.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,6 @@ note: associated function defined here
LL | extern "rust-call" fn call_mut(&mut self, args: Args) -> Self::Output;
| ^^^^^^^^

error[E0308]: mismatched types
--> $DIR/overloaded-calls-bad.rs:31:17
|
LL | let ans = s("burma", "shave");
| ^^^^^^^ expected `isize`, found `&str`

error[E0057]: this function takes 1 argument but 2 arguments were supplied
--> $DIR/overloaded-calls-bad.rs:31:15
|
Expand All @@ -38,7 +32,7 @@ note: associated function defined here
LL | extern "rust-call" fn call_mut(&mut self, args: Args) -> Self::Output;
| ^^^^^^^^

error: aborting due to 4 previous errors
error: aborting due to 3 previous errors

Some errors have detailed explanations: E0057, E0308.
For more information about an error, try `rustc --explain E0057`.
17 changes: 17 additions & 0 deletions src/test/ui/tuple/wrong_argument_ice-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
fn test(t: (i32, i32)) {}

struct Foo;

impl Foo {
fn qux(&self) -> i32 {
0
}
}

fn bar() {
let x = Foo;
test(x.qux(), x.qux());
//~^ ERROR this function takes 1 argument but 2 arguments were supplied
}

fn main() {}
19 changes: 19 additions & 0 deletions src/test/ui/tuple/wrong_argument_ice-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error[E0061]: this function takes 1 argument but 2 arguments were supplied
--> $DIR/wrong_argument_ice-2.rs:13:5
|
LL | test(x.qux(), x.qux());
| ^^^^ ------- ------- supplied 2 arguments
|
note: function defined here
--> $DIR/wrong_argument_ice-2.rs:1:4
|
LL | fn test(t: (i32, i32)) {}
| ^^^^ -------------
help: use parentheses to construct a tuple
|
LL | test((x.qux(), x.qux()));
| + +

error: aborting due to previous error

For more information about this error, try `rustc --explain E0061`.

0 comments on commit 2353e83

Please sign in to comment.