Skip to content

Commit

Permalink
Rollup merge of rust-lang#95970 - WaffleLapkin:nicer_trait_suggestion…
Browse files Browse the repository at this point in the history
…s, r=compiler-errors

Fix suggestions in case of `T:` bounds

This PR fixes a corner case in `suggest_constraining_type_params` that was causing incorrect suggestions.

For the following functions:
```rust
fn a<T:>(t: T) { [t, t]; }
fn b<T>(t: T) where T: { [t, t]; }
```

We previously suggested the following:
```text
...
help: consider restricting type parameter `T`
  |
1 | fn a<T: Copy:>(t: T) { [t, t]; }
  |       ++++++
...
help: consider further restricting this bound
  |
2 | fn b<T>(t: T) where T: + Copy { [t, t]; }
  |                        ++++++
```

Note that neither `T: Copy:` not `where T: + Copy` is a correct bound.

With this commit the suggestions are correct:
```text
...
help: consider restricting type parameter `T`
  |
1 | fn a<T: Copy>(t: T) { [t, t]; }
  |         ++++
...
help: consider further restricting this bound
  |
2 | fn b<T>(t: T) where T: Copy { [t, t]; }
  |                        ++++
```

r? `@compiler-errors`

I've tried fixing rust-lang#95898 here too, but got too confused with how `suggest_traits_to_import` works and what it does 😅
  • Loading branch information
Dylan-DPC authored Apr 12, 2022
2 parents 91813a7 + e4710fe commit d63a46a
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 9 deletions.
36 changes: 35 additions & 1 deletion compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use rustc_error_messages::MultiSpan;
use rustc_index::vec::IndexVec;
use rustc_macros::HashStable_Generic;
use rustc_span::hygiene::MacroKind;
use rustc_span::source_map::Spanned;
use rustc_span::source_map::{SourceMap, Spanned};
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{def_id::LocalDefId, BytePos, Span, DUMMY_SP};
use rustc_target::asm::InlineAsmRegOrRegClass;
Expand Down Expand Up @@ -523,6 +523,40 @@ impl<'hir> GenericParam<'hir> {
})
.map(|sp| sp.shrink_to_hi())
}

/// Returns the span of `:` after a generic parameter.
///
/// For example:
///
/// ```text
/// fn a<T:>()
/// ^
/// | here
/// here |
/// v
/// fn b<T :>()
///
/// fn c<T
///
/// :>()
/// ^
/// |
/// here
/// ```
pub fn colon_span_for_suggestions(&self, source_map: &SourceMap) -> Option<Span> {
let sp = source_map
.span_extend_while(self.span.shrink_to_hi(), |c| c.is_whitespace() || c == ':')
.ok()?;

let snippet = source_map.span_to_snippet(sp).ok()?;
let offset = snippet.find(':')?;

let colon_sp = sp
.with_lo(BytePos(sp.lo().0 + offset as u32))
.with_hi(BytePos(sp.lo().0 + (offset + ':'.len_utf8()) as u32));

Some(colon_sp)
}
}

#[derive(Default)]
Expand Down
35 changes: 28 additions & 7 deletions compiler/rustc_middle/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,14 @@ pub fn suggest_constraining_type_params<'a>(
}

let constraint = constraints.iter().map(|&(c, _)| c).collect::<Vec<_>>().join(" + ");
let mut suggest_restrict = |span| {
let mut suggest_restrict = |span, bound_list_non_empty| {
suggestions.push((
span,
format!(" + {}", constraint),
if bound_list_non_empty {
format!(" + {}", constraint)
} else {
format!(" {}", constraint)
},
SuggestChangingConstraintsMessage::RestrictBoundFurther,
))
};
Expand All @@ -360,7 +364,10 @@ pub fn suggest_constraining_type_params<'a>(
// |
// replace with: `impl Foo + Bar`

suggest_restrict(param.span.shrink_to_hi());
// `impl Trait` must have at least one trait in the list
let bound_list_non_empty = true;

suggest_restrict(param.span.shrink_to_hi(), bound_list_non_empty);
continue;
}

Expand All @@ -383,15 +390,25 @@ pub fn suggest_constraining_type_params<'a>(
// --
// |
// replace with: `T: Bar +`
suggest_restrict(span);

