Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrap dyn type with parentheses in suggestion #120929

Merged
merged 1 commit into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 41 additions & 5 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,13 +644,49 @@ impl<'hir> Generics<'hir> {
})
}

pub fn bounds_span_for_suggestions(&self, param_def_id: LocalDefId) -> Option<Span> {
/// Returns a suggestable empty span right after the "final" bound of the generic parameter.
///
/// If that bound needs to be wrapped in parentheses to avoid ambiguity with
/// subsequent bounds, it also returns an empty span for an open parenthesis
/// as the second component.
///
/// E.g., adding `+ 'static` after `Fn() -> dyn Future<Output = ()>` or
/// `Fn() -> &'static dyn Debug` requires parentheses:
/// `Fn() -> (dyn Future<Output = ()>) + 'static` and
/// `Fn() -> &'static (dyn Debug) + 'static`, respectively.
pub fn bounds_span_for_suggestions(
&self,
param_def_id: LocalDefId,
) -> Option<(Span, Option<Span>)> {
self.bounds_for_param(param_def_id).flat_map(|bp| bp.bounds.iter().rev()).find_map(
|bound| {
// We include bounds that come from a `#[derive(_)]` but point at the user's code,
// as we use this method to get a span appropriate for suggestions.
let bs = bound.span();
bs.can_be_used_for_suggestions().then(|| bs.shrink_to_hi())
let span_for_parentheses = if let Some(trait_ref) = bound.trait_ref()
&& let [.., segment] = trait_ref.path.segments
&& segment.args().parenthesized == GenericArgsParentheses::ParenSugar
&& let [binding] = segment.args().bindings
&& let TypeBindingKind::Equality { term: Term::Ty(ret_ty) } = binding.kind
long-long-float marked this conversation as resolved.
Show resolved Hide resolved
&& let ret_ty = ret_ty.peel_refs()
&& let TyKind::TraitObject(
_,
_,
TraitObjectSyntax::Dyn | TraitObjectSyntax::DynStar,
) = ret_ty.kind
&& ret_ty.span.can_be_used_for_suggestions()
{
Some(ret_ty.span)
} else {
None
};

span_for_parentheses.map_or_else(
|| {
// We include bounds that come from a `#[derive(_)]` but point at the user's code,
// as we use this method to get a span appropriate for suggestions.
let bs = bound.span();
bs.can_be_used_for_suggestions().then(|| (bs.shrink_to_hi(), None))
},
|span| Some((span.shrink_to_hi(), Some(span.shrink_to_lo()))),
)
},
)
}
Expand Down
65 changes: 41 additions & 24 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
long-long-float marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -3291,14 +3291,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
param.name.ident(),
));
let bounds_span = hir_generics.bounds_span_for_suggestions(def_id);
if rcvr_ty.is_ref() && param.is_impl_trait() && bounds_span.is_some() {
if rcvr_ty.is_ref()
&& param.is_impl_trait()
&& let Some((bounds_span, _)) = bounds_span
{
err.multipart_suggestions(
msg,
candidates.iter().map(|t| {
vec![
(param.span.shrink_to_lo(), "(".to_string()),
(
bounds_span.unwrap(),
bounds_span,
long-long-float marked this conversation as resolved.
Show resolved Hide resolved
format!(" + {})", self.tcx.def_path_str(t.def_id)),
),
]
Expand All @@ -3308,32 +3311,46 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return;
}

let (sp, introducer) = if let Some(span) = bounds_span {
(span, Introducer::Plus)
} else if let Some(colon_span) = param.colon_span {
(colon_span.shrink_to_hi(), Introducer::Nothing)
} else if param.is_impl_trait() {
(param.span.shrink_to_hi(), Introducer::Plus)
} else {
(param.span.shrink_to_hi(), Introducer::Colon)
};
let (sp, introducer, open_paren_sp) =
if let Some((span, open_paren_sp)) = bounds_span {
(span, Introducer::Plus, open_paren_sp)
} else if let Some(colon_span) = param.colon_span {
(colon_span.shrink_to_hi(), Introducer::Nothing, None)
} else if param.is_impl_trait() {
(param.span.shrink_to_hi(), Introducer::Plus, None)
} else {
(param.span.shrink_to_hi(), Introducer::Colon, None)
};

err.span_suggestions(
sp,
let all_suggs = candidates.iter().map(|cand| {
let suggestion = format!(
"{} {}",
match introducer {
Introducer::Plus => " +",
Introducer::Colon => ":",
Introducer::Nothing => "",
},
self.tcx.def_path_str(cand.def_id)
);

let mut suggs = vec![];

if let Some(open_paren_sp) = open_paren_sp {
suggs.push((open_paren_sp, "(".to_string()));
suggs.push((sp, format!("){suggestion}")));
} else {
suggs.push((sp, suggestion));
}

suggs
});

err.multipart_suggestions(
msg,
candidates.iter().map(|t| {
format!(
"{} {}",
match introducer {
Introducer::Plus => " +",
Introducer::Colon => ":",
Introducer::Nothing => "",
},
self.tcx.def_path_str(t.def_id)
)
}),
all_suggs,
Applicability::MaybeIncorrect,
);

return;
}
Node::Item(hir::Item {
Expand Down
16 changes: 11 additions & 5 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2369,7 +2369,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
generic_param_scope = self.tcx.local_parent(generic_param_scope);
}

// type_param_sugg_span is (span, has_bounds)
// type_param_sugg_span is (span, has_bounds, needs_parentheses)
let (type_scope, type_param_sugg_span) = match bound_kind {
GenericKind::Param(ref param) => {
let generics = self.tcx.generics_of(generic_param_scope);
Expand All @@ -2380,10 +2380,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
// instead we suggest `T: 'a + 'b` in that case.
let hir_generics = self.tcx.hir().get_generics(scope).unwrap();
let sugg_span = match hir_generics.bounds_span_for_suggestions(def_id) {
Some(span) => Some((span, true)),
Some((span, open_paren_sp)) => Some((span, true, open_paren_sp)),
// If `param` corresponds to `Self`, no usable suggestion span.
None if generics.has_self && param.index == 0 => None,
None => Some((self.tcx.def_span(def_id).shrink_to_hi(), false)),
None => Some((self.tcx.def_span(def_id).shrink_to_hi(), false, None)),
};
(scope, sugg_span)
}
Expand All @@ -2406,12 +2406,18 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
let mut suggs = vec![];
let lt_name = self.suggest_name_region(sub, &mut suggs);

if let Some((sp, has_lifetimes)) = type_param_sugg_span
if let Some((sp, has_lifetimes, open_paren_sp)) = type_param_sugg_span
&& suggestion_scope == type_scope
{
let suggestion =
if has_lifetimes { format!(" + {lt_name}") } else { format!(": {lt_name}") };
suggs.push((sp, suggestion))

if let Some(open_paren_sp) = open_paren_sp {
suggs.push((open_paren_sp, "(".to_string()));
suggs.push((sp, format!("){suggestion}")));
} else {
suggs.push((sp, suggestion))
}
} else if let GenericKind::Alias(ref p) = bound_kind
&& let ty::Projection = p.kind(self.tcx)
&& let DefKind::AssocTy = self.tcx.def_kind(p.def_id)
Expand Down
39 changes: 22 additions & 17 deletions compiler/rustc_middle/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,24 +279,29 @@ pub fn suggest_constraining_type_params<'a>(
constraint.sort();
constraint.dedup();
let constraint = constraint.join(" + ");
let mut suggest_restrict = |span, bound_list_non_empty| {
suggestions.push((
span,
if span_to_replace.is_some() {
constraint.clone()
} else if constraint.starts_with('<') {
constraint.to_string()
} else if bound_list_non_empty {
format!(" + {constraint}")
} else {
format!(" {constraint}")
},
SuggestChangingConstraintsMessage::RestrictBoundFurther,
))
let mut suggest_restrict = |span, bound_list_non_empty, open_paren_sp| {
let suggestion = if span_to_replace.is_some() {
constraint.clone()
} else if constraint.starts_with('<') {
constraint.to_string()
} else if bound_list_non_empty {
format!(" + {constraint}")
} else {
format!(" {constraint}")
};

use SuggestChangingConstraintsMessage::RestrictBoundFurther;

if let Some(open_paren_sp) = open_paren_sp {
suggestions.push((open_paren_sp, "(".to_string(), RestrictBoundFurther));
suggestions.push((span, format!("){suggestion}"), RestrictBoundFurther));
} else {
suggestions.push((span, suggestion, RestrictBoundFurther));
}
};

if let Some(span) = span_to_replace {
suggest_restrict(span, true);
suggest_restrict(span, true, None);
continue;
}

Expand Down Expand Up @@ -327,8 +332,8 @@ pub fn suggest_constraining_type_params<'a>(
// --
// |
// replace with: `T: Bar +`
if let Some(span) = generics.bounds_span_for_suggestions(param.def_id) {
suggest_restrict(span, true);
if let Some((span, open_paren_sp)) = generics.bounds_span_for_suggestions(param.def_id) {
suggest_restrict(span, true, open_paren_sp);
continue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2938,17 +2938,28 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
}
_ => {}
};

// Didn't add an indirection suggestion, so add a general suggestion to relax `Sized`.
let (span, separator) = if let Some(s) = generics.bounds_span_for_suggestions(param.def_id)
{
(s, " +")
let (span, separator, open_paren_sp) =
if let Some((s, open_paren_sp)) = generics.bounds_span_for_suggestions(param.def_id) {
(s, " +", open_paren_sp)
} else {
(param.name.ident().span.shrink_to_hi(), ":", None)
};

let mut suggs = vec![];
let suggestion = format!("{separator} ?Sized");

if let Some(open_paren_sp) = open_paren_sp {
suggs.push((open_paren_sp, "(".to_string()));
suggs.push((span, format!("){suggestion}")));
} else {
(param.name.ident().span.shrink_to_hi(), ":")
};
err.span_suggestion_verbose(
span,
suggs.push((span, suggestion));
}

err.multipart_suggestion_verbose(
"consider relaxing the implicit `Sized` restriction",
format!("{separator} ?Sized"),
suggs,
Applicability::MachineApplicable,
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,18 @@ impl Foo for S {}
impl Bar for S {}

fn test(foo: impl Foo + Bar) {
foo.hello(); //~ ERROR E0599
foo.hello(); //~ ERROR no method named `hello` found
}

trait Trait {
fn method(&self) {}
}

impl Trait for fn() {}

#[allow(dead_code)]
fn test2(f: impl Fn() -> (dyn std::fmt::Debug) + Trait) {
f.method(); //~ ERROR no method named `method` found
}

fn main() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,18 @@ impl Foo for S {}
impl Bar for S {}

fn test(foo: impl Foo) {
foo.hello(); //~ ERROR E0599
foo.hello(); //~ ERROR no method named `hello` found
}

trait Trait {
fn method(&self) {}
}

impl Trait for fn() {}

#[allow(dead_code)]
fn test2(f: impl Fn() -> dyn std::fmt::Debug) {
f.method(); //~ ERROR no method named `method` found
}

fn main() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,20 @@ help: the following trait defines an item `hello`, perhaps you need to restrict
LL | fn test(foo: impl Foo + Bar) {
| +++++

error: aborting due to 1 previous error
error[E0599]: no method named `method` found for type parameter `impl Fn() -> dyn std::fmt::Debug` in the current scope
--> $DIR/impl-trait-with-missing-trait-bounds-in-arg.rs:26:7
|
LL | fn test2(f: impl Fn() -> dyn std::fmt::Debug) {
| -------------------------------- method `method` not found for this type parameter
LL | f.method();
| ^^^^^^ method not found in `impl Fn() -> dyn std::fmt::Debug`
|
= help: items from traits can only be used if the type parameter is bounded by the trait
help: the following trait defines an item `method`, perhaps you need to restrict type parameter `impl Fn() -> dyn std::fmt::Debug` with it:
|
LL | fn test2(f: impl Fn() -> (dyn std::fmt::Debug) + Trait) {
| + +++++++++

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0599`.
35 changes: 35 additions & 0 deletions tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#![feature(dyn_star)] //~ WARNING the feature `dyn_star` is incomplete

use std::future::Future;

pub fn dyn_func<T>(
executor: impl FnOnce(T) -> dyn Future<Output = ()>,
) -> Box<dyn FnOnce(T) -> dyn Future<Output = ()>> {
Box::new(executor) //~ ERROR may not live long enough
}

pub fn dyn_star_func<T>(
executor: impl FnOnce(T) -> dyn* Future<Output = ()>,
) -> Box<dyn FnOnce(T) -> dyn* Future<Output = ()>> {
Box::new(executor) //~ ERROR may not live long enough
}

trait Trait {
fn method(&self) {}
}

impl Trait for fn() {}

pub fn in_ty_param<T: Fn() -> dyn std::fmt::Debug> (t: T) {
t.method();
//~^ ERROR no method named `method` found for type parameter `T`
}

fn with_sized<T: Fn() -> &'static (dyn std::fmt::Debug) + ?Sized>() {
without_sized::<T>();
//~^ ERROR the size for values of type `T` cannot be known at compilation time
}

fn without_sized<T: Fn() -> &'static dyn std::fmt::Debug>() {}

fn main() {}
Loading
Loading