Skip to content

Commit 8520105

Browse files
notriddlenotriddle
committed
fix(rustc_typeck): report function argument errors on matching type
Fixes #90101
1 parent 84c2a85 commit 8520105

33 files changed

+250
-139
lines changed

‎compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1492,6 +1492,9 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
14921492
}
14931493
}
14941494
}
1495+
ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } => {
1496+
self.get_parent_trait_ref(&parent_code)
1497+
}
14951498
_ => None,
14961499
}
14971500
}

‎compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
14221422
while let Some(code) = next_code {
14231423
debug!("maybe_note_obligation_cause_for_async_await: code={:?}", code);
14241424
match code {
1425+
ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } => {
1426+
next_code = Some(parent_code.as_ref());
1427+
}
14251428
ObligationCauseCode::DerivedObligation(derived_obligation)
14261429
| ObligationCauseCode::BuiltinDerivedObligation(derived_obligation)
14271430
| ObligationCauseCode::ImplDerivedObligation(derived_obligation) => {

‎compiler/rustc_typeck/src/check/fn_ctxt/checks.rs

Lines changed: 83 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -370,45 +370,29 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
370370
// `ExpectHasType(expected_ty)`, or the `formal_ty` otherwise.
371371
let coerce_ty = expected.only_has_type(self).unwrap_or(formal_ty);
372372

373+
final_arg_types.push((i, checked_ty, coerce_ty));
374+
373375
// Cause selection errors caused by resolving a single argument to point at the
374376
// argument and not the call. This is otherwise redundant with the `demand_coerce`
375377
// call immediately after, but it lets us customize the span pointed to in the
376378
// fulfillment error to be more accurate.
377379
let _ = self.resolve_vars_with_obligations_and_mutate_fulfillment(
378380
coerce_ty,
379381
|errors| {
380-
// This is not coming from a macro or a `derive`.
381-
if sp.desugaring_kind().is_none()
382-
&& !arg.span.from_expansion()
383-
// Do not change the spans of `async fn`s.
384-
&& !matches!(
385-
expr.kind,
386-
hir::ExprKind::Call(
387-
hir::Expr {
388-
kind: hir::ExprKind::Path(hir::QPath::LangItem(_, _)),
389-
..
390-
},
391-
_
392-
)
393-
) {
394-
for error in errors {
395-
error.obligation.cause.make_mut().span = arg.span;
396-
let code = error.obligation.cause.code.clone();
397-
error.obligation.cause.make_mut().code =
398-
ObligationCauseCode::FunctionArgumentObligation {
399-
arg_hir_id: arg.hir_id,
400-
call_hir_id: expr.hir_id,
401-
parent_code: Lrc::new(code),
402-
};
403-
}
404-
}
382+
self.point_at_type_arg_instead_of_call_if_possible(errors, expr);
383+
self.point_at_arg_instead_of_call_if_possible(
384+
errors,
385+
&final_arg_types,
386+
expr,
387+
sp,
388+
args,
389+
);
405390
},
406391
);
407392

408393
// We're processing function arguments so we definitely want to use
409394
// two-phase borrows.
410395
self.demand_coerce(&arg, checked_ty, coerce_ty, None, AllowTwoPhase::Yes);
411-
final_arg_types.push((i, checked_ty, coerce_ty));
412396

413397
// 3. Relate the expected type and the formal one,
414398
// if the expected type was used for the coercion.
@@ -973,45 +957,79 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
973957
continue;
974958
}
975959

976-
if let ty::PredicateKind::Trait(predicate) =
977-
error.obligation.predicate.kind().skip_binder()
978-
{
979-
// Collect the argument position for all arguments that could have caused this
980-
// `FulfillmentError`.
981-
let mut referenced_in = final_arg_types
982-
.iter()
983-
.map(|&(i, checked_ty, _)| (i, checked_ty))
984-
.chain(final_arg_types.iter().map(|&(i, _, coerced_ty)| (i, coerced_ty)))
985-
.flat_map(|(i, ty)| {
986-
let ty = self.resolve_vars_if_possible(ty);
987-
// We walk the argument type because the argument's type could have
988-
// been `Option<T>`, but the `FulfillmentError` references `T`.
989-
if ty.walk(self.tcx).any(|arg| arg == predicate.self_ty().into()) {
990-
Some(i)
991-
} else {
992-
None
993-
}
994-
})
995-
.collect::<Vec<usize>>();
996-
997-
// Both checked and coerced types could have matched, thus we need to remove
998-
// duplicates.
999-
1000-
// We sort primitive type usize here and can use unstable sort
1001-
referenced_in.sort_unstable();
1002-
referenced_in.dedup();
1003-
1004-
if let (Some(ref_in), None) = (referenced_in.pop(), referenced_in.pop()) {
1005-
// We make sure that only *one* argument matches the obligation failure
1006-
// and we assign the obligation's span to its expression's.
1007-
error.obligation.cause.make_mut().span = args[ref_in].span;
1008-
let code = error.obligation.cause.code.clone();
1009-
error.obligation.cause.make_mut().code =
1010-
ObligationCauseCode::FunctionArgumentObligation {
1011-
arg_hir_id: args[ref_in].hir_id,
1012-
call_hir_id: expr.hir_id,
1013-
parent_code: Lrc::new(code),
1014-
};
960+
// Peel derived obligation, because it's the type that originally
961+
// started this inference chain that matters, not the one we wound
962+
// up with at the end.
963+
fn unpeel_to_top(
964+
mut code: Lrc<ObligationCauseCode<'_>>,
965+
) -> Lrc<ObligationCauseCode<'_>> {
966+
let mut result_code = code.clone();
967+
loop {
968+
let parent = match &*code {
969+
ObligationCauseCode::BuiltinDerivedObligation(c)
970+
| ObligationCauseCode::ImplDerivedObligation(c)
971+
| ObligationCauseCode::DerivedObligation(c) => c.parent_code.clone(),
972+
_ => break,
973+
};
974+
result_code = std::mem::replace(&mut code, parent);
975+
}
976+
result_code
977+
}
978+
let self_: ty::subst::GenericArg<'_> = match &*unpeel_to_top(Lrc::new(error.obligation.cause.code.clone())) {
979+
ObligationCauseCode::BuiltinDerivedObligation(code) |
980+
ObligationCauseCode::ImplDerivedObligation(code) |
981+
ObligationCauseCode::DerivedObligation(code) => {
982+
code.parent_trait_ref.self_ty().skip_binder().into()
983+
}
984+
_ if let ty::PredicateKind::Trait(predicate) =
985+
error.obligation.predicate.kind().skip_binder() => {
986+
predicate.self_ty().into()
987+
}
988+
_ => continue,
989+
};
990+
let self_ = self.resolve_vars_if_possible(self_);
991+
992+
// Collect the argument position for all arguments that could have caused this
993+
// `FulfillmentError`.
994+
let mut referenced_in = final_arg_types
995+
.iter()
996+
.map(|&(i, checked_ty, _)| (i, checked_ty))
997+
.chain(final_arg_types.iter().map(|&(i, _, coerced_ty)| (i, coerced_ty)))
998+
.flat_map(|(i, ty)| {
999+
let ty = self.resolve_vars_if_possible(ty);
1000+
// We walk the argument type because the argument's type could have
1001+
// been `Option<T>`, but the `FulfillmentError` references `T`.
1002+
if ty.walk(self.tcx).any(|arg| arg == self_) { Some(i) } else { None }
1003+
})
1004+
.collect::<Vec<usize>>();
1005+
1006+
// Both checked and coerced types could have matched, thus we need to remove
1007+
// duplicates.
1008+
1009+
// We sort primitive type usize here and can use unstable sort
1010+
referenced_in.sort_unstable();
1011+
referenced_in.dedup();
1012+
1013+
if let (Some(ref_in), None) = (referenced_in.pop(), referenced_in.pop()) {
1014+
// Do not point at the inside of a macro.
1015+
// That would often result in poor error messages.
1016+
if args[ref_in].span.from_expansion() {
1017+
return;
1018+
}
1019+
// We make sure that only *one* argument matches the obligation failure
1020+
// and we assign the obligation's span to its expression's.
1021+
error.obligation.cause.make_mut().span = args[ref_in].span;
1022+
let code = error.obligation.cause.code.clone();
1023+
error.obligation.cause.make_mut().code =
1024+
ObligationCauseCode::FunctionArgumentObligation {
1025+
arg_hir_id: args[ref_in].hir_id,
1026+
call_hir_id: expr.hir_id,
1027+
parent_code: Lrc::new(code),
1028+
};
1029+
} else if error.obligation.cause.make_mut().span == call_sp {
1030+
// Make function calls point at the callee, not the whole thing.
1031+
if let hir::ExprKind::Call(callee, _) = expr.kind {
1032+
error.obligation.cause.make_mut().span = callee.span;
10151033
}
10161034
}
10171035
}

