Skip to content
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

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

Merged
merged 1 commit into from
Oct 26, 2021
Merged
Show file tree
Hide file tree
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
Expand Up @@ -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,
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
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
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
estebank marked this conversation as resolved.
Show resolved Hide resolved
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;
notriddle marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down
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
Expand Up @@ -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() {
Expand Down
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
Expand Up @@ -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
Expand All @@ -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
|
Expand All @@ -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`
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
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
Expand Down
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
Expand Down
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
Expand Down
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
Expand Down
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
Expand Down
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`
Expand Down
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`
Expand Down
Loading