Skip to content

Commit 8cb193a

Browse files
estebankestebank
committed
Suggest type param when encountering _ in fn defs
When encountering `_` type placeholder in fn arguments and return type, suggest using generic type parameters. Expand what counts as an inferable return type to slice, array and tuples of `_`.
1 parent 2ba0d2a commit 8cb193a

File tree

8 files changed

+376
-80
lines changed

8 files changed

+376
-80
lines changed

‎src/librustc_typeck/astconv.rs

Lines changed: 80 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use crate::hir::def_id::DefId;
77
use crate::hir::print;
88
use crate::hir::ptr::P;
99
use crate::hir::{self, ExprKind, GenericArg, GenericArgs, HirVec};
10+
use crate::hir::intravisit::{NestedVisitorMap, Visitor};
1011
use crate::lint;
1112
use crate::middle::lang_items::SizedTraitLangItem;
1213
use crate::middle::resolve_lifetime as rl;
@@ -66,6 +67,8 @@ pub trait AstConv<'tcx> {
6667
/// Returns the type to use when a type is omitted.
6768
fn ty_infer(&self, param: Option<&ty::GenericParamDef>, span: Span) -> Ty<'tcx>;
6869

70+
fn allow_ty_infer(&self) -> bool;
71+
6972
/// Returns the const to use when a const is omitted.
7073
fn ct_infer(
7174
&self,
@@ -2593,7 +2596,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
25932596
}
25942597
hir::TyKind::BareFn(ref bf) => {
25952598
require_c_abi_if_c_variadic(tcx, &bf.decl, bf.abi, ast_ty.span);
2596-
tcx.mk_fn_ptr(self.ty_of_fn(bf.unsafety, bf.abi, &bf.decl))
2599+
tcx.mk_fn_ptr(self.ty_of_fn(bf.unsafety, bf.abi, &bf.decl, &[], None))
25972600
}
25982601
hir::TyKind::TraitObject(ref bounds, ref lifetime) => {
25992602
self.conv_object_ty_poly_trait_ref(ast_ty.span, bounds, lifetime)
@@ -2758,14 +2761,55 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
27582761
unsafety: hir::Unsafety,
27592762
abi: abi::Abi,
27602763
decl: &hir::FnDecl<'_>,
2764+
generic_params: &[hir::GenericParam<'_>],
2765+
ident_span: Option<Span>,
27612766
) -> ty::PolyFnSig<'tcx> {
27622767
debug!("ty_of_fn");
27632768

27642769
let tcx = self.tcx();
2765-
let input_tys = decl.inputs.iter().map(|a| self.ty_of_arg(a, None));
27662770

2771+
// We proactively collect all the infered type params to emit a single error per fn def.
2772+
struct PlaceholderHirTyCollector(Vec<Span>);
2773+
impl<'v> Visitor<'v> for PlaceholderHirTyCollector {
2774+
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'v> {
2775+
NestedVisitorMap::None
2776+
}
2777+
fn visit_ty(&mut self, t: &'v hir::Ty<'v>) {
2778+
if let hir::TyKind::Infer = t.kind {
2779+
self.0.push(t.span);
2780+
}
2781+
hir::intravisit::walk_ty(self, t)
2782+
}
2783+
}
2784+
let mut placeholder_types = vec![];
2785+
let mut output_placeholder_types = vec![];
2786+
2787+
let input_tys = decl.inputs.iter().map(|a| {
2788+
let mut visitor = PlaceholderHirTyCollector(vec![]);
2789+
visitor.visit_ty(&a);
2790+
if visitor.0.is_empty() || self.allow_ty_infer() {
2791+
self.ty_of_arg(a, None)
2792+
} else {
2793+
placeholder_types.extend(visitor.0);
2794+
tcx.types.err
2795+
}
2796+
});
27672797
let output_ty = match decl.output {
2768-
hir::Return(ref output) => self.ast_ty_to_ty(output),
2798+
hir::Return(ref output) => {
2799+
let mut visitor = PlaceholderHirTyCollector(vec![]);
2800+
visitor.visit_ty(output);
2801+
let is_infer = if let hir::TyKind::Infer = output.kind {
2802+
true
2803+
} else {
2804+
false
2805+
};
2806+
if (is_infer || !visitor.0.is_empty()) && !self.allow_ty_infer() {
2807+
output_placeholder_types.extend(visitor.0);
2808+
tcx.types.err
2809+
} else {
2810+
self.ast_ty_to_ty(output)
2811+
}
2812+
}
27692813
hir::DefaultReturn(..) => tcx.mk_unit(),
27702814
};
27712815

