Skip to content

Commit

Permalink
Unrolled build for rust-lang#120929
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#120929 - long-long-float:wrap-dyn-in-suggestion, r=fmease

Wrap dyn type with parentheses in suggestion

Close rust-lang#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,
  |                                 +                       +++++++++++
```
  • Loading branch information
rust-timer committed Apr 23, 2024
2 parents cd90d5c + 31e581e commit 888a0ad
Show file tree
Hide file tree
Showing 10 changed files with 286 additions and 62 deletions.
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
&& 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
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,
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(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

0 comments on commit 888a0ad

Please sign in to comment.