Skip to content

Commit 12de141

Browse files
veera-sivarajanveera-sivarajan
committed
Suggest impl Trait for References to Bare Trait in Function Header
1 parent 102997a commit 12de141

File tree

4 files changed

+92
-52
lines changed

4 files changed

+92
-52
lines changed

‎compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs‎

Lines changed: 55 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
120120
return;
121121
};
122122
let sugg = self.add_generic_param_suggestion(generics, self_ty.span, &impl_trait_name);
123-
if sugg.is_empty() {
124-
return;
125-
};
126123
diag.multipart_suggestion(
127124
format!(
128125
"alternatively use a blanket implementation to implement `{of_trait_name}` for \
@@ -157,6 +154,10 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
157154
let parent_id = tcx.hir().get_parent_item(self_ty.hir_id).def_id;
158155
// FIXME: If `type_alias_impl_trait` is enabled, also look for `Trait0<Ty = Trait1>`
159156
// and suggest `Trait0<Ty = impl Trait1>`.
157+
// Functions are found in three different contexts.
158+
// 1. Independent functions
159+
// 2. Functions inside trait blocks
160+
// 3. Functions inside impl blocks
160161
let (sig, generics, owner) = match tcx.hir_node_by_def_id(parent_id) {
161162
hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(sig, generics, _), .. }) => {
162163
(sig, generics, None)
@@ -167,13 +168,21 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
167168
owner_id,
168169
..
169170
}) => (sig, generics, Some(tcx.parent(owner_id.to_def_id()))),
171+
hir::Node::ImplItem(hir::ImplItem {
172+
kind: hir::ImplItemKind::Fn(sig, _),
173+
generics,
174+
owner_id,
175+
..
176+
}) => (sig, generics, Some(tcx.parent(owner_id.to_def_id()))),
170177
_ => return false,
171178
};
172179
let Ok(trait_name) = tcx.sess.source_map().span_to_snippet(self_ty.span) else {
173180
return false;
174181
};
175182
let impl_sugg = vec![(self_ty.span.shrink_to_lo(), "impl ".to_string())];
176183
let mut is_downgradable = true;
184+
185+
// Check if trait object is safe for suggesting dynamic dispatch.
177186
let is_object_safe = match self_ty.kind {
178187
hir::TyKind::TraitObject(objects, ..) => {
179188
objects.iter().all(|o| match o.trait_ref.path.res {
@@ -189,8 +198,15 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
189198
}
190199
_ => false,
191200
};
201+
202+
let borrowed = matches!(
203+
tcx.parent_hir_node(self_ty.hir_id),
204+
hir::Node::Ty(hir::Ty { kind: hir::TyKind::Ref(..), .. })
205+
);
206+
207+
// Suggestions for function return type.
192208
if let hir::FnRetTy::Return(ty) = sig.decl.output
193-
&& ty.hir_id == self_ty.hir_id
209+
&& ty.peel_refs().hir_id == self_ty.hir_id
194210
{
195211
let pre = if !is_object_safe {
196212
format!("`{trait_name}` is not object safe, ")
@@ -201,14 +217,26 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
201217
"{pre}use `impl {trait_name}` to return an opaque type, as long as you return a \
202218
single underlying type",
203219
);
220+
204221
diag.multipart_suggestion_verbose(msg, impl_sugg, Applicability::MachineApplicable);
222+
223+
// Suggest `Box<dyn Trait>` for return type
205224
if is_object_safe {
206-
diag.multipart_suggestion_verbose(
207-
"alternatively, you can return an owned trait object",
225+
// If the return type is `&Trait`, we don't want
226+
// the ampersand to be displayed in the `Box<dyn Trait>`
227+
// suggestion.
228+
let suggestion = if borrowed {
229+
vec![(ty.span, format!("Box<dyn {trait_name}>"))]
230+
} else {
208231
vec![
209232
(ty.span.shrink_to_lo(), "Box<dyn ".to_string()),
210233
(ty.span.shrink_to_hi(), ">".to_string()),
211-
],
234+
]
235+
};
236+
237+
diag.multipart_suggestion_verbose(
238+
"alternatively, you can return an owned trait object",
239+
suggestion,
212240
Applicability::MachineApplicable,
213241
);
214242
} else if is_downgradable {
@@ -217,39 +245,43 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
217245
}
218246
return true;
219247
}
248+
249+
// Suggestions for function parameters.
220250
for ty in sig.decl.inputs {
221-
if ty.hir_id != self_ty.hir_id {
251+
if ty.peel_refs().hir_id != self_ty.hir_id {
222252
continue;
223253
}
224254
let sugg = self.add_generic_param_suggestion(generics, self_ty.span, &trait_name);
225-
if !sugg.is_empty() {
226-
diag.multipart_suggestion_verbose(
227-
format!("use a new generic type parameter, constrained by `{trait_name}`"),
228-
sugg,
229-
Applicability::MachineApplicable,
230-
);
231-
diag.multipart_suggestion_verbose(
232-
"you can also use an opaque type, but users won't be able to specify the type \
233-
parameter when calling the `fn`, having to rely exclusively on type inference",
234-
impl_sugg,
235-
Applicability::MachineApplicable,
236-
);
237-
}
255+
diag.multipart_suggestion_verbose(
256+
format!("use a new generic type parameter, constrained by `{trait_name}`"),
257+
sugg,
258+
Applicability::MachineApplicable,
259+
);
260+
diag.multipart_suggestion_verbose(
261+
"you can also use an opaque type, but users won't be able to specify the type \
262+
parameter when calling the `fn`, having to rely exclusively on type inference",
263+
impl_sugg,
264+
Applicability::MachineApplicable,
265+
);
238266
if !is_object_safe {
239267
diag.note(format!("`{trait_name}` it is not object safe, so it can't be `dyn`"));
240268
if is_downgradable {
241269
// We'll emit the object safety error already, with a structured suggestion.
242270
diag.downgrade_to_delayed_bug();
243271
}
244272
} else {
273+
// No ampersand in suggestion if it's borrowed already
274+
let (dyn_str, paren_dyn_str) =
275+
if borrowed { ("dyn ", "(dyn ") } else { ("&dyn ", "&(dyn ") };
276+
245277
let sugg = if let hir::TyKind::TraitObject([_, _, ..], _, _) = self_ty.kind {
246278
// There are more than one trait bound, we need surrounding parentheses.
247279
vec![
248-
(self_ty.span.shrink_to_lo(), "&(dyn ".to_string()),
280+
(self_ty.span.shrink_to_lo(), paren_dyn_str.to_string()),
249281
(self_ty.span.shrink_to_hi(), ")".to_string()),
250282
]
251283
} else {
252-
vec![(self_ty.span.shrink_to_lo(), "&dyn ".to_string())]
284+
vec![(self_ty.span.shrink_to_lo(), dyn_str.to_string())]
253285
};
254286
diag.multipart_suggestion_verbose(
255287
format!(

‎compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4925,24 +4925,32 @@ impl<'v> Visitor<'v> for AwaitsVisitor {
49254925
}
49264926
}
49274927

4928+
/// Suggest a new type parameter name for diagnostic purposes.
4929+
///
4930+
/// `name` is the preferred name you'd like to suggest if it's not in use already.
49284931
pub trait NextTypeParamName {
49294932
fn next_type_param_name(&self, name: Option<&str>) -> String;
49304933
}
49314934

49324935
impl NextTypeParamName for &[hir::GenericParam<'_>] {
49334936
fn next_type_param_name(&self, name: Option<&str>) -> String {
4934-
// This is the list of possible parameter names that we might suggest.
4937+
// Type names are usually single letters in uppercase. So convert the first letter of input string to uppercase.
49354938
let name = name.and_then(|n| n.chars().next()).map(|c| c.to_uppercase().to_string());
49364939
let name = name.as_deref();
4940+
4941+
// This is the list of possible parameter names that we might suggest.
49374942
let possible_names = [name.unwrap_or("T"), "T", "U", "V", "X", "Y", "Z", "A", "B", "C"];
4938-
let used_names = self
4943+
4944+
// Filter out used names based on `filter_fn`.
4945+
let used_names: Vec<Symbol> = self
49394946
.iter()
4940-
.filter_map(|p| match p.name {
4947+
.filter_map(|param| match param.name {
49414948
hir::ParamName::Plain(ident) => Some(ident.name),
49424949
_ => None,
49434950
})
4944-
.collect::<Vec<_>>();
4951+
.collect();
49454952

4953+
// Find a name from `possible_names` that is not in `used_names`.
49464954
possible_names
49474955
.iter()
49484956
.find(|n| !used_names.contains(&Symbol::intern(n)))

‎tests/ui/object-safety/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ impl IceCream {
4343

4444
fn parrot() -> &mut Trait {
4545
//~^ ERROR: missing lifetime specifier
46-
//~| ERROR: cannot return a mutable reference to a bare trait
46+
//~| ERROR: trait objects must include the `dyn` keyword
4747
&mut Type
4848
//~^ ERROR: cannot return reference to temporary value
4949
}
@@ -86,12 +86,12 @@ trait Sing {
8686

8787
fn parrot() -> &mut Trait {
8888
//~^ ERROR: missing lifetime specifier
89-
//~| ERROR: cannot return a mutable reference to a bare trait
89+
//~| ERROR: trait objects must include the `dyn` keyword
9090
&mut Type
9191
//~^ ERROR: cannot return reference to temporary value
9292
}
9393
}
94-
94+
9595
fn foo(_: &Trait) {}
9696
//~^ ERROR: trait objects must include the `dyn` keyword
9797

@@ -134,7 +134,7 @@ fn puppy<'a>() -> &'a Trait {
134134

135135
fn parrot() -> &mut Trait {
136136
//~^ ERROR: missing lifetime specifier
137-
//~| ERROR: cannot return a mutable reference to a bare trait
137+
//~| ERROR: trait objects must include the `dyn` keyword
138138
&mut Type
139139
//~^ ERROR: cannot return reference to temporary value
140140
}

‎tests/ui/object-safety/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.stderr‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -298,8 +298,8 @@ LL | fn cat() -> &Trait;
298298
|
299299
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
300300
|
301-
LL | fn cat<'a>() -> &'a impl Trait;
302-
| ++++ +++++++
301+
LL | fn cat() -> &impl Trait;
302+
| ++++
303303
help: alternatively, you can return an owned trait object
304304
|
305305
LL | fn cat() -> Box<dyn Trait>;
@@ -313,8 +313,8 @@ LL | fn dog<'a>() -> &Trait {
313313
|
314314
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
315315
|
316-
LL | fn dog<'b, 'a>() -> &'b impl Trait {
317-
| +++ +++++++
316+
LL | fn dog<'a>() -> &impl Trait {
317+
| ++++
318318
help: alternatively, you can return an owned trait object
319319
|
320320
LL | fn dog<'a>() -> Box<dyn Trait> {
@@ -350,16 +350,16 @@ help: alternatively, you can return an owned trait object
350350
LL | fn puppy<'a>() -> Box<dyn Trait> {
351351
| ~~~~~~~~~~~~~~
352352

353-
error[E0782]: cannot return a mutable reference to a bare trait
353+
error[E0782]: trait objects must include the `dyn` keyword
354354
--> $DIR/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs:87:25
355355
|
356356
LL | fn parrot() -> &mut Trait {
357357
| ^^^^^
358358
|
359359
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
360360
|
361-
LL | fn parrot() -> impl Trait {
362-
| ~~~~~~~~~~
361+
LL | fn parrot() -> &mut impl Trait {
362+
| ++++
363363
help: alternatively, you can return an owned trait object
364364
|
365365
LL | fn parrot() -> Box<dyn Trait> {
@@ -449,8 +449,8 @@ LL | fn cat() -> &Trait {
449449
|
450450
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
451451
|
452-
LL | fn cat<'a>() -> &'a impl Trait {
453-
| ++++ +++++++
452+
LL | fn cat() -> &impl Trait {
453+
| ++++
454454
help: alternatively, you can return an owned trait object
455455
|
456456
LL | fn cat() -> Box<dyn Trait> {
@@ -464,8 +464,8 @@ LL | fn dog<'a>() -> &Trait {
464464
|
465465
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
466466
|
467-
LL | fn dog<'b, 'a>() -> &'b impl Trait {
468-
| +++ +++++++
467+
LL | fn dog<'a>() -> &impl Trait {
468+
| ++++
469469
help: alternatively, you can return an owned trait object
470470
|
471471
LL | fn dog<'a>() -> Box<dyn Trait> {
@@ -501,16 +501,16 @@ help: alternatively, you can return an owned trait object
501501
LL | fn puppy<'a>() -> Box<dyn Trait> {
502502
| ~~~~~~~~~~~~~~
503503

504-
error[E0782]: cannot return a mutable reference to a bare trait
504+
error[E0782]: trait objects must include the `dyn` keyword
505505
--> $DIR/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs:135:21
506506
|
507507
LL | fn parrot() -> &mut Trait {
508508
| ^^^^^
509509
|
510510
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
511511
|
512-
LL | fn parrot() -> impl Trait {
513-
| ~~~~~~~~~~
512+
LL | fn parrot() -> &mut impl Trait {
513+
| ++++
514514
help: alternatively, you can return an owned trait object
515515
|
516516
LL | fn parrot() -> Box<dyn Trait> {
@@ -600,8 +600,8 @@ LL | fn cat() -> &Trait {
600600
|
601601
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
602602
|
603-
LL | fn cat<'a>() -> &'a impl Trait {
604-
| ++++ +++++++
603+
LL | fn cat() -> &impl Trait {
604+
| ++++
605605
help: alternatively, you can return an owned trait object
606606
|
607607
LL | fn cat() -> Box<dyn Trait> {
@@ -615,8 +615,8 @@ LL | fn dog<'a>() -> &Trait {
615615
|
616616
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
617617
|
618-
LL | fn dog<'b, 'a>() -> &'b impl Trait {
619-
| +++ +++++++
618+
LL | fn dog<'a>() -> &impl Trait {
619+
| ++++
620620
help: alternatively, you can return an owned trait object
621621
|
622622
LL | fn dog<'a>() -> Box<dyn Trait> {
@@ -652,16 +652,16 @@ help: alternatively, you can return an owned trait object
652652
LL | fn puppy<'a>() -> Box<dyn Trait> {
653653
| ~~~~~~~~~~~~~~
654654

655-
error[E0782]: cannot return a mutable reference to a bare trait
655+
error[E0782]: trait objects must include the `dyn` keyword
656656
--> $DIR/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs:44:25
657657
|
658658
LL | fn parrot() -> &mut Trait {
659659
| ^^^^^
660660
|
661661
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
662662
|
663-
LL | fn parrot() -> impl Trait {
664-
| ~~~~~~~~~~
663+
LL | fn parrot() -> &mut impl Trait {
664+
| ++++
665665
help: alternatively, you can return an owned trait object
666666
|
667667
LL | fn parrot() -> Box<dyn Trait> {

0 commit comments

Comments
 (0)