diff options
4 files changed, 92 insertions, 52 deletions
diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs index 29c71c3fa50..a480d0b3886 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs @@ -120,9 +120,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { return; }; let sugg = self.add_generic_param_suggestion(generics, self_ty.span, &impl_trait_name); - if sugg.is_empty() { - return; - }; diag.multipart_suggestion( format!( "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; // FIXME: If `type_alias_impl_trait` is enabled, also look for `Trait0<Ty = 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) { hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(sig, generics, _), .. }) => { (sig, generics, None) @@ -167,6 +168,12 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { owner_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, }; 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 mut is_downgradable = true; + + // Check if trait object is safe for suggesting dynamic dispatch. let is_object_safe = match self_ty.kind { hir::TyKind::TraitObject(objects, ..) => { objects.iter().all(|o| match o.trait_ref.path.res { @@ -189,8 +198,15 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } _ => 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 - && ty.hir_id == self_ty.hir_id + && ty.peel_refs().hir_id == self_ty.hir_id { let pre = if !is_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 \ single underlying type", ); + diag.multipart_suggestion_verbose(msg, impl_sugg, Applicability::MachineApplicable); + + // Suggest `Box<dyn Trait>` for return type if is_object_safe { - diag.multipart_suggestion_verbose( - "alternatively, you can return an owned trait object", + // If the return type is `&Trait`, we don't want + // 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![ (ty.span.shrink_to_lo(), "Box<dyn ".to_string()), (ty.span.shrink_to_hi(), ">".to_string()), - ], + ] + }; + + diag.multipart_suggestion_verbose( + "alternatively, you can return an owned trait object", + suggestion, Applicability::MachineApplicable, ); } else if is_downgradable { @@ -217,24 +245,24 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } return true; } + + // Suggestions for function parameters. 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; } let sugg = self.add_generic_param_suggestion(generics, self_ty.span, &trait_name); - if !sugg.is_empty() { - diag.multipart_suggestion_verbose( - format!("use a new generic type parameter, constrained by `{trait_name}`"), - sugg, - Applicability::MachineApplicable, - ); - diag.multipart_suggestion_verbose( - "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", - impl_sugg, - Applicability::MachineApplicable, - ); - } + diag.multipart_suggestion_verbose( + format!("use a new generic type parameter, constrained by `{trait_name}`"), + sugg, + Applicability::MachineApplicable, + ); + diag.multipart_suggestion_verbose( + "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", + impl_sugg, + Applicability::MachineApplicable, + ); if !is_object_safe { diag.note(format!("`{trait_name}` it is not object safe, so it can't be `dyn`")); if is_downgradable { @@ -242,14 +270,18 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { diag.downgrade_to_delayed_bug(); } } 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 { // There are more than one trait bound, we need surrounding parentheses. 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()), ] } 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( format!( diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs index 2cf808f962f..43bc925d09b 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs @@ -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 { fn next_type_param_name(&self, name: Option<&str>) -> String; } impl NextTypeParamName for &[hir::GenericParam<'_>] { 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.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 used_names = self + + // Filter out used names based on `filter_fn`. + let used_names: Vec<Symbol> = self .iter() - .filter_map(|p| match p.name { + .filter_map(|param| match param.name { hir::ParamName::Plain(ident) => Some(ident.name), _ => None, }) - .collect::<Vec<_>>(); + .collect(); + // Find a name from `possible_names` that is not in `used_names`. possible_names .iter() .find(|n| !used_names.contains(&Symbol::intern(n))) diff --git a/tests/ui/object-safety/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs b/tests/ui/object-safety/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs index dceb6e0fe71..dabaa309c16 100644 --- a/tests/ui/object-safety/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs +++ b/tests/ui/object-safety/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs @@ -43,7 +43,7 @@ impl IceCream { fn parrot() -> &mut Trait { //~^ ERROR: missing lifetime specifier - //~| ERROR: cannot return a mutable reference to a bare trait + //~| ERROR: trait objects must include the `dyn` keyword &mut Type //~^ ERROR: cannot return reference to temporary value } @@ -86,12 +86,12 @@ trait Sing { fn parrot() -> &mut Trait { //~^ ERROR: missing lifetime specifier - //~| ERROR: cannot return a mutable reference to a bare trait + //~| ERROR: trait objects must include the `dyn` keyword &mut Type //~^ ERROR: cannot return reference to temporary value } } - + fn foo(_: &Trait) {} //~^ ERROR: trait objects must include the `dyn` keyword @@ -134,7 +134,7 @@ fn puppy<'a>() -> &'a Trait { fn parrot() -> &mut Trait { //~^ ERROR: missing lifetime specifier - //~| ERROR: cannot return a mutable reference to a bare trait + //~| ERROR: trait objects must include the `dyn` keyword &mut Type //~^ ERROR: cannot return reference to temporary value } diff --git a/tests/ui/object-safety/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.stderr b/tests/ui/object-safety/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.stderr index 15c8eb5d16c..8bdfea7766e 100644 --- a/tests/ui/object-safety/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.stderr +++ b/tests/ui/object-safety/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.stderr @@ -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 | -LL | fn cat<'a>() -> &'a impl Trait; - | ++++ +++++++ +LL | fn cat() -> &impl Trait; + | ++++ help: alternatively, you can return an owned trait object | 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 | -LL | fn dog<'b, 'a>() -> &'b impl Trait { - | +++ +++++++ +LL | fn dog<'a>() -> &impl Trait { + | ++++ help: alternatively, you can return an owned trait object | 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> { | ~~~~~~~~~~~~~~ -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 | 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 | -LL | fn parrot() -> impl Trait { - | ~~~~~~~~~~ +LL | fn parrot() -> &mut impl Trait { + | ++++ help: alternatively, you can return an owned trait object | 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 | -LL | fn cat<'a>() -> &'a impl Trait { - | ++++ +++++++ +LL | fn cat() -> &impl Trait { + | ++++ help: alternatively, you can return an owned trait object | 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 | -LL | fn dog<'b, 'a>() -> &'b impl Trait { - | +++ +++++++ +LL | fn dog<'a>() -> &impl Trait { + | ++++ help: alternatively, you can return an owned trait object | 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> { | ~~~~~~~~~~~~~~ -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 | 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 | -LL | fn parrot() -> impl Trait { - | ~~~~~~~~~~ +LL | fn parrot() -> &mut impl Trait { + | ++++ help: alternatively, you can return an owned trait object | 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 | -LL | fn cat<'a>() -> &'a impl Trait { - | ++++ +++++++ +LL | fn cat() -> &impl Trait { + | ++++ help: alternatively, you can return an owned trait object | 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 | -LL | fn dog<'b, 'a>() -> &'b impl Trait { - | +++ +++++++ +LL | fn dog<'a>() -> &impl Trait { + | ++++ help: alternatively, you can return an owned trait object | 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> { | ~~~~~~~~~~~~~~ -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 | 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 | -LL | fn parrot() -> impl Trait { - | ~~~~~~~~~~ +LL | fn parrot() -> &mut impl Trait { + | ++++ help: alternatively, you can return an owned trait object | LL | fn parrot() -> Box<dyn Trait> { |