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 nonsense non-tupled Fn trait error #99942

Merged
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
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1750,6 +1750,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
if let Some(ValuePairs::PolyTraitRefs(exp_found)) = values
&& let ty::Closure(def_id, _) = exp_found.expected.skip_binder().self_ty().kind()
&& let Some(def_id) = def_id.as_local()
&& terr.involves_regions()
{
let span = self.tcx.def_span(def_id);
diag.span_note(span, "this closure does not fulfill the lifetime requirements");
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1935,6 +1935,18 @@ impl<'tcx> TypeTrace<'tcx> {
}
}

pub fn poly_trait_refs(
cause: &ObligationCause<'tcx>,
a_is_expected: bool,
a: ty::PolyTraitRef<'tcx>,
b: ty::PolyTraitRef<'tcx>,
) -> TypeTrace<'tcx> {
TypeTrace {
cause: cause.clone(),
values: PolyTraitRefs(ExpectedFound::new(a_is_expected, a.into(), b.into())),
}
}

pub fn consts(
cause: &ObligationCause<'tcx>,
a_is_expected: bool,
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_middle/src/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,18 @@ pub enum TypeError<'tcx> {
TargetFeatureCast(DefId),
}

impl TypeError<'_> {
pub fn involves_regions(self) -> bool {
match self {
TypeError::RegionsDoesNotOutlive(_, _)
| TypeError::RegionsInsufficientlyPolymorphic(_, _)
| TypeError::RegionsOverlyPolymorphic(_, _)
| TypeError::RegionsPlaceholderMismatch => true,
_ => false,
}
}
}

