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

Suggest returning closure as impl Fn #101019

Merged
merged 2 commits into from
Aug 30, 2022
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
37 changes: 37 additions & 0 deletions compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1536,6 +1536,34 @@ pub trait PrettyPrinter<'tcx>:
}
Ok(self)
}

fn pretty_closure_as_impl(
mut self,
closure: ty::ClosureSubsts<'tcx>,
) -> Result<Self::Const, Self::Error> {
let sig = closure.sig();
let kind = closure.kind_ty().to_opt_closure_kind().unwrap_or(ty::ClosureKind::Fn);

write!(self, "impl ")?;
self.wrap_binder(&sig, |sig, mut cx| {
define_scoped_cx!(cx);

p!(print(kind), "(");
for (i, arg) in sig.inputs()[0].tuple_fields().iter().enumerate() {
if i > 0 {
p!(", ");
}
p!(print(arg));
}
p!(")");

if !sig.output().is_unit() {
p!(" -> ", print(sig.output()));
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if one of the printed types is an APIT?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think APITs are is_suggestable. I can try tho

Copy link
Member Author

Choose a reason for hiding this comment

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

Confirmed that this falls back to the old suggestion and doesn't suggest something like -> Fn() -> impl Fn() or anything like that. Added test for it.

}

Ok(cx)
})
}
}

// HACK(eddyb) boxed to avoid moving around a large struct by-value.
Expand Down Expand Up @@ -2450,6 +2478,11 @@ impl<'tcx> ty::PolyTraitPredicate<'tcx> {
}
}

#[derive(Debug, Copy, Clone, TypeFoldable, TypeVisitable, Lift)]
pub struct PrintClosureAsImpl<'tcx> {
pub closure: ty::ClosureSubsts<'tcx>,
}

forward_display_to_print! {
ty::Region<'tcx>,
Ty<'tcx>,
Expand Down Expand Up @@ -2542,6 +2575,10 @@ define_print_and_forward_display! {
p!(print(self.0.trait_ref.print_only_trait_path()));
}

PrintClosureAsImpl<'tcx> {
p!(pretty_closure_as_impl(self.closure))
}

ty::ParamTy {
p!(write("{}", self.name))
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,10 @@ impl<'tcx> ClosureSubsts<'tcx> {
_ => bug!("closure_sig_as_fn_ptr_ty is not a fn-ptr: {:?}", ty.kind()),
}
}

pub fn print_as_impl_trait(self) -> ty::print::PrintClosureAsImpl<'tcx> {
ty::print::PrintClosureAsImpl { closure: self }
}
}

/// Similar to `ClosureSubsts`; see the above documentation for more.
Expand Down
45 changes: 23 additions & 22 deletions compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,30 +506,30 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.resolve_numeric_literals_with_default(self.resolve_vars_if_possible(found));
// Only suggest changing the return type for methods that
// haven't set a return type at all (and aren't `fn main()` or an impl).
match (
&fn_decl.output,
found.is_suggestable(self.tcx, false),
can_suggest,
expected.is_unit(),
) {
(&hir::FnRetTy::DefaultReturn(span), true, true, true) => {
err.subdiagnostic(AddReturnTypeSuggestion::Add { span, found });
true
}
(&hir::FnRetTy::DefaultReturn(span), false, true, true) => {
// FIXME: if `found` could be `impl Iterator` or `impl Fn*`, we should suggest
// that.
err.subdiagnostic(AddReturnTypeSuggestion::MissingHere { span });
true
}
(&hir::FnRetTy::DefaultReturn(span), _, false, true) => {
match &fn_decl.output {
&hir::FnRetTy::DefaultReturn(span) if expected.is_unit() && !can_suggest => {
// `fn main()` must return `()`, do not suggest changing return type
err.subdiagnostic(ExpectedReturnTypeLabel::Unit { span });
true
return true;
}
// expectation was caused by something else, not the default return
(&hir::FnRetTy::DefaultReturn(_), _, _, false) => false,
(&hir::FnRetTy::Return(ref ty), _, _, _) => {
&hir::FnRetTy::DefaultReturn(span) if expected.is_unit() => {
if found.is_suggestable(self.tcx, false) {
err.subdiagnostic(AddReturnTypeSuggestion::Add { span, found: found.to_string() });
return true;
} else if let ty::Closure(_, substs) = found.kind()
// FIXME(compiler-errors): Get better at printing binders...
&& let closure = substs.as_closure()
&& closure.sig().is_suggestable(self.tcx, false)
{
err.subdiagnostic(AddReturnTypeSuggestion::Add { span, found: closure.print_as_impl_trait().to_string() });
return true;
} else {
// FIXME: if `found` could be `impl Iterator` we should suggest that.
err.subdiagnostic(AddReturnTypeSuggestion::MissingHere { span });
return true
}
}
&hir::FnRetTy::Return(ref ty) => {
// Only point to return type if the expected type is the return type, as if they
// are not, the expectation must have been caused by something else.
debug!("suggest_missing_return_type: return type {:?} node {:?}", ty, ty.kind);
Expand All @@ -546,9 +546,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.try_suggest_return_impl_trait(err, expected, ty, fn_id);
return true;
}
false
}
_ => {}
}
false
}

/// check whether the return type is a generic type with a trait bound
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_typeck/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ pub struct AddressOfTemporaryTaken {
}

#[derive(SessionSubdiagnostic)]
pub enum AddReturnTypeSuggestion<'tcx> {
pub enum AddReturnTypeSuggestion {
#[suggestion(
typeck::add_return_type_add,
code = "-> {found} ",
Expand All @@ -204,7 +204,7 @@ pub enum AddReturnTypeSuggestion<'tcx> {
Add {
#[primary_span]
span: Span,
found: Ty<'tcx>,
found: String,
},
#[suggestion(
typeck::add_return_type_missing_here,
Expand Down
13 changes: 13 additions & 0 deletions src/test/ui/suggestions/return-closures.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
fn foo() {
//~^ HELP try adding a return type
|x: &i32| 1i32
//~^ ERROR mismatched types
}

fn bar(i: impl Sized) {
//~^ HELP a return type might be missing here
|| i
//~^ ERROR mismatched types
}

fn main() {}
27 changes: 27 additions & 0 deletions src/test/ui/suggestions/return-closures.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
error[E0308]: mismatched types
--> $DIR/return-closures.rs:3:5
|
LL | fn foo() {
| - help: try adding a return type: `-> impl for<'r> Fn(&'r i32) -> i32`
LL |
LL | |x: &i32| 1i32
| ^^^^^^^^^^^^^^ expected `()`, found closure
|
= note: expected unit type `()`
found closure `[closure@$DIR/return-closures.rs:3:5: 3:14]`

error[E0308]: mismatched types
--> $DIR/return-closures.rs:9:5
|
LL | fn bar(i: impl Sized) {
| - help: a return type might be missing here: `-> _`
LL |
LL | || i
| ^^^^ expected `()`, found closure
|
= note: expected unit type `()`
found closure `[closure@$DIR/return-closures.rs:9:5: 9:7]`

error: aborting due to 2 previous errors

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