Skip to content

Commit

Permalink
Rollup merge of #106200 - compiler-errors:suggest-impl-trait, r=estebank
Browse files Browse the repository at this point in the history
Suggest `impl Fn*` and `impl Future` in `-> _` return suggestions

Follow-up to #106172, only the last commit is relevant. Can rebase once that PR is landed for easier review.

Suggests `impl Future` and `impl Fn{,Mut,Once}` in `-> _` return suggestions.

r? `@estebank`
  • Loading branch information
matthiaskrgr committed Jan 4, 2023
2 parents c361616 + 89086f7 commit 70468af
Show file tree
Hide file tree
Showing 9 changed files with 220 additions and 55 deletions.
147 changes: 97 additions & 50 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,16 @@
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};
use rustc_hir as hir;
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};
Expand Down Expand Up @@ -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,
Expand All @@ -1223,26 +1223,26 @@ 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
// 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",
);
} 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<Item = {}>", i_ty),
Applicability::MachineApplicable,
);
}
diag.emit();

fn_sig
ty::Binder::dummy(fn_sig)
}
None => <dyn AstConv<'_>>::ty_of_fn(
icx,
Expand All @@ -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<Ty<'tcx>> {
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 `<ret_ty as Iterator>::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<String> {
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::<Vec<_>>().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
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ symbols! {
FromIterator,
FromResidual,
Future,
FutureOutput,
FxHashMap,
FxHashSet,
GlobalAlloc,
Expand Down
1 change: 1 addition & 0 deletions library/core/src/future/future.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use crate::task::{Context, Poll};
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
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/fn/issue-80179.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ 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
//~| NOTE for more information on `Fn` traits and closure types
|| 0
}

Expand Down
6 changes: 4 additions & 2 deletions src/test/ui/fn/issue-80179.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ 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
| ^
| |
| not allowed in type signatures
| help: replace with an appropriate return type: `impl Fn() -> i32`
|
= 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

error: aborting due to 2 previous errors
Expand Down
34 changes: 34 additions & 0 deletions src/test/ui/fn/suggest-return-closure.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
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()
//~| NOTE for more information on `Fn` traits and closure types
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)
//~| NOTE for more information on `Fn` traits and closure types
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
//~| NOTE for more information on `Fn` traits and closure types
|| 1i32
}

fn main() {}
36 changes: 36 additions & 0 deletions src/test/ui/fn/suggest-return-closure.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
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()`
|
= 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: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: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

For more information about this error, try `rustc --explain E0121`.
23 changes: 23 additions & 0 deletions src/test/ui/fn/suggest-return-future.rs
Original file line number Diff line number Diff line change
@@ -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<Output = i32>
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<Output = i32>
async { a().await }
}

fn main() {}
21 changes: 21 additions & 0 deletions src/test/ui/fn/suggest-return-future.stderr
Original file line number Diff line number Diff line change
@@ -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<Output = i32>`

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<Output = i32>`

error: aborting due to 2 previous errors

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

0 comments on commit 70468af

Please sign in to comment.