/// Explains the source of a type err in a short, human readable way. This is meant to be placed
/// in parentheses after some larger message. You should also invoke `note_and_explain_type_err()`
/// afterwards to present additional details, particularly when it comes to lifetime-related
Expand Down
30 changes: 27 additions & 3 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use rustc_hir::intravisit::Visitor;
use rustc_hir::GenericParam;
use rustc_hir::Item;
use rustc_hir::Node;
use rustc_infer::infer::TypeTrace;
use rustc_infer::traits::TraitEngine;
use rustc_middle::traits::select::OverflowError;
use rustc_middle::ty::abstract_const::NotConstEvaluatable;
Expand Down Expand Up @@ -941,20 +942,43 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {

self.reported_closure_mismatch.borrow_mut().insert((span, found_span));

let mut not_tupled = false;

let found = match found_trait_ref.skip_binder().substs.type_at(1).kind() {
ty::Tuple(ref tys) => vec![ArgKind::empty(); tys.len()],
_ => vec![ArgKind::empty()],
_ => {
not_tupled = true;
vec![ArgKind::empty()]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we keep a single ArgKind::empty(), like 1-tuples, instead of an empty vec?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No particular reason, I can fix that I guess.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I know why. This code also handles Generator<T> which takes a generic argument that isn't a tupled set of arguments.

}
};

let expected_ty = expected_trait_ref.skip_binder().substs.type_at(1);
let expected = match expected_ty.kind() {
ty::Tuple(ref tys) => {
tys.iter().map(|t| ArgKind::from_expected_ty(t, Some(span))).collect()
}
_ => vec![ArgKind::Arg("_".to_owned(), expected_ty.to_string())],
_ => {
not_tupled = true;
vec![ArgKind::Arg("_".to_owned(), expected_ty.to_string())]
}
};

if found.len() == expected.len() {
// If this is a `Fn` family trait and either the expected or found
// is not tupled, then fall back to just a regular mismatch error.
// This shouldn't be common unless manually implementing one of the
// traits manually, but don't make it more confusing when it does
// happen.
if Some(expected_trait_ref.def_id()) != tcx.lang_items().gen_trait() && not_tupled {
self.report_and_explain_type_error(
TypeTrace::poly_trait_refs(
&obligation.cause,
true,
expected_trait_ref,
found_trait_ref,
),
ty::error::TypeError::Mismatch,
)
} else if found.len() == expected.len() {
self.report_closure_arg_mismatch(
span,
found_span,
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/mismatched_types/E0631.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![feature(unboxed_closures)]

fn foo<F: Fn(usize)>(_: F) {}
fn bar<F: Fn<usize>>(_: F) {}
fn bar<F: Fn<(usize,)>>(_: F) {}
fn main() {
fn f(_: u64) {}
foo(|_: isize| {}); //~ ERROR type mismatch
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/mismatched_types/E0631.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ LL | bar(|_: isize| {});
note: required by a bound in `bar`
--> $DIR/E0631.rs:4:11
|
LL | fn bar<F: Fn<usize>>(_: F) {}
| ^^^^^^^^^ required by this bound in `bar`
LL | fn bar<F: Fn<(usize,)>>(_: F) {}
| ^^^^^^^^^^^^ required by this bound in `bar`

error[E0631]: type mismatch in function arguments
--> $DIR/E0631.rs:9:9
Expand Down Expand Up @@ -65,8 +65,8 @@ LL | bar(f);
note: required by a bound in `bar`
--> $DIR/E0631.rs:4:11
|
LL | fn bar<F: Fn<usize>>(_: F) {}
| ^^^^^^^^^ required by this bound in `bar`
LL | fn bar<F: Fn<(usize,)>>(_: F) {}
| ^^^^^^^^^^^^ required by this bound in `bar`

error: aborting due to 4 previous errors

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/mismatched_types/closure-arg-count.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![feature(unboxed_closures)]

fn f<F: Fn<usize>>(_: F) {}
fn f<F: Fn<(usize,)>>(_: F) {}
fn main() {
[1, 2, 3].sort_by(|| panic!());
//~^ ERROR closure is expected to take
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/mismatched_types/closure-arg-count.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ LL | f(|| panic!());
note: required by a bound in `f`
--> $DIR/closure-arg-count.rs:3:9
|
LL | fn f<F: Fn<usize>>(_: F) {}
| ^^^^^^^^^ required by this bound in `f`
LL | fn f<F: Fn<(usize,)>>(_: F) {}
| ^^^^^^^^^^^^ required by this bound in `f`
help: consider changing the closure to take and ignore the expected argument
|
LL | f(|_| panic!());
Expand All @@ -74,8 +74,8 @@ LL | f( move || panic!());
note: required by a bound in `f`
--> $DIR/closure-arg-count.rs:3:9
|
LL | fn f<F: Fn<usize>>(_: F) {}
| ^^^^^^^^^ required by this bound in `f`
LL | fn f<F: Fn<(usize,)>>(_: F) {}
| ^^^^^^^^^^^^ required by this bound in `f`
help: consider changing the closure to take and ignore the expected argument
|
LL | f( move |_| panic!());
Expand Down
8 changes: 8 additions & 0 deletions src/test/ui/unboxed-closures/non-tupled-arg-mismatch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#![feature(unboxed_closures)]

fn a<F: Fn<usize>>(f: F) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we just report that a tuple is expected and bail out?

Copy link
Member Author

@compiler-errors compiler-errors Aug 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well making this a hard error in #99943, so I don't want to just introduce a special case error logic here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, reporting that a tuple is expected and bailing out is not gonna work when you have a generic Fn<T> where T is a type param.


fn main() {
a(|_: usize| {});
//~^ ERROR mismatched types
}
17 changes: 17 additions & 0 deletions src/test/ui/unboxed-closures/non-tupled-arg-mismatch.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error[E0308]: mismatched types
--> $DIR/non-tupled-arg-mismatch.rs:6:5
|
LL | a(|_: usize| {});
| ^ types differ
|
= note: expected trait `Fn<usize>`
found trait `Fn<(usize,)>`
note: required by a bound in `a`
--> $DIR/non-tupled-arg-mismatch.rs:3:9
|
LL | fn a<F: Fn<usize>>(f: F) {}
| ^^^^^^^^^ required by this bound in `a`

error: aborting due to previous error

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