From a0390463fc27b627a86682722aaa11ff567212b8 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 27 Dec 2022 23:56:46 +0000 Subject: [PATCH 1/2] Suggest more impl Trait on `-> _` --- compiler/rustc_hir_analysis/src/collect.rs | 141 ++++++++++++------- compiler/rustc_span/src/symbol.rs | 1 + library/core/src/future/future.rs | 1 + src/test/ui/fn/issue-80179.rs | 5 +- src/test/ui/fn/issue-80179.stderr | 8 +- src/test/ui/fn/suggest-return-closure.rs | 31 ++++ src/test/ui/fn/suggest-return-closure.stderr | 30 ++++ src/test/ui/fn/suggest-return-future.rs | 23 +++ src/test/ui/fn/suggest-return-future.stderr | 21 +++ 9 files changed, 207 insertions(+), 54 deletions(-) create mode 100644 src/test/ui/fn/suggest-return-closure.rs create mode 100644 src/test/ui/fn/suggest-return-closure.stderr create mode 100644 src/test/ui/fn/suggest-return-future.rs create mode 100644 src/test/ui/fn/suggest-return-future.stderr diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 1ff7429e415..53a58df0916 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -17,6 +17,7 @@ use crate::astconv::AstConv; use crate::check::intrinsic::intrinsic_operation_unsafety; use crate::errors; +use hir::def::DefKind; use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder, ErrorGuaranteed, StashKey}; @@ -24,8 +25,8 @@ use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::{GenericParamKind, Node}; -use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_infer::infer::TyCtxtInferExt; +use rustc_infer::traits::ObligationCause; use rustc_middle::hir::nested_filter; use rustc_middle::ty::query::Providers; use rustc_middle::ty::util::{Discr, IntTypeExt}; @@ -1195,12 +1196,11 @@ fn infer_return_ty_for_fn_sig<'tcx>( ty::ReErased => tcx.lifetimes.re_static, _ => r, }); - let fn_sig = ty::Binder::dummy(fn_sig); let mut visitor = HirPlaceholderCollector::default(); visitor.visit_ty(ty); let mut diag = bad_placeholder(tcx, visitor.0, "return type"); - let ret_ty = fn_sig.skip_binder().output(); + let ret_ty = fn_sig.output(); if ret_ty.is_suggestable(tcx, false) { diag.span_suggestion( ty.span, @@ -1223,6 +1223,13 @@ fn infer_return_ty_for_fn_sig<'tcx>( Applicability::MachineApplicable, ); } + } else if let Some(sugg) = suggest_impl_trait(tcx, ret_ty, ty.span, hir_id, def_id) { + diag.span_suggestion( + ty.span, + "replace with an appropriate return type", + sugg, + Applicability::MachineApplicable, + ); } else if ret_ty.is_closure() { // We're dealing with a closure, so we should suggest using `impl Fn` or trait bounds // to prevent the user from getting a papercut while trying to use the unique closure @@ -1232,17 +1239,10 @@ fn infer_return_ty_for_fn_sig<'tcx>( "for more information on `Fn` traits and closure types, see \ https://doc.rust-lang.org/book/ch13-01-closures.html", ); - } else if let Some(i_ty) = suggest_impl_iterator(tcx, ret_ty, ty.span, hir_id, def_id) { - diag.span_suggestion( - ty.span, - "replace with an appropriate return type", - format!("impl Iterator", i_ty), - Applicability::MachineApplicable, - ); } diag.emit(); - fn_sig + ty::Binder::dummy(fn_sig) } None => >::ty_of_fn( icx, @@ -1256,47 +1256,94 @@ fn infer_return_ty_for_fn_sig<'tcx>( } } -fn suggest_impl_iterator<'tcx>( +fn suggest_impl_trait<'tcx>( tcx: TyCtxt<'tcx>, ret_ty: Ty<'tcx>, span: Span, hir_id: hir::HirId, def_id: LocalDefId, -) -> Option> { - let Some(iter_trait) = tcx.get_diagnostic_item(sym::Iterator) else { return None; }; - let Some(iterator_item) = tcx.get_diagnostic_item(sym::IteratorItem) else { return None; }; - if !tcx - .infer_ctxt() - .build() - .type_implements_trait(iter_trait, [ret_ty], tcx.param_env(def_id)) - .must_apply_modulo_regions() - { - return None; - } - let infcx = tcx.infer_ctxt().build(); - let ocx = ObligationCtxt::new_in_snapshot(&infcx); - // Find the type of `Iterator::Item`. - let origin = TypeVariableOrigin { kind: TypeVariableOriginKind::TypeInference, span }; - let ty_var = infcx.next_ty_var(origin); - let projection = ty::Binder::dummy(ty::PredicateKind::Clause(ty::Clause::Projection( - ty::ProjectionPredicate { - projection_ty: tcx.mk_alias_ty(iterator_item, tcx.mk_substs([ret_ty.into()].iter())), - term: ty_var.into(), - }, - ))); - // Add `::Item = _` obligation. - ocx.register_obligation(crate::traits::Obligation::misc( - tcx, - span, - hir_id, - tcx.param_env(def_id), - projection, - )); - if ocx.select_where_possible().is_empty() - && let item_ty = infcx.resolve_vars_if_possible(ty_var) - && item_ty.is_suggestable(tcx, false) - { - return Some(item_ty); +) -> Option { + let format_as_assoc: fn(_, _, _, _, _) -> _ = + |tcx: TyCtxt<'tcx>, + _: ty::SubstsRef<'tcx>, + trait_def_id: DefId, + assoc_item_def_id: DefId, + item_ty: Ty<'tcx>| { + let trait_name = tcx.item_name(trait_def_id); + let assoc_name = tcx.item_name(assoc_item_def_id); + Some(format!("impl {trait_name}<{assoc_name} = {item_ty}>")) + }; + let format_as_parenthesized: fn(_, _, _, _, _) -> _ = + |tcx: TyCtxt<'tcx>, + substs: ty::SubstsRef<'tcx>, + trait_def_id: DefId, + _: DefId, + item_ty: Ty<'tcx>| { + let trait_name = tcx.item_name(trait_def_id); + let args_tuple = substs.type_at(1); + let ty::Tuple(types) = *args_tuple.kind() else { return None; }; + if !types.is_suggestable(tcx, false) { + return None; + } + let maybe_ret = + if item_ty.is_unit() { String::new() } else { format!(" -> {item_ty}") }; + Some(format!( + "impl {trait_name}({}){maybe_ret}", + types.iter().map(|ty| ty.to_string()).collect::>().join(", ") + )) + }; + + for (trait_def_id, assoc_item_def_id, formatter) in [ + ( + tcx.get_diagnostic_item(sym::Iterator), + tcx.get_diagnostic_item(sym::IteratorItem), + format_as_assoc, + ), + ( + tcx.lang_items().future_trait(), + tcx.get_diagnostic_item(sym::FutureOutput), + format_as_assoc, + ), + (tcx.lang_items().fn_trait(), tcx.lang_items().fn_once_output(), format_as_parenthesized), + ( + tcx.lang_items().fn_mut_trait(), + tcx.lang_items().fn_once_output(), + format_as_parenthesized, + ), + ( + tcx.lang_items().fn_once_trait(), + tcx.lang_items().fn_once_output(), + format_as_parenthesized, + ), + ] { + let Some(trait_def_id) = trait_def_id else { continue; }; + let Some(assoc_item_def_id) = assoc_item_def_id else { continue; }; + if tcx.def_kind(assoc_item_def_id) != DefKind::AssocTy { + continue; + } + let param_env = tcx.param_env(def_id); + let infcx = tcx.infer_ctxt().build(); + let substs = ty::InternalSubsts::for_item(tcx, trait_def_id, |param, _| { + if param.index == 0 { ret_ty.into() } else { infcx.var_for_def(span, param) } + }); + if !infcx.type_implements_trait(trait_def_id, substs, param_env).must_apply_modulo_regions() + { + continue; + } + let ocx = ObligationCtxt::new_in_snapshot(&infcx); + let item_ty = ocx.normalize( + &ObligationCause::misc(span, hir_id), + param_env, + tcx.mk_projection(assoc_item_def_id, substs), + ); + // FIXME(compiler-errors): We may benefit from resolving regions here. + if ocx.select_where_possible().is_empty() + && let item_ty = infcx.resolve_vars_if_possible(item_ty) + && item_ty.is_suggestable(tcx, false) + && let Some(sugg) = formatter(tcx, infcx.resolve_vars_if_possible(substs), trait_def_id, assoc_item_def_id, item_ty) + { + return Some(sugg); + } } None } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 85510fa2c66..5d5f8d6d654 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -193,6 +193,7 @@ FromIterator, FromResidual, Future, + FutureOutput, FxHashMap, FxHashSet, GlobalAlloc, diff --git a/library/core/src/future/future.rs b/library/core/src/future/future.rs index f29d3e1e98b..8c7111cb3ff 100644 --- a/library/core/src/future/future.rs +++ b/library/core/src/future/future.rs @@ -37,6 +37,7 @@ pub trait Future { /// The type of value produced on completion. #[stable(feature = "futures_api", since = "1.36.0")] + #[rustc_diagnostic_item = "FutureOutput"] type Output; /// Attempt to resolve the future to a final value, registering diff --git a/src/test/ui/fn/issue-80179.rs b/src/test/ui/fn/issue-80179.rs index fcef6f1b60e..bea8deadc89 100644 --- a/src/test/ui/fn/issue-80179.rs +++ b/src/test/ui/fn/issue-80179.rs @@ -18,9 +18,8 @@ fn returns_fn_ptr() -> _ { fn returns_closure() -> _ { //~^ ERROR the placeholder `_` is not allowed within types on item signatures for return types [E0121] //~| NOTE not allowed in type signatures -//~| HELP consider using an `Fn`, `FnMut`, or `FnOnce` trait bound -//~| NOTE for more information on `Fn` traits and closure types, see -// https://doc.rust-lang.org/book/ch13-01-closures.html +//~| HELP replace with an appropriate return type +//~| SUGGESTION impl Fn() -> i32 || 0 } diff --git a/src/test/ui/fn/issue-80179.stderr b/src/test/ui/fn/issue-80179.stderr index 2ca4ae982d9..dff8252b371 100644 --- a/src/test/ui/fn/issue-80179.stderr +++ b/src/test/ui/fn/issue-80179.stderr @@ -11,10 +11,10 @@ error[E0121]: the placeholder `_` is not allowed within types on item signatures --> $DIR/issue-80179.rs:18:25 | LL | fn returns_closure() -> _ { - | ^ not allowed in type signatures - | - = help: consider using an `Fn`, `FnMut`, or `FnOnce` trait bound - = note: for more information on `Fn` traits and closure types, see https://doc.rust-lang.org/book/ch13-01-closures.html + | ^ + | | + | not allowed in type signatures + | help: replace with an appropriate return type: `impl Fn() -> i32` error: aborting due to 2 previous errors diff --git a/src/test/ui/fn/suggest-return-closure.rs b/src/test/ui/fn/suggest-return-closure.rs new file mode 100644 index 00000000000..1b9094cea38 --- /dev/null +++ b/src/test/ui/fn/suggest-return-closure.rs @@ -0,0 +1,31 @@ +fn fn_once() -> _ { + //~^ ERROR the placeholder `_` is not allowed within types on item signatures for return types [E0121] + //~| NOTE not allowed in type signatures + //~| HELP replace with an appropriate return type + //~| SUGGESTION impl FnOnce() + let x = String::new(); + || { + drop(x); + } +} + +fn fn_mut() -> _ { + //~^ ERROR the placeholder `_` is not allowed within types on item signatures for return types [E0121] + //~| NOTE not allowed in type signatures + //~| HELP replace with an appropriate return type + //~| SUGGESTION impl FnMut(char) + let x = String::new(); + |c| { + x.push(c); + } +} + +fn fun() -> _ { + //~^ ERROR the placeholder `_` is not allowed within types on item signatures for return types [E0121] + //~| NOTE not allowed in type signatures + //~| HELP replace with an appropriate return type + //~| SUGGESTION impl Fn() -> i32 + || 1i32 +} + +fn main() {} diff --git a/src/test/ui/fn/suggest-return-closure.stderr b/src/test/ui/fn/suggest-return-closure.stderr new file mode 100644 index 00000000000..f4b2c7f5274 --- /dev/null +++ b/src/test/ui/fn/suggest-return-closure.stderr @@ -0,0 +1,30 @@ +error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types + --> $DIR/suggest-return-closure.rs:1:17 + | +LL | fn fn_once() -> _ { + | ^ + | | + | not allowed in type signatures + | help: replace with an appropriate return type: `impl FnOnce()` + +error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types + --> $DIR/suggest-return-closure.rs:12:16 + | +LL | fn fn_mut() -> _ { + | ^ + | | + | not allowed in type signatures + | help: replace with an appropriate return type: `impl FnMut(char)` + +error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types + --> $DIR/suggest-return-closure.rs:23:13 + | +LL | fn fun() -> _ { + | ^ + | | + | not allowed in type signatures + | help: replace with an appropriate return type: `impl Fn() -> i32` + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0121`. diff --git a/src/test/ui/fn/suggest-return-future.rs b/src/test/ui/fn/suggest-return-future.rs new file mode 100644 index 00000000000..750740d9426 --- /dev/null +++ b/src/test/ui/fn/suggest-return-future.rs @@ -0,0 +1,23 @@ +// edition: 2021 + +async fn a() -> i32 { + 0 +} + +fn foo() -> _ { + //~^ ERROR the placeholder `_` is not allowed within types on item signatures for return types [E0121] + //~| NOTE not allowed in type signatures + //~| HELP replace with an appropriate return type + //~| SUGGESTION impl Future + a() +} + +fn bar() -> _ { + //~^ ERROR the placeholder `_` is not allowed within types on item signatures for return types [E0121] + //~| NOTE not allowed in type signatures + //~| HELP replace with an appropriate return type + //~| SUGGESTION impl Future + async { a().await } +} + +fn main() {} diff --git a/src/test/ui/fn/suggest-return-future.stderr b/src/test/ui/fn/suggest-return-future.stderr new file mode 100644 index 00000000000..a4c8b5d8c4b --- /dev/null +++ b/src/test/ui/fn/suggest-return-future.stderr @@ -0,0 +1,21 @@ +error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types + --> $DIR/suggest-return-future.rs:7:13 + | +LL | fn foo() -> _ { + | ^ + | | + | not allowed in type signatures + | help: replace with an appropriate return type: `impl Future` + +error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types + --> $DIR/suggest-return-future.rs:15:13 + | +LL | fn bar() -> _ { + | ^ + | | + | not allowed in type signatures + | help: replace with an appropriate return type: `impl Future` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0121`. From 89086f7d36e3a692b48cae8a408057734f044567 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 4 Jan 2023 00:26:53 +0000 Subject: [PATCH 2/2] Restore Fn trait note --- compiler/rustc_hir_analysis/src/collect.rs | 6 +++--- src/test/ui/fn/issue-80179.rs | 1 + src/test/ui/fn/issue-80179.stderr | 2 ++ src/test/ui/fn/suggest-return-closure.rs | 3 +++ src/test/ui/fn/suggest-return-closure.stderr | 10 ++++++++-- 5 files changed, 17 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 53a58df0916..cf847047c90 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -1231,10 +1231,10 @@ fn infer_return_ty_for_fn_sig<'tcx>( Applicability::MachineApplicable, ); } else if ret_ty.is_closure() { - // We're dealing with a closure, so we should suggest using `impl Fn` or trait bounds - // to prevent the user from getting a papercut while trying to use the unique closure - // syntax (e.g. `[closure@src/lib.rs:2:5: 2:9]`). diag.help("consider using an `Fn`, `FnMut`, or `FnOnce` trait bound"); + } + // Also note how `Fn` traits work just in case! + if ret_ty.is_closure() { diag.note( "for more information on `Fn` traits and closure types, see \ https://doc.rust-lang.org/book/ch13-01-closures.html", diff --git a/src/test/ui/fn/issue-80179.rs b/src/test/ui/fn/issue-80179.rs index bea8deadc89..35e39bebb29 100644 --- a/src/test/ui/fn/issue-80179.rs +++ b/src/test/ui/fn/issue-80179.rs @@ -20,6 +20,7 @@ fn returns_closure() -> _ { //~| NOTE not allowed in type signatures //~| HELP replace with an appropriate return type //~| SUGGESTION impl Fn() -> i32 +//~| NOTE for more information on `Fn` traits and closure types || 0 } diff --git a/src/test/ui/fn/issue-80179.stderr b/src/test/ui/fn/issue-80179.stderr index dff8252b371..f5d6c44db75 100644 --- a/src/test/ui/fn/issue-80179.stderr +++ b/src/test/ui/fn/issue-80179.stderr @@ -15,6 +15,8 @@ LL | fn returns_closure() -> _ { | | | not allowed in type signatures | help: replace with an appropriate return type: `impl Fn() -> i32` + | + = note: for more information on `Fn` traits and closure types, see https://doc.rust-lang.org/book/ch13-01-closures.html error: aborting due to 2 previous errors diff --git a/src/test/ui/fn/suggest-return-closure.rs b/src/test/ui/fn/suggest-return-closure.rs index 1b9094cea38..33daa1ea0b4 100644 --- a/src/test/ui/fn/suggest-return-closure.rs +++ b/src/test/ui/fn/suggest-return-closure.rs @@ -3,6 +3,7 @@ fn fn_once() -> _ { //~| NOTE not allowed in type signatures //~| HELP replace with an appropriate return type //~| SUGGESTION impl FnOnce() + //~| NOTE for more information on `Fn` traits and closure types let x = String::new(); || { drop(x); @@ -14,6 +15,7 @@ fn fn_mut() -> _ { //~| NOTE not allowed in type signatures //~| HELP replace with an appropriate return type //~| SUGGESTION impl FnMut(char) + //~| NOTE for more information on `Fn` traits and closure types let x = String::new(); |c| { x.push(c); @@ -25,6 +27,7 @@ fn fun() -> _ { //~| NOTE not allowed in type signatures //~| HELP replace with an appropriate return type //~| SUGGESTION impl Fn() -> i32 + //~| NOTE for more information on `Fn` traits and closure types || 1i32 } diff --git a/src/test/ui/fn/suggest-return-closure.stderr b/src/test/ui/fn/suggest-return-closure.stderr index f4b2c7f5274..341044469ea 100644 --- a/src/test/ui/fn/suggest-return-closure.stderr +++ b/src/test/ui/fn/suggest-return-closure.stderr @@ -6,24 +6,30 @@ LL | fn fn_once() -> _ { | | | not allowed in type signatures | help: replace with an appropriate return type: `impl FnOnce()` + | + = note: for more information on `Fn` traits and closure types, see https://doc.rust-lang.org/book/ch13-01-closures.html error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types - --> $DIR/suggest-return-closure.rs:12:16 + --> $DIR/suggest-return-closure.rs:13:16 | LL | fn fn_mut() -> _ { | ^ | | | not allowed in type signatures | help: replace with an appropriate return type: `impl FnMut(char)` + | + = note: for more information on `Fn` traits and closure types, see https://doc.rust-lang.org/book/ch13-01-closures.html error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types - --> $DIR/suggest-return-closure.rs:23:13 + --> $DIR/suggest-return-closure.rs:25:13 | LL | fn fun() -> _ { | ^ | | | not allowed in type signatures | help: replace with an appropriate return type: `impl Fn() -> i32` + | + = note: for more information on `Fn` traits and closure types, see https://doc.rust-lang.org/book/ch13-01-closures.html error: aborting due to 3 previous errors