@@ -2774,6 +2818,39 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
27742818
let bare_fn_ty =
27752819
ty::Binder::bind(tcx.mk_fn_sig(input_tys, output_ty, decl.c_variadic, unsafety, abi));
27762820

2821+
placeholder_types.extend(output_placeholder_types);
2822+
2823+
if !placeholder_types.is_empty() {
2824+
let mut sugg = placeholder_types.iter().cloned()
2825+
.map(|sp| (sp, "T".to_owned()))
2826+
.collect::<Vec<_>>();
2827+
if let Some(span) = ident_span {
2828+
if generic_params.is_empty() {
2829+
sugg.push((span.shrink_to_hi(), "<T>".to_string()));
2830+
} else {
2831+
sugg.push((
2832+
generic_params.iter().last().unwrap().span.shrink_to_hi(),
2833+
", T".to_string(),
2834+
));
2835+
}
2836+
}
2837+
let mut err = struct_span_err!(
2838+
tcx.sess,
2839+
placeholder_types,
2840+
E0121,
2841+
"the type placeholder `_` is not allowed within types on item signatures",
2842+
);
2843+
if ident_span.is_some() {
2844+
err.multipart_suggestion(
2845+
"use type parameters instead",
2846+
sugg,
2847+
Applicability::HasPlaceholders,
2848+
);
2849+
}
2850+
err.emit();
2851+
}
2852+
2853+
27772854
// Find any late-bound regions declared in return type that do
27782855
// not appear in the arguments. These are not well-formed.
27792856
//

‎src/librustc_typeck/check/mod.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -963,7 +963,7 @@ fn typeck_tables_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::TypeckTables<'_> {
963963
let fcx = if let (Some(header), Some(decl)) = (fn_header, fn_decl) {
964964
let fn_sig = if crate::collect::get_infer_ret_ty(&decl.output).is_some() {
965965
let fcx = FnCtxt::new(&inh, param_env, body.value.hir_id);
966-
AstConv::ty_of_fn(&fcx, header.unsafety, header.abi, decl)
966+
AstConv::ty_of_fn(&fcx, header.unsafety, header.abi, decl, &[], None)
967967
} else {
968968
tcx.fn_sig(def_id)
969969
};
@@ -1069,6 +1069,7 @@ fn typeck_tables_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::TypeckTables<'_> {
10691069
let ty = fcx.normalize_ty(span, ty);
10701070
fcx.require_type_is_sized(ty, span, code);
10711071
}
1072+
10721073
fcx.select_all_obligations_or_error();
10731074

10741075
if fn_decl.is_some() {
@@ -2563,6 +2564,10 @@ impl<'a, 'tcx> AstConv<'tcx> for FnCtxt<'a, 'tcx> {
25632564
Some(self.next_region_var(v))
25642565
}
25652566