// `bounds_span_for_suggestions` returns `None` if the list is empty
let bound_list_non_empty = true;

suggest_restrict(span, bound_list_non_empty);
} else {
let (colon, span) = match param.colon_span_for_suggestions(tcx.sess.source_map()) {
// If there is already a colon after generic, do not suggest adding it again
Some(sp) => ("", sp.shrink_to_hi()),
None => (":", param.span.shrink_to_hi()),
};

// If user hasn't provided any bounds, suggest adding a new one:
//
// fn foo<T>(t: T) { ... }
// - help: consider restricting this type parameter with `T: Foo`
suggestions.push((
param.span.shrink_to_hi(),
format!(": {}", constraint),
span,
format!("{colon} {constraint}"),
SuggestChangingConstraintsMessage::RestrictType { ty: param_name },
));
}
Expand Down Expand Up @@ -459,17 +476,21 @@ pub fn suggest_constraining_type_params<'a>(
));
} else {
let mut param_spans = Vec::new();
let mut non_empty = false;

for predicate in generics.where_clause.predicates {
if let WherePredicate::BoundPredicate(WhereBoundPredicate {
span,
bounded_ty,
bounds,
..
}) = predicate
{
if let TyKind::Path(QPath::Resolved(_, path)) = &bounded_ty.kind {
if let Some(segment) = path.segments.first() {
if segment.ident.to_string() == param_name {
non_empty = !bounds.is_empty();

param_spans.push(span);
}
}
Expand All @@ -478,7 +499,7 @@ pub fn suggest_constraining_type_params<'a>(
}

match param_spans[..] {
[&param_span] => suggest_restrict(param_span.shrink_to_hi()),
[&param_span] => suggest_restrict(param_span.shrink_to_hi(), non_empty),
_ => {
suggestions.push((
generics.where_clause.tail_span_for_suggestion(),
Expand Down
14 changes: 14 additions & 0 deletions src/test/ui/moves/use_of_moved_value_copy_suggestions.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,18 @@ where
(t, t) //~ use of moved value: `t`
}

#[rustfmt::skip]
fn existing_colon<T: Copy>(t: T) {
//~^ HELP consider restricting type parameter `T`
[t, t]; //~ use of moved value: `t`
}

fn existing_colon_in_where<T>(t: T)
where
T: Copy,
//~^ HELP consider further restricting this bound
{
[t, t]; //~ use of moved value: `t`
}

fn main() {}
14 changes: 14 additions & 0 deletions src/test/ui/moves/use_of_moved_value_copy_suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,18 @@ where
(t, t) //~ use of moved value: `t`
}

#[rustfmt::skip]
fn existing_colon<T:>(t: T) {
//~^ HELP consider restricting type parameter `T`
[t, t]; //~ use of moved value: `t`
}

fn existing_colon_in_where<T>(t: T)
where
T:,
//~^ HELP consider further restricting this bound
{
[t, t]; //~ use of moved value: `t`
}

fn main() {}
34 changes: 33 additions & 1 deletion src/test/ui/moves/use_of_moved_value_copy_suggestions.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,38 @@ help: consider further restricting this bound
LL | T: B + Trait + Copy,
| ++++++++++++++

error: aborting due to 9 previous errors
error[E0382]: use of moved value: `t`
--> $DIR/use_of_moved_value_copy_suggestions.rs:83:9
|
LL | fn existing_colon_in_where<T>(t: T)
| - move occurs because `t` has type `T`, which does not implement the `Copy` trait
...
LL | [t, t];
| - ^ value used here after move
| |
| value moved here
|
help: consider further restricting this bound
|
LL | T: Copy,
| ++++

error[E0382]: use of moved value: `t`
--> $DIR/use_of_moved_value_copy_suggestions.rs:75:9
|
LL | fn existing_colon<T:>(t: T) {
| - move occurs because `t` has type `T`, which does not implement the `Copy` trait
LL |
LL | [t, t];
| - ^ value used here after move
| |
| value moved here
|
help: consider restricting type parameter `T`
|
LL | fn existing_colon<T: Copy>(t: T) {
| ++++

error: aborting due to 11 previous errors

For more information about this error, try `rustc --explain E0382`.

0 comments on commit d63a46a

Please sign in to comment.