rust-lang/rust
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Suggest impl Trait for References to Bare Trait in Function Header

Browse Source
This commit is contained in:
Veera
2024-07-13 12:05:16 -04:00
parent 102997a939
commit 12de141df2
4 changed files with 92 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

78
compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs
View File

@@ -120,9 +120,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
return; return;
}; };
let sugg = self.add_generic_param_suggestion(generics, self_ty.span, &impl_trait_name); let sugg = self.add_generic_param_suggestion(generics, self_ty.span, &impl_trait_name);
if sugg.is_empty() {
return;
};
diag.multipart_suggestion( diag.multipart_suggestion(
format!( format!(
"alternatively use a blanket implementation to implement `{of_trait_name}` for \ "alternatively use a blanket implementation to implement `{of_trait_name}` for \
@@ -157,6 +154,10 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
let parent_id = tcx.hir().get_parent_item(self_ty.hir_id).def_id; let parent_id = tcx.hir().get_parent_item(self_ty.hir_id).def_id;
// FIXME: If `type_alias_impl_trait` is enabled, also look for `Trait0<Ty = Trait1>` // FIXME: If `type_alias_impl_trait` is enabled, also look for `Trait0<Ty = Trait1>`
// and suggest `Trait0<Ty = impl Trait1>`. // and suggest `Trait0<Ty = impl Trait1>`.
// Functions are found in three different contexts.
// 1. Independent functions
// 2. Functions inside trait blocks
// 3. Functions inside impl blocks
let (sig, generics, owner) = match tcx.hir_node_by_def_id(parent_id) { let (sig, generics, owner) = match tcx.hir_node_by_def_id(parent_id) {
hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(sig, generics, _), .. }) => { hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(sig, generics, _), .. }) => {
(sig, generics, None) (sig, generics, None)
@@ -167,6 +168,12 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
owner_id, owner_id,
.. ..
}) => (sig, generics, Some(tcx.parent(owner_id.to_def_id()))), }) => (sig, generics, Some(tcx.parent(owner_id.to_def_id()))),
hir::Node::ImplItem(hir::ImplItem {
kind: hir::ImplItemKind::Fn(sig, _),
generics,
owner_id,
..
}) => (sig, generics, Some(tcx.parent(owner_id.to_def_id()))),
_ => return false, _ => return false,
}; };
let Ok(trait_name) = tcx.sess.source_map().span_to_snippet(self_ty.span) else { let Ok(trait_name) = tcx.sess.source_map().span_to_snippet(self_ty.span) else {
@@ -174,6 +181,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
}; };
let impl_sugg = vec![(self_ty.span.shrink_to_lo(), "impl ".to_string())]; let impl_sugg = vec![(self_ty.span.shrink_to_lo(), "impl ".to_string())];
let mut is_downgradable = true; let mut is_downgradable = true;
// Check if trait object is safe for suggesting dynamic dispatch.
let is_object_safe = match self_ty.kind { let is_object_safe = match self_ty.kind {
hir::TyKind::TraitObject(objects, ..) => { hir::TyKind::TraitObject(objects, ..) => {
objects.iter().all(|o| match o.trait_ref.path.res { objects.iter().all(|o| match o.trait_ref.path.res {
@@ -189,8 +198,15 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
} }
_ => false, _ => false,
}; };
let borrowed = matches!(
tcx.parent_hir_node(self_ty.hir_id),
hir::Node::Ty(hir::Ty { kind: hir::TyKind::Ref(..), .. })
);
// Suggestions for function return type.
if let hir::FnRetTy::Return(ty) = sig.decl.output if let hir::FnRetTy::Return(ty) = sig.decl.output
&& ty.hir_id == self_ty.hir_id && ty.peel_refs().hir_id == self_ty.hir_id
{ {
let pre = if !is_object_safe { let pre = if !is_object_safe {
format!("`{trait_name}` is not object safe, ") format!("`{trait_name}` is not object safe, ")
@@ -201,14 +217,26 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
"{pre}use `impl {trait_name}` to return an opaque type, as long as you return a \ "{pre}use `impl {trait_name}` to return an opaque type, as long as you return a \
single underlying type", single underlying type",
); );
diag.multipart_suggestion_verbose(msg, impl_sugg, Applicability::MachineApplicable); diag.multipart_suggestion_verbose(msg, impl_sugg, Applicability::MachineApplicable);
// Suggest `Box<dyn Trait>` for return type
if is_object_safe { if is_object_safe {
diag.multipart_suggestion_verbose( // If the return type is `&Trait`, we don't want
"alternatively, you can return an owned trait object", // the ampersand to be displayed in the `Box<dyn Trait>`
// suggestion.
let suggestion = if borrowed {
vec![(ty.span, format!("Box<dyn {trait_name}>"))]
} else {
vec![ vec![
(ty.span.shrink_to_lo(), "Box<dyn ".to_string()), (ty.span.shrink_to_lo(), "Box<dyn ".to_string()),
(ty.span.shrink_to_hi(), ">".to_string()), (ty.span.shrink_to_hi(), ">".to_string()),
], ]
};
diag.multipart_suggestion_verbose(
"alternatively, you can return an owned trait object",
suggestion,
Applicability::MachineApplicable, Applicability::MachineApplicable,
); );
} else if is_downgradable { } else if is_downgradable {
@@ -217,24 +245,24 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
} }
return true; return true;
} }
// Suggestions for function parameters.
for ty in sig.decl.inputs { for ty in sig.decl.inputs {
if ty.hir_id != self_ty.hir_id { if ty.peel_refs().hir_id != self_ty.hir_id {
continue; continue;
} }
let sugg = self.add_generic_param_suggestion(generics, self_ty.span, &trait_name); let sugg = self.add_generic_param_suggestion(generics, self_ty.span, &trait_name);
if !sugg.is_empty() { diag.multipart_suggestion_verbose(
diag.multipart_suggestion_verbose( format!("use a new generic type parameter, constrained by `{trait_name}`"),
format!("use a new generic type parameter, constrained by `{trait_name}`"), sugg,
sugg, Applicability::MachineApplicable,
Applicability::MachineApplicable, );
); diag.multipart_suggestion_verbose(
diag.multipart_suggestion_verbose( "you can also use an opaque type, but users won't be able to specify the type \
"you can also use an opaque type, but users won't be able to specify the type \ parameter when calling the `fn`, having to rely exclusively on type inference",
parameter when calling the `fn`, having to rely exclusively on type inference", impl_sugg,
impl_sugg, Applicability::MachineApplicable,
Applicability::MachineApplicable, );
);
}
if !is_object_safe { if !is_object_safe {
diag.note(format!("`{trait_name}` it is not object safe, so it can't be `dyn`")); diag.note(format!("`{trait_name}` it is not object safe, so it can't be `dyn`"));
if is_downgradable { if is_downgradable {
@@ -242,14 +270,18 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
diag.downgrade_to_delayed_bug(); diag.downgrade_to_delayed_bug();
} }
} else { } else {
// No ampersand in suggestion if it's borrowed already
let (dyn_str, paren_dyn_str) =
if borrowed { ("dyn ", "(dyn ") } else { ("&dyn ", "&(dyn ") };
let sugg = if let hir::TyKind::TraitObject([_, _, ..], _, _) = self_ty.kind { let sugg = if let hir::TyKind::TraitObject([_, _, ..], _, _) = self_ty.kind {
// There are more than one trait bound, we need surrounding parentheses. // There are more than one trait bound, we need surrounding parentheses.
vec![ vec![
(self_ty.span.shrink_to_lo(), "&(dyn ".to_string()), (self_ty.span.shrink_to_lo(), paren_dyn_str.to_string()),
(self_ty.span.shrink_to_hi(), ")".to_string()), (self_ty.span.shrink_to_hi(), ")".to_string()),
] ]
} else { } else {
vec![(self_ty.span.shrink_to_lo(), "&dyn ".to_string())] vec![(self_ty.span.shrink_to_lo(), dyn_str.to_string())]
}; };
diag.multipart_suggestion_verbose( diag.multipart_suggestion_verbose(
format!( format!(

16
compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs
View File

@@ -4925,24 +4925,32 @@ impl<'v> Visitor<'v> for AwaitsVisitor {
} }
} }
/// Suggest a new type parameter name for diagnostic purposes.
///
/// `name` is the preferred name you'd like to suggest if it's not in use already.
pub trait NextTypeParamName { pub trait NextTypeParamName {
fn next_type_param_name(&self, name: Option<&str>) -> String; fn next_type_param_name(&self, name: Option<&str>) -> String;
} }
impl NextTypeParamName for &[hir::GenericParam<'_>] { impl NextTypeParamName for &[hir::GenericParam<'_>] {
fn next_type_param_name(&self, name: Option<&str>) -> String { fn next_type_param_name(&self, name: Option<&str>) -> String {
// This is the list of possible parameter names that we might suggest. // Type names are usually single letters in uppercase. So convert the first letter of input string to uppercase.
let name = name.and_then(|n| n.chars().next()).map(|c| c.to_uppercase().to_string()); let name = name.and_then(|n| n.chars().next()).map(|c| c.to_uppercase().to_string());
let name = name.as_deref(); let name = name.as_deref();
// This is the list of possible parameter names that we might suggest.
let possible_names = [name.unwrap_or("T"), "T", "U", "V", "X", "Y", "Z", "A", "B", "C"]; let possible_names = [name.unwrap_or("T"), "T", "U", "V", "X", "Y", "Z", "A", "B", "C"];
let used_names = self
// Filter out used names based on `filter_fn`.
let used_names: Vec<Symbol> = self
.iter() .iter()
.filter_map(|p| match p.name { .filter_map(|param| match param.name {
hir::ParamName::Plain(ident) => Some(ident.name), hir::ParamName::Plain(ident) => Some(ident.name),
_ => None, _ => None,
}) })
.collect::<Vec<_>>(); .collect();
// Find a name from `possible_names` that is not in `used_names`.
possible_names possible_names
.iter() .iter()
.find(|n| !used_names.contains(&Symbol::intern(n))) .find(|n| !used_names.contains(&Symbol::intern(n)))

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

@@ -43,7 +43,7 @@ impl IceCream {
fn parrot() -> &mut Trait { fn parrot() -> &mut Trait {
//~^ ERROR: missing lifetime specifier //~^ ERROR: missing lifetime specifier
//~| ERROR: cannot return a mutable reference to a bare trait //~| ERROR: trait objects must include the `dyn` keyword
&mut Type &mut Type
//~^ ERROR: cannot return reference to temporary value //~^ ERROR: cannot return reference to temporary value
} }
@@ -86,12 +86,12 @@ trait Sing {
fn parrot() -> &mut Trait { fn parrot() -> &mut Trait {
//~^ ERROR: missing lifetime specifier //~^ ERROR: missing lifetime specifier
//~| ERROR: cannot return a mutable reference to a bare trait //~| ERROR: trait objects must include the `dyn` keyword
&mut Type &mut Type
//~^ ERROR: cannot return reference to temporary value //~^ ERROR: cannot return reference to temporary value
} }
} }
fn foo(_: &Trait) {} fn foo(_: &Trait) {}
//~^ ERROR: trait objects must include the `dyn` keyword //~^ ERROR: trait objects must include the `dyn` keyword
@@ -134,7 +134,7 @@ fn puppy<'a>() -> &'a Trait {
fn parrot() -> &mut Trait { fn parrot() -> &mut Trait {
//~^ ERROR: missing lifetime specifier //~^ ERROR: missing lifetime specifier
//~| ERROR: cannot return a mutable reference to a bare trait //~| ERROR: trait objects must include the `dyn` keyword
&mut Type &mut Type
//~^ ERROR: cannot return reference to temporary value //~^ ERROR: cannot return reference to temporary value
} }

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

@@ -298,8 +298,8 @@ LL | fn cat() -> &Trait;
| |
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
| |
LL | fn cat<'a>() -> &'a impl Trait; LL | fn cat() -> &impl Trait;
| ++++ +++++++ | ++++
help: alternatively, you can return an owned trait object help: alternatively, you can return an owned trait object
| |
LL | fn cat() -> Box<dyn Trait>; LL | fn cat() -> Box<dyn Trait>;
@@ -313,8 +313,8 @@ LL | fn dog<'a>() -> &Trait {
| |
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
| |
LL | fn dog<'b, 'a>() -> &'b impl Trait { LL | fn dog<'a>() -> &impl Trait {
| +++ +++++++ | ++++
help: alternatively, you can return an owned trait object help: alternatively, you can return an owned trait object
| |
LL | fn dog<'a>() -> Box<dyn Trait> { LL | fn dog<'a>() -> Box<dyn Trait> {
@@ -350,7 +350,7 @@ help: alternatively, you can return an owned trait object
LL | fn puppy<'a>() -> Box<dyn Trait> { LL | fn puppy<'a>() -> Box<dyn Trait> {
| ~~~~~~~~~~~~~~ | ~~~~~~~~~~~~~~
error[E0782]: cannot return a mutable reference to a bare trait error[E0782]: trait objects must include the `dyn` keyword
--> $DIR/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs:87:25 --> $DIR/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs:87:25
| |
LL | fn parrot() -> &mut Trait { LL | fn parrot() -> &mut Trait {
@@ -358,8 +358,8 @@ LL | fn parrot() -> &mut Trait {
| |
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
| |
LL | fn parrot() -> impl Trait { LL | fn parrot() -> &mut impl Trait {
| ~~~~~~~~~~ | ++++
help: alternatively, you can return an owned trait object help: alternatively, you can return an owned trait object
| |
LL | fn parrot() -> Box<dyn Trait> { LL | fn parrot() -> Box<dyn Trait> {
@@ -449,8 +449,8 @@ LL | fn cat() -> &Trait {
| |
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
| |
LL | fn cat<'a>() -> &'a impl Trait { LL | fn cat() -> &impl Trait {
| ++++ +++++++ | ++++
help: alternatively, you can return an owned trait object help: alternatively, you can return an owned trait object
| |
LL | fn cat() -> Box<dyn Trait> { LL | fn cat() -> Box<dyn Trait> {
@@ -464,8 +464,8 @@ LL | fn dog<'a>() -> &Trait {
| |
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
| |
LL | fn dog<'b, 'a>() -> &'b impl Trait { LL | fn dog<'a>() -> &impl Trait {
| +++ +++++++ | ++++
help: alternatively, you can return an owned trait object help: alternatively, you can return an owned trait object
| |
LL | fn dog<'a>() -> Box<dyn Trait> { LL | fn dog<'a>() -> Box<dyn Trait> {
@@ -501,7 +501,7 @@ help: alternatively, you can return an owned trait object
LL | fn puppy<'a>() -> Box<dyn Trait> { LL | fn puppy<'a>() -> Box<dyn Trait> {
| ~~~~~~~~~~~~~~ | ~~~~~~~~~~~~~~
error[E0782]: cannot return a mutable reference to a bare trait error[E0782]: trait objects must include the `dyn` keyword
--> $DIR/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs:135:21 --> $DIR/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs:135:21
| |
LL | fn parrot() -> &mut Trait { LL | fn parrot() -> &mut Trait {
@@ -509,8 +509,8 @@ LL | fn parrot() -> &mut Trait {
| |
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
| |
LL | fn parrot() -> impl Trait { LL | fn parrot() -> &mut impl Trait {
| ~~~~~~~~~~ | ++++
help: alternatively, you can return an owned trait object help: alternatively, you can return an owned trait object
| |
LL | fn parrot() -> Box<dyn Trait> { LL | fn parrot() -> Box<dyn Trait> {
@@ -600,8 +600,8 @@ LL | fn cat() -> &Trait {
| |
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
| |
LL | fn cat<'a>() -> &'a impl Trait { LL | fn cat() -> &impl Trait {
| ++++ +++++++ | ++++
help: alternatively, you can return an owned trait object help: alternatively, you can return an owned trait object
| |
LL | fn cat() -> Box<dyn Trait> { LL | fn cat() -> Box<dyn Trait> {
@@ -615,8 +615,8 @@ LL | fn dog<'a>() -> &Trait {
| |
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
| |
LL | fn dog<'b, 'a>() -> &'b impl Trait { LL | fn dog<'a>() -> &impl Trait {
| +++ +++++++ | ++++
help: alternatively, you can return an owned trait object help: alternatively, you can return an owned trait object
| |
LL | fn dog<'a>() -> Box<dyn Trait> { LL | fn dog<'a>() -> Box<dyn Trait> {
@@ -652,7 +652,7 @@ help: alternatively, you can return an owned trait object
LL | fn puppy<'a>() -> Box<dyn Trait> { LL | fn puppy<'a>() -> Box<dyn Trait> {
| ~~~~~~~~~~~~~~ | ~~~~~~~~~~~~~~
error[E0782]: cannot return a mutable reference to a bare trait error[E0782]: trait objects must include the `dyn` keyword
--> $DIR/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs:44:25 --> $DIR/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs:44:25
| |
LL | fn parrot() -> &mut Trait { LL | fn parrot() -> &mut Trait {
@@ -660,8 +660,8 @@ LL | fn parrot() -> &mut Trait {
| |
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
| |
LL | fn parrot() -> impl Trait { LL | fn parrot() -> &mut impl Trait {
| ~~~~~~~~~~ | ++++
help: alternatively, you can return an owned trait object help: alternatively, you can return an owned trait object
| |
LL | fn parrot() -> Box<dyn Trait> { LL | fn parrot() -> Box<dyn Trait> {