‎src/test/ui/associated-types/associated-types-path-2.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,13 @@ pub fn f1_int_uint() {
2828
pub fn f1_uint_uint() {
2929
f1(2u32, 4u32);
3030
//~^ ERROR `u32: Foo` is not satisfied
31+
//~| ERROR `u32: Foo` is not satisfied
3132
}
3233

3334
pub fn f1_uint_int() {
3435
f1(2u32, 4i32);
3536
//~^ ERROR `u32: Foo` is not satisfied
37+
//~| ERROR `u32: Foo` is not satisfied
3638
}
3739

3840
pub fn f2_int() {

‎src/test/ui/associated-types/associated-types-path-2.stderr

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,10 @@ LL | f1(2i32, 4u32);
1010
| ~~~
1111

1212
error[E0277]: the trait bound `u32: Foo` is not satisfied
13-
--> $DIR/associated-types-path-2.rs:29:14
13+
--> $DIR/associated-types-path-2.rs:29:5
1414
|
1515
LL | f1(2u32, 4u32);
16-
| -- ^^^^ the trait `Foo` is not implemented for `u32`
17-
| |
18-
| required by a bound introduced by this call
16+
| ^^ the trait `Foo` is not implemented for `u32`
1917
|
2018
note: required by a bound in `f1`
2119
--> $DIR/associated-types-path-2.rs:13:14
@@ -24,10 +22,16 @@ LL | pub fn f1<T: Foo>(a: T, x: T::A) {}
2422
| ^^^ required by this bound in `f1`
2523

2624
error[E0277]: the trait bound `u32: Foo` is not satisfied
27-
--> $DIR/associated-types-path-2.rs:34:14
25+
--> $DIR/associated-types-path-2.rs:29:14
26+
|
27+
LL | f1(2u32, 4u32);
28+
| ^^^^ the trait `Foo` is not implemented for `u32`
29+
30+
error[E0277]: the trait bound `u32: Foo` is not satisfied
31+
--> $DIR/associated-types-path-2.rs:35:8
2832
|
2933
LL | f1(2u32, 4i32);
30-
| -- ^^^^ the trait `Foo` is not implemented for `u32`
34+
| -- ^^^^ the trait `Foo` is not implemented for `u32`
3135
| |
3236
| required by a bound introduced by this call
3337
|
@@ -37,8 +41,14 @@ note: required by a bound in `f1`
3741
LL | pub fn f1<T: Foo>(a: T, x: T::A) {}
3842
| ^^^ required by this bound in `f1`
3943

44+
error[E0277]: the trait bound `u32: Foo` is not satisfied
45+
--> $DIR/associated-types-path-2.rs:35:14
46+
|
47+
LL | f1(2u32, 4i32);
48+
| ^^^^ the trait `Foo` is not implemented for `u32`
49+
4050
error[E0308]: mismatched types
41-
--> $DIR/associated-types-path-2.rs:39:18
51+
--> $DIR/associated-types-path-2.rs:41:18
4252
|
4353
LL | let _: i32 = f2(2i32);
4454
| --- ^^^^^^^^ expected `i32`, found `u32`
@@ -50,7 +60,7 @@ help: you can convert a `u32` to an `i32` and panic if the converted value doesn
5060
LL | let _: i32 = f2(2i32).try_into().unwrap();
5161
| ++++++++++++++++++++
5262

53-
error: aborting due to 4 previous errors
63+
error: aborting due to 6 previous errors
5464

5565
Some errors have detailed explanations: E0277, E0308.
5666
For more information about an error, try `rustc --explain E0277`.

‎src/test/ui/async-await/async-fn-nonsend.stderr

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: future cannot be sent between threads safely
2-
--> $DIR/async-fn-nonsend.rs:49:5
2+
--> $DIR/async-fn-nonsend.rs:49:17
33
|
44
LL | assert_send(local_dropped_before_await());
5-
| ^^^^^^^^^^^ future returned by `local_dropped_before_await` is not `Send`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `local_dropped_before_await` is not `Send`
66
|
77
= help: within `impl Future`, the trait `Send` is not implemented for `Rc<()>`
88
note: future is not `Send` as this value is used across an await
@@ -22,10 +22,10 @@ LL | fn assert_send(_: impl Send) {}
2222
| ^^^^ required by this bound in `assert_send`
2323

2424
error: future cannot be sent between threads safely
25-
--> $DIR/async-fn-nonsend.rs:51:5
25+
--> $DIR/async-fn-nonsend.rs:51:17
2626
|
2727
LL | assert_send(non_send_temporary_in_match());
28-
| ^^^^^^^^^^^ future returned by `non_send_temporary_in_match` is not `Send`
28+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `non_send_temporary_in_match` is not `Send`
2929
|
3030
= help: within `impl Future`, the trait `Send` is not implemented for `Rc<()>`
3131
note: future is not `Send` as this value is used across an await
@@ -45,10 +45,10 @@ LL | fn assert_send(_: impl Send) {}
4545
| ^^^^ required by this bound in `assert_send`
4646

4747
error: future cannot be sent between threads safely
48-
--> $DIR/async-fn-nonsend.rs:53:5
48+
--> $DIR/async-fn-nonsend.rs:53:17
4949
|
5050
LL | assert_send(non_sync_with_method_call());
51-
| ^^^^^^^^^^^ future returned by `non_sync_with_method_call` is not `Send`
51+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `non_sync_with_method_call` is not `Send`
5252
|
5353
= help: the trait `Send` is not implemented for `dyn std::fmt::Write`
5454
note: future is not `Send` as this value is used across an await

‎src/test/ui/async-await/issue-64130-1-sync.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: future cannot be shared between threads safely
2-
--> $DIR/issue-64130-1-sync.rs:21:5
2+
--> $DIR/issue-64130-1-sync.rs:21:13
33
|
44
LL | is_sync(bar());
5-
| ^^^^^^^ future returned by `bar` is not `Sync`
5+
| ^^^^^ future returned by `bar` is not `Sync`
66
|
77
= help: within `impl Future`, the trait `Sync` is not implemented for `Foo`
88
note: future is not `Sync` as this value is used across an await

‎src/test/ui/async-await/issue-64130-2-send.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: future cannot be sent between threads safely
2-
--> $DIR/issue-64130-2-send.rs:21:5
2+
--> $DIR/issue-64130-2-send.rs:21:13
33
|
44
LL | is_send(bar());
5-
| ^^^^^^^ future returned by `bar` is not `Send`
5+
| ^^^^^ future returned by `bar` is not `Send`
66
|
77
= help: within `impl Future`, the trait `Send` is not implemented for `Foo`
88
note: future is not `Send` as this value is used across an await

‎src/test/ui/async-await/issue-64130-3-other.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
error[E0277]: the trait bound `Foo: Qux` is not satisfied in `impl Future`
2-
--> $DIR/issue-64130-3-other.rs:24:5
2+
--> $DIR/issue-64130-3-other.rs:24:12
33
|
44
LL | async fn bar() {
55
| - within this `impl Future`
66
...
77
LL | is_qux(bar());
8-
| ^^^^^^ within `impl Future`, the trait `Qux` is not implemented for `Foo`
8+
| ^^^^^ within `impl Future`, the trait `Qux` is not implemented for `Foo`
99
|
1010
note: future does not implement `Qux` as this value is used across an await
1111
--> $DIR/issue-64130-3-other.rs:18:5

‎src/test/ui/async-await/issue-64130-non-send-future-diags.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: future cannot be sent between threads safely
2-
--> $DIR/issue-64130-non-send-future-diags.rs:21:5
2+
--> $DIR/issue-64130-non-send-future-diags.rs:21:13
33
|
44
LL | is_send(foo());
5-
| ^^^^^^^ future returned by `foo` is not `Send`
5+
| ^^^^^ future returned by `foo` is not `Send`
66
|
77
= help: within `impl Future`, the trait `Send` is not implemented for `MutexGuard<'_, u32>`
88
note: future is not `Send` as this value is used across an await

‎src/test/ui/async-await/issue-71137.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: future cannot be sent between threads safely
2-
--> $DIR/issue-71137.rs:20:3
2+
--> $DIR/issue-71137.rs:20:14
33
|
44
LL | fake_spawn(wrong_mutex());
5-
| ^^^^^^^^^^ future returned by `wrong_mutex` is not `Send`
5+
| ^^^^^^^^^^^^^ future returned by `wrong_mutex` is not `Send`
66
|
77
= help: within `impl Future`, the trait `Send` is not implemented for `MutexGuard<'_, i32>`
88
note: future is not `Send` as this value is used across an await

‎src/test/ui/async-await/issues/issue-67893.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: generator cannot be sent between threads safely
2-
--> $DIR/issue-67893.rs:9:5
2+
--> $DIR/issue-67893.rs:9:7
33
|
44
LL | g(issue_67893::run())
5-
| ^ generator is not `Send`
5+
| ^^^^^^^^^^^^^^^^^^ generator is not `Send`
66
|
77
= help: within `impl Future`, the trait `Send` is not implemented for `MutexGuard<'_, ()>`
88
note: required by a bound in `g`

‎src/test/ui/async-await/pin-needed-to-poll-2.stderr

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
error[E0277]: `PhantomPinned` cannot be unpinned
2-
--> $DIR/pin-needed-to-poll-2.rs:43:9
2+
--> $DIR/pin-needed-to-poll-2.rs:43:18
33
|
44
LL | Pin::new(&mut self.sleep).poll(cx)
5-
| ^^^^^^^^ within `Sleep`, the trait `Unpin` is not implemented for `PhantomPinned`
5+
| -------- ^^^^^^^^^^^^^^^ within `Sleep`, the trait `Unpin` is not implemented for `PhantomPinned`
6+
| |
7+
| required by a bound introduced by this call
68
|
79
= note: consider using `Box::pin`
810
note: required because it appears within the type `Sleep`

0 commit comments

Comments
 (0)