2567+
fn allow_ty_infer(&self) -> bool {
2568+
true
2569+
}
2570+
25662571
fn ty_infer(&self, param: Option<&ty::GenericParamDef>, span: Span) -> Ty<'tcx> {
25672572
if let Some(param) = param {
25682573
if let GenericArgKind::Type(ty) = self.var_for_def(span, param).unpack() {

‎src/librustc_typeck/collect.rs

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,10 @@ impl AstConv<'tcx> for ItemCtxt<'tcx> {
195195
None
196196
}
197197

198+
fn allow_ty_infer(&self) -> bool {
199+
false
200+
}
201+
198202
fn ty_infer(&self, _: Option<&ty::GenericParamDef>, span: Span) -> Ty<'tcx> {
199203
bad_placeholder_type(self.tcx(), span).emit();
200204

@@ -1699,9 +1703,26 @@ fn find_opaque_ty_constraints(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
16991703
}
17001704
}
17011705

1706+
fn is_infer_ty(ty: &hir::Ty<'_>) -> bool {
1707+
match &ty.kind {
1708+
hir::TyKind::Infer => true,
1709+
hir::TyKind::Slice(ty) | hir::TyKind::Array(ty, _) => is_infer_ty(ty),
1710+
hir::TyKind::Tup(tys)
1711+
if !tys.is_empty()
1712+
&& tys.iter().all(|ty| match ty.kind {
1713+
hir::TyKind::Infer => true,
1714+
_ => false,
1715+
}) =>
1716+
{
1717+
true
1718+
}
1719+
_ => false,
1720+
}
1721+
}
1722+
17021723
pub fn get_infer_ret_ty(output: &'hir hir::FunctionRetTy<'hir>) -> Option<&'hir hir::Ty<'hir>> {
17031724
if let hir::FunctionRetTy::Return(ref ty) = output {
1704-
if let hir::TyKind::Infer = ty.kind {
1725+
if is_infer_ty(ty) {
17051726
return Some(&**ty);
17061727
}
17071728
}
@@ -1719,10 +1740,12 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: DefId) -> ty::PolyFnSig<'_> {
17191740
match tcx.hir().get(hir_id) {
17201741
TraitItem(hir::TraitItem {
17211742
kind: TraitItemKind::Method(sig, TraitMethod::Provided(_)),
1743+
ident,
1744+
generics,
17221745
..
17231746
})
1724-
| ImplItem(hir::ImplItem { kind: ImplItemKind::Method(sig, _), .. })
1725-
| Item(hir::Item { kind: ItemKind::Fn(sig, _, _), .. }) => {
1747+
| ImplItem(hir::ImplItem { kind: ImplItemKind::Method(sig, _), ident, generics, .. })
1748+
| Item(hir::Item { kind: ItemKind::Fn(sig, generics, _), ident, .. }) => {
17261749
match get_infer_ret_ty(&sig.decl.output) {
17271750
Some(ty) => {
17281751
let fn_sig = tcx.typeck_tables_of(def_id).liberated_fn_sigs()[hir_id];
@@ -1731,22 +1754,38 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: DefId) -> ty::PolyFnSig<'_> {
17311754
if ret_ty != tcx.types.err {
17321755
diag.span_suggestion(
17331756
ty.span,
1734-
"replace `_` with the correct return type",
1757+
"replace this with the correct return type",
17351758
ret_ty.to_string(),
17361759
Applicability::MaybeIncorrect,
17371760
);
17381761
}
17391762
diag.emit();
17401763
ty::Binder::bind(fn_sig)
17411764
}
1742-
None => AstConv::ty_of_fn(&icx, sig.header.unsafety, sig.header.abi, &sig.decl),
1765+
None => AstConv::ty_of_fn(
1766+
&icx,
1767+
sig.header.unsafety,
1768+
sig.header.abi,
1769+
&sig.decl,
1770+
&generics.params[..],
1771+
Some(ident.span),
1772+
),
17431773
}
17441774
}
17451775

17461776
TraitItem(hir::TraitItem {
17471777
kind: TraitItemKind::Method(FnSig { header, decl }, _),
1778+
ident,
1779+
generics,
17481780
..
1749-
}) => AstConv::ty_of_fn(&icx, header.unsafety, header.abi, decl),
1781+
}) => AstConv::ty_of_fn(
1782+
&icx,
1783+
header.unsafety,
1784+
header.abi,
1785+
decl,
1786+
&generics.params[..],
1787+
Some(ident.span),
1788+
),
17501789

17511790
ForeignItem(&hir::ForeignItem { kind: ForeignItemKind::Fn(ref fn_decl, _, _), .. }) => {
17521791
let abi = tcx.hir().get_foreign_abi(hir_id);
@@ -2351,7 +2390,7 @@ fn compute_sig_of_foreign_fn_decl<'tcx>(
23512390
} else {
23522391
hir::Unsafety::Unsafe
23532392
};
2354-
let fty = AstConv::ty_of_fn(&ItemCtxt::new(tcx, def_id), unsafety, abi, decl);
2393+
let fty = AstConv::ty_of_fn(&ItemCtxt::new(tcx, def_id), unsafety, abi, decl, &[], None);
23552394

23562395
// Feature gate SIMD types in FFI, since I am not sure that the
23572396
// ABIs are handled at all correctly. -huonw

‎src/test/ui/error-codes/E0121.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | fn foo() -> _ { 5 }
55
| ^
66
| |
77
| not allowed in type signatures
8-
| help: replace `_` with the correct return type: `i32`
8+
| help: replace this with the correct return type: `i32`
99

1010
error[E0121]: the type placeholder `_` is not allowed within types on item signatures
1111
--> $DIR/E0121.rs:3:13

‎src/test/ui/self/self-infer.stderr

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,23 @@ error[E0121]: the type placeholder `_` is not allowed within types on item signa
22
--> $DIR/self-infer.rs:4:16
33
|
44
LL | fn f(self: _) {}
5-
| ^ not allowed in type signatures
5+
| ^
6+
|
7+
help: use type parameters instead
8+
|
9+
LL | fn f<T>(self: T) {}
10+
| ^^^ ^
611

712
error[E0121]: the type placeholder `_` is not allowed within types on item signatures
813
--> $DIR/self-infer.rs:5:17
914
|
1015
LL | fn g(self: &_) {}
11-
| ^ not allowed in type signatures
16+
| ^
17+
|
18+
help: use type parameters instead
19+
|
20+
LL | fn g<T>(self: &T) {}
21+
| ^^^ ^
1222

1323
error: aborting due to 2 previous errors
1424

‎src/test/ui/typeck/typeck_type_placeholder_item.rs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ fn test() -> _ { 5 }
66

77
fn test2() -> (_, _) { (5, 5) }
88
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures
9-
//~^^ ERROR the type placeholder `_` is not allowed within types on item signatures
109

1110
static TEST3: _ = "test";
1211
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures
@@ -59,7 +58,6 @@ pub fn main() {
5958

6059
fn fn_test2() -> (_, _) { (5, 5) }
6160
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures
62-
//~^^ ERROR the type placeholder `_` is not allowed within types on item signatures
6361

6462
static FN_TEST3: _ = "test";
6563
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures
@@ -106,4 +104,28 @@ pub fn main() {
106104
//~^^ ERROR the type placeholder `_` is not allowed within types on item signatures
107105
}
108106

107+
fn fn_test11(_: _) -> (_, _) { panic!() }
108+
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures
109+
//~| ERROR type annotations needed
110+
111+
fn fn_test12(x: i32) -> (_, _) { (x, x) }
112+
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures
113+
114+
fn fn_test13(x: _) -> (i32, _) { (x, x) }
115+
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures
109116
}
117+
118+
trait T {
119+
fn method_test1(&self, x: _);
120+
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures
121+
fn method_test2(&self, x: _) -> _;
122+
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures
123+
fn method_test3(&self) -> _;
124+
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures
125+
fn assoc_fn_test1(x: _);
126+
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures
127+
fn assoc_fn_test2(x: _) -> _;
128+
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures
129+
fn assoc_fn_test3() -> _;
130+
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures
131+
}

0 commit comments

Comments
 (0)