Rollup merge of #120929 - long-long-float:wrap-dyn-in-suggestion, r=fmease

Wrap dyn type with parentheses in suggestion

Close #120223

Fix wrong suggestion that is grammatically incorrect.
Specifically, I added parentheses to dyn types that need lifetime bound.

```
help: consider adding an explicit lifetime bound
  |
4 |     executor: impl FnOnce(T) -> (dyn Future<Output = ()>) + 'static,
  |                                 +                       +++++++++++
```
This commit is contained in:
León Orell Valerian Liehr 2024-04-23 17:25:14 +02:00 committed by GitHub
commit 80f2b91b20
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 286 additions and 62 deletions

View File

@ -644,13 +644,49 @@ pub fn outlives_for_param(
})
}
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| {
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
&& 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())
bs.can_be_used_for_suggestions().then(|| (bs.shrink_to_hi(), None))
},
|span| Some((span.shrink_to_hi(), Some(span.shrink_to_lo()))),
)
},
)
}

View File

@ -3291,14 +3291,17 @@ enum Introducer {
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,
format!(" + {})", self.tcx.def_path_str(t.def_id)),
),
]
@ -3308,32 +3311,46 @@ enum Introducer {
return;
}
let (sp, introducer) = if let Some(span) = bounds_span {
(span, Introducer::Plus)
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)
(colon_span.shrink_to_hi(), Introducer::Nothing, None)
} else if param.is_impl_trait() {
(param.span.shrink_to_hi(), Introducer::Plus)
(param.span.shrink_to_hi(), Introducer::Plus, None)
} else {
(param.span.shrink_to_hi(), Introducer::Colon)
(param.span.shrink_to_hi(), Introducer::Colon, None)
};
err.span_suggestions(
sp,
msg,
candidates.iter().map(|t| {
format!(
let all_suggs = candidates.iter().map(|cand| {
let suggestion = format!(
"{} {}",
match introducer {
Introducer::Plus => " +",
Introducer::Colon => ":",
Introducer::Nothing => "",
},
self.tcx.def_path_str(t.def_id)
)
}),
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,
all_suggs,
Applicability::MaybeIncorrect,
);
return;
}
Node::Item(hir::Item {

View File

@ -2369,7 +2369,7 @@ pub fn construct_generic_bound_failure(
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(param) => {
let generics = self.tcx.generics_of(generic_param_scope);
@ -2380,10 +2380,10 @@ pub fn construct_generic_bound_failure(
// 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)
}
@ -2406,12 +2406,18 @@ pub fn construct_generic_bound_failure(
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}") };
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)

View File

@ -279,10 +279,8 @@ 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() {
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()
@ -290,13 +288,20 @@ pub fn suggest_constraining_type_params<'a>(
format!(" + {constraint}")
} else {
format!(" {constraint}")
},
SuggestChangingConstraintsMessage::RestrictBoundFurther,
))
};
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;
}
@ -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;
}

View File

@ -2938,17 +2938,28 @@ fn maybe_suggest_unsized_generics(&self, err: &mut Diag<'_>, span: Span, node: N
}
_ => {}
};
// 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(), ":")
(param.name.ident().span.shrink_to_hi(), ":", None)
};
err.span_suggestion_verbose(
span,
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 {
suggs.push((span, suggestion));
}
err.multipart_suggestion_verbose(
"consider relaxing the implicit `Sized` restriction",
format!("{separator} ?Sized"),
suggs,
Applicability::MachineApplicable,
);
}

View File

@ -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() {

View File

@ -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() {

View File

@ -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`.

View File

@ -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() {}

View File

@ -0,0 +1,78 @@
warning: the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:1:12
|
LL | #![feature(dyn_star)]
| ^^^^^^^^
|
= note: see issue #102425 <https://github.com/rust-lang/rust/issues/102425> for more information
= note: `#[warn(incomplete_features)]` on by default
error[E0599]: no method named `method` found for type parameter `T` in the current scope
--> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:24:7
|
LL | pub fn in_ty_param<T: Fn() -> dyn std::fmt::Debug> (t: T) {
| - method `method` not found for this type parameter
LL | t.method();
| ^^^^^^ method not found in `T`
|
= 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 `T` with it:
|
LL | pub fn in_ty_param<T: Fn() -> (dyn std::fmt::Debug) + Trait> (t: T) {
| + +++++++++
error[E0277]: the size for values of type `T` cannot be known at compilation time
--> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:29:21
|
LL | fn with_sized<T: Fn() -> &'static (dyn std::fmt::Debug) + ?Sized>() {
| - this type parameter needs to be `Sized`
LL | without_sized::<T>();
| ^ doesn't have a size known at compile-time
|
note: required by an implicit `Sized` bound in `without_sized`
--> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:33:18
|
LL | fn without_sized<T: Fn() -> &'static dyn std::fmt::Debug>() {}
| ^ required by the implicit `Sized` requirement on this type parameter in `without_sized`
help: consider removing the `?Sized` bound to make the type parameter `Sized`
|
LL - fn with_sized<T: Fn() -> &'static (dyn std::fmt::Debug) + ?Sized>() {
LL + fn with_sized<T: Fn() -> &'static (dyn std::fmt::Debug)>() {
|
help: consider relaxing the implicit `Sized` restriction
|
LL | fn without_sized<T: Fn() -> &'static (dyn std::fmt::Debug) + ?Sized>() {}
| + ++++++++++
error[E0310]: the parameter type `impl FnOnce(T) -> dyn Future<Output = ()>` may not live long enough
--> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:8:5
|
LL | Box::new(executor)
| ^^^^^^^^^^^^^^^^^^
| |
| the parameter type `impl FnOnce(T) -> dyn Future<Output = ()>` must be valid for the static lifetime...
| ...so that the type `impl FnOnce(T) -> dyn Future<Output = ()>` will meet its required lifetime bounds
|
help: consider adding an explicit lifetime bound
|
LL | executor: impl FnOnce(T) -> (dyn Future<Output = ()>) + 'static,
| + +++++++++++
error[E0310]: the parameter type `impl FnOnce(T) -> Future<Output = ()>` may not live long enough
--> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:14:5
|
LL | Box::new(executor)
| ^^^^^^^^^^^^^^^^^^
| |
| the parameter type `impl FnOnce(T) -> Future<Output = ()>` must be valid for the static lifetime...
| ...so that the type `impl FnOnce(T) -> Future<Output = ()>` will meet its required lifetime bounds
|
help: consider adding an explicit lifetime bound
|
LL | executor: impl FnOnce(T) -> (dyn* Future<Output = ()>) + 'static,
| + +++++++++++
error: aborting due to 4 previous errors; 1 warning emitted
Some errors have detailed explanations: E0277, E0310, E0599.
For more information about an error, try `rustc --explain E0277`.