Skip to content

fix(rustc_typeck): report function argument errors on matching type #90181

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

Merged
merged 1 commit into from
Oct 26, 2021
Merged
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
Original file line number Diff line number Diff line change
@@ -1492,6 +1492,9 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
}
}
}
ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } => {
self.get_parent_trait_ref(&parent_code)
}
_ => None,
}
}
Original file line number Diff line number Diff line change
@@ -1422,6 +1422,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
while let Some(code) = next_code {
debug!("maybe_note_obligation_cause_for_async_await: code={:?}", code);
match code {
ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } => {
next_code = Some(parent_code.as_ref());
}
ObligationCauseCode::DerivedObligation(derived_obligation)
| ObligationCauseCode::BuiltinDerivedObligation(derived_obligation)
| ObligationCauseCode::ImplDerivedObligation(derived_obligation) => {
148 changes: 83 additions & 65 deletions compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
@@ -370,45 +370,29 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// `ExpectHasType(expected_ty)`, or the `formal_ty` otherwise.
let coerce_ty = expected.only_has_type(self).unwrap_or(formal_ty);

final_arg_types.push((i, checked_ty, coerce_ty));

// Cause selection errors caused by resolving a single argument to point at the
// argument and not the call. This is otherwise redundant with the `demand_coerce`
// call immediately after, but it lets us customize the span pointed to in the
// fulfillment error to be more accurate.
let _ = self.resolve_vars_with_obligations_and_mutate_fulfillment(
coerce_ty,
|errors| {
// This is not coming from a macro or a `derive`.
if sp.desugaring_kind().is_none()
&& !arg.span.from_expansion()
// Do not change the spans of `async fn`s.
&& !matches!(
expr.kind,
hir::ExprKind::Call(
hir::Expr {
kind: hir::ExprKind::Path(hir::QPath::LangItem(_, _)),
..
},
_
)
) {
for error in errors {
error.obligation.cause.make_mut().span = arg.span;
let code = error.obligation.cause.code.clone();
error.obligation.cause.make_mut().code =
ObligationCauseCode::FunctionArgumentObligation {
arg_hir_id: arg.hir_id,
call_hir_id: expr.hir_id,
parent_code: Lrc::new(code),
};
}
}
self.point_at_type_arg_instead_of_call_if_possible(errors, expr);
self.point_at_arg_instead_of_call_if_possible(
errors,
&final_arg_types,
expr,
sp,
args,
);
},
);

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

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

if let ty::PredicateKind::Trait(predicate) =
error.obligation.predicate.kind().skip_binder()
{
// Collect the argument position for all arguments that could have caused this
// `FulfillmentError`.
let mut referenced_in = final_arg_types
.iter()
.map(|&(i, checked_ty, _)| (i, checked_ty))
.chain(final_arg_types.iter().map(|&(i, _, coerced_ty)| (i, coerced_ty)))
.flat_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<T>`, but the `FulfillmentError` references `T`.
if ty.walk(self.tcx).any(|arg| arg == predicate.self_ty().into()) {
Some(i)
} else {
None
}
})
.collect::<Vec<usize>>();

// Both checked and coerced types could have matched, thus we need to remove
// duplicates.

// We sort primitive type usize here and can use unstable sort
referenced_in.sort_unstable();
referenced_in.dedup();

if let (Some(ref_in), None) = (referenced_in.pop(), referenced_in.pop()) {
// 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.make_mut().span = args[ref_in].span;
let code = error.obligation.cause.code.clone();
error.obligation.cause.make_mut().code =
ObligationCauseCode::FunctionArgumentObligation {
arg_hir_id: args[ref_in].hir_id,
call_hir_id: expr.hir_id,
parent_code: Lrc::new(code),
};
// Peel derived obligation, because it's the type that originally
// started this inference chain that matters, not the one we wound
// up with at the end.
fn unpeel_to_top(
mut code: Lrc<ObligationCauseCode<'_>>,
) -> Lrc<ObligationCauseCode<'_>> {
let mut result_code = code.clone();
loop {
let parent = match &*code {
ObligationCauseCode::BuiltinDerivedObligation(c)
| ObligationCauseCode::ImplDerivedObligation(c)
| ObligationCauseCode::DerivedObligation(c) => c.parent_code.clone(),
_ => break,
};
result_code = std::mem::replace(&mut code, parent);
}
result_code
}
let self_: ty::subst::GenericArg<'_> = match &*unpeel_to_top(Lrc::new(error.obligation.cause.code.clone())) {
ObligationCauseCode::BuiltinDerivedObligation(code) |
ObligationCauseCode::ImplDerivedObligation(code) |
ObligationCauseCode::DerivedObligation(code) => {
code.parent_trait_ref.self_ty().skip_binder().into()
}
_ if let ty::PredicateKind::Trait(predicate) =
error.obligation.predicate.kind().skip_binder() => {
predicate.self_ty().into()
}
_ => 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()
.map(|&(i, checked_ty, _)| (i, checked_ty))
.chain(final_arg_types.iter().map(|&(i, _, coerced_ty)| (i, coerced_ty)))
.flat_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<T>`, but the `FulfillmentError` references `T`.
if ty.walk(self.tcx).any(|arg| arg == self_) { Some(i) } else { None }
})
.collect::<Vec<usize>>();

// Both checked and coerced types could have matched, thus we need to remove
// duplicates.

// We sort primitive type usize here and can use unstable sort
referenced_in.sort_unstable();
referenced_in.dedup();

if let (Some(ref_in), None) = (referenced_in.pop(), referenced_in.pop()) {
// 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;
}
// 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.make_mut().span = args[ref_in].span;
let code = error.obligation.cause.code.clone();
error.obligation.cause.make_mut().code =
ObligationCauseCode::FunctionArgumentObligation {
arg_hir_id: args[ref_in].hir_id,
call_hir_id: expr.hir_id,
parent_code: Lrc::new(code),
};
} else if error.obligation.cause.make_mut().span == call_sp {
// Make function calls point at the callee, not the whole thing.
if let hir::ExprKind::Call(callee, _) = expr.kind {
error.obligation.cause.make_mut().span = callee.span;
}
}
}
2 changes: 2 additions & 0 deletions src/test/ui/associated-types/associated-types-path-2.rs
Original file line number Diff line number Diff line change
@@ -28,11 +28,13 @@ pub fn f1_int_uint() {
pub fn f1_uint_uint() {
f1(2u32, 4u32);
//~^ ERROR `u32: Foo` is not satisfied
//~| ERROR `u32: Foo` is not satisfied
}

pub fn f1_uint_int() {
f1(2u32, 4i32);
//~^ ERROR `u32: Foo` is not satisfied
//~| ERROR `u32: Foo` is not satisfied
}

pub fn f2_int() {
26 changes: 18 additions & 8 deletions src/test/ui/associated-types/associated-types-path-2.stderr
Original file line number Diff line number Diff line change
@@ -10,12 +10,10 @@ LL | f1(2i32, 4u32);
| ~~~

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

error[E0277]: the trait bound `u32: Foo` is not satisfied
--> $DIR/associated-types-path-2.rs:34:14
--> $DIR/associated-types-path-2.rs:29:14
|
LL | f1(2u32, 4u32);
| ^^^^ the trait `Foo` is not implemented for `u32`

error[E0277]: the trait bound `u32: Foo` is not satisfied
--> $DIR/associated-types-path-2.rs:35:8
|
LL | f1(2u32, 4i32);
| -- ^^^^ the trait `Foo` is not implemented for `u32`
| -- ^^^^ the trait `Foo` is not implemented for `u32`
| |
| required by a bound introduced by this call
|
@@ -37,8 +41,14 @@ note: required by a bound in `f1`
LL | pub fn f1<T: Foo>(a: T, x: T::A) {}
| ^^^ required by this bound in `f1`

error[E0277]: the trait bound `u32: Foo` is not satisfied
--> $DIR/associated-types-path-2.rs:35:14
|
LL | f1(2u32, 4i32);
| ^^^^ the trait `Foo` is not implemented for `u32`

error[E0308]: mismatched types
--> $DIR/associated-types-path-2.rs:39:18
--> $DIR/associated-types-path-2.rs:41:18
|
LL | let _: i32 = f2(2i32);
| --- ^^^^^^^^ expected `i32`, found `u32`
@@ -50,7 +60,7 @@ help: you can convert a `u32` to an `i32` and panic if the converted value doesn
LL | let _: i32 = f2(2i32).try_into().unwrap();
| ++++++++++++++++++++

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

Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.
12 changes: 6 additions & 6 deletions src/test/ui/async-await/async-fn-nonsend.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: future cannot be sent between threads safely
--> $DIR/async-fn-nonsend.rs:49:5
--> $DIR/async-fn-nonsend.rs:49:17
|
LL | assert_send(local_dropped_before_await());
| ^^^^^^^^^^^ future returned by `local_dropped_before_await` is not `Send`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `local_dropped_before_await` is not `Send`
|
= help: within `impl Future`, the trait `Send` is not implemented for `Rc<()>`
note: future is not `Send` as this value is used across an await
@@ -22,10 +22,10 @@ LL | fn assert_send(_: impl Send) {}
| ^^^^ required by this bound in `assert_send`

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

error: future cannot be sent between threads safely
--> $DIR/async-fn-nonsend.rs:53:5
--> $DIR/async-fn-nonsend.rs:53:17
|
LL | assert_send(non_sync_with_method_call());
| ^^^^^^^^^^^ future returned by `non_sync_with_method_call` is not `Send`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `non_sync_with_method_call` is not `Send`
|
= help: the trait `Send` is not implemented for `dyn std::fmt::Write`
note: future is not `Send` as this value is used across an await
4 changes: 2 additions & 2 deletions src/test/ui/async-await/issue-64130-1-sync.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: future cannot be shared between threads safely
--> $DIR/issue-64130-1-sync.rs:21:5
--> $DIR/issue-64130-1-sync.rs:21:13
|
LL | is_sync(bar());
| ^^^^^^^ future returned by `bar` is not `Sync`
| ^^^^^ future returned by `bar` is not `Sync`
|
= help: within `impl Future`, the trait `Sync` is not implemented for `Foo`
note: future is not `Sync` as this value is used across an await
4 changes: 2 additions & 2 deletions src/test/ui/async-await/issue-64130-2-send.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: future cannot be sent between threads safely
--> $DIR/issue-64130-2-send.rs:21:5
--> $DIR/issue-64130-2-send.rs:21:13
|
LL | is_send(bar());
| ^^^^^^^ future returned by `bar` is not `Send`
| ^^^^^ future returned by `bar` is not `Send`
|
= help: within `impl Future`, the trait `Send` is not implemented for `Foo`
note: future is not `Send` as this value is used across an await
4 changes: 2 additions & 2 deletions src/test/ui/async-await/issue-64130-3-other.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
error[E0277]: the trait bound `Foo: Qux` is not satisfied in `impl Future`
--> $DIR/issue-64130-3-other.rs:24:5
--> $DIR/issue-64130-3-other.rs:24:12
|
LL | async fn bar() {
| - within this `impl Future`
...
LL | is_qux(bar());
| ^^^^^^ within `impl Future`, the trait `Qux` is not implemented for `Foo`
| ^^^^^ within `impl Future`, the trait `Qux` is not implemented for `Foo`
|
note: future does not implement `Qux` as this value is used across an await
--> $DIR/issue-64130-3-other.rs:18:5
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: future cannot be sent between threads safely
--> $DIR/issue-64130-non-send-future-diags.rs:21:5
--> $DIR/issue-64130-non-send-future-diags.rs:21:13
|
LL | is_send(foo());
| ^^^^^^^ future returned by `foo` is not `Send`
| ^^^^^ future returned by `foo` is not `Send`
|
= help: within `impl Future`, the trait `Send` is not implemented for `MutexGuard<'_, u32>`
note: future is not `Send` as this value is used across an await
4 changes: 2 additions & 2 deletions src/test/ui/async-await/issue-71137.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: future cannot be sent between threads safely
--> $DIR/issue-71137.rs:20:3
--> $DIR/issue-71137.rs:20:14
|
LL | fake_spawn(wrong_mutex());
| ^^^^^^^^^^ future returned by `wrong_mutex` is not `Send`
| ^^^^^^^^^^^^^ future returned by `wrong_mutex` is not `Send`
|
= help: within `impl Future`, the trait `Send` is not implemented for `MutexGuard<'_, i32>`
note: future is not `Send` as this value is used across an await
4 changes: 2 additions & 2 deletions src/test/ui/async-await/issues/issue-67893.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: generator cannot be sent between threads safely
--> $DIR/issue-67893.rs:9:5
--> $DIR/issue-67893.rs:9:7
|
LL | g(issue_67893::run())
| ^ generator is not `Send`
| ^^^^^^^^^^^^^^^^^^ generator is not `Send`
|
= help: within `impl Future`, the trait `Send` is not implemented for `MutexGuard<'_, ()>`
note: required by a bound in `g`
6 changes: 4 additions & 2 deletions src/test/ui/async-await/pin-needed-to-poll-2.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
error[E0277]: `PhantomPinned` cannot be unpinned
--> $DIR/pin-needed-to-poll-2.rs:43:9
--> $DIR/pin-needed-to-poll-2.rs:43:18
|
LL | Pin::new(&mut self.sleep).poll(cx)
| ^^^^^^^^ within `Sleep`, the trait `Unpin` is not implemented for `PhantomPinned`
| -------- ^^^^^^^^^^^^^^^ within `Sleep`, the trait `Unpin` is not implemented for `PhantomPinned`
| |
| required by a bound introduced by this call
|
= note: consider using `Box::pin`
note: required because it appears within the type `Sleep`
Loading