Skip to content

Commit

Permalink
Rollup merge of rust-lang#90519 - estebank:issue-84003, r=petrochenkov
Browse files Browse the repository at this point in the history
Keep spans for generics in `#[derive(_)]` desugaring

Keep the spans for generics coming from a `derive`d Item, so that errors
and suggestions have better detail.

Fix rust-lang#84003.
  • Loading branch information
matthiaskrgr authored Dec 4, 2021
2 parents 9f8e822 + b0c3968 commit 0311cfa
Show file tree
Hide file tree
Showing 14 changed files with 409 additions and 135 deletions.
154 changes: 76 additions & 78 deletions compiler/rustc_builtin_macros/src/deriving/generic/mod.rs

Large diffs are not rendered by default.

16 changes: 6 additions & 10 deletions compiler/rustc_builtin_macros/src/deriving/generic/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,14 +211,6 @@ fn mk_ty_param(
cx.typaram(span, Ident::new(name, span), attrs.to_owned(), bounds, None)
}

fn mk_generics(params: Vec<ast::GenericParam>, span: Span) -> Generics {
Generics {
params,
where_clause: ast::WhereClause { has_where_token: false, predicates: Vec::new(), span },
span,
}
}

/// Bounds on type parameters.
#[derive(Clone)]
pub struct Bounds {
Expand All @@ -236,7 +228,7 @@ impl Bounds {
self_ty: Ident,
self_generics: &Generics,
) -> Generics {
let generic_params = self
let params = self
.bounds
.iter()
.map(|t| {
Expand All @@ -245,7 +237,11 @@ impl Bounds {
})
.collect();

mk_generics(generic_params, span)
Generics {
params,
where_clause: ast::WhereClause { has_where_token: false, predicates: Vec::new(), span },
span,
}
}
}

Expand Down
23 changes: 15 additions & 8 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ use rustc_index::vec::IndexVec;
use rustc_macros::HashStable_Generic;
use rustc_span::source_map::Spanned;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{def_id::LocalDefId, BytePos};
use rustc_span::{MultiSpan, Span, DUMMY_SP};
use rustc_span::{def_id::LocalDefId, BytePos, MultiSpan, Span, DUMMY_SP};
use rustc_target::asm::InlineAsmRegOrRegClass;
use rustc_target::spec::abi::Abi;

Expand Down Expand Up @@ -526,12 +525,20 @@ pub struct GenericParam<'hir> {
}

impl GenericParam<'hir> {
pub fn bounds_span(&self) -> Option<Span> {
self.bounds.iter().fold(None, |span, bound| {
let span = span.map(|s| s.to(bound.span())).unwrap_or_else(|| bound.span());

Some(span)
})
pub fn bounds_span_for_suggestions(&self) -> Option<Span> {
self.bounds
.iter()
.fold(None, |span: Option<Span>, 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.
if !bound.span().can_be_used_for_suggestions() {
None
} else {
let span = span.map(|s| s.to(bound.span())).unwrap_or_else(|| bound.span());
Some(span)
}
})
.map(|sp| sp.shrink_to_hi())
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ pub fn suggest_constraining_type_param(
// `where` clause instead of `trait Base<T: Copy = String>: Super<T>`.
&& !matches!(param.kind, hir::GenericParamKind::Type { default: Some(_), .. })
{
if let Some(bounds_span) = param.bounds_span() {
if let Some(span) = param.bounds_span_for_suggestions() {
// If user has provided some bounds, suggest restricting them:
//
// fn foo<T: Foo>(t: T) { ... }
Expand All @@ -284,7 +284,7 @@ pub fn suggest_constraining_type_param(
// --
// |
// replace with: `T: Bar +`
suggest_restrict(bounds_span.shrink_to_hi());
suggest_restrict(span);
} else {
// If user hasn't provided any bounds, suggest adding a new one:
//
Expand Down
20 changes: 7 additions & 13 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1735,7 +1735,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
(generics.span, format!("<{}>", ident))
};
// Do not suggest if this is coming from macro expansion.
if !span.from_expansion() {
if span.can_be_used_for_suggestions() {
return Some((
span.shrink_to_hi(),
msg,
Expand Down Expand Up @@ -1803,7 +1803,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
);
err.span_label(lifetime_ref.span, "undeclared lifetime");
let mut suggests_in_band = false;
let mut suggest_note = true;
let mut suggested_spans = vec![];
for missing in &self.missing_named_lifetime_spots {
match missing {
MissingLifetimeSpot::Generics(generics) => {
Expand All @@ -1821,23 +1821,17 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
suggests_in_band = true;
(generics.span, format!("<{}>", lifetime_ref))
};
if !span.from_expansion() {
if suggested_spans.contains(&span) {
continue;
}
suggested_spans.push(span);
if span.can_be_used_for_suggestions() {
err.span_suggestion(
span,
&format!("consider introducing lifetime `{}` here", lifetime_ref),
sugg,
Applicability::MaybeIncorrect,
);
} else if suggest_note {
suggest_note = false; // Avoid displaying the same help multiple times.
err.span_label(
span,
&format!(
"lifetime `{}` is missing in item created through this procedural \
macro",
lifetime_ref,
),
);
}
}
MissingLifetimeSpot::HigherRanked { span, span_type } => {
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_span/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,16 @@ impl Span {
matches!(self.ctxt().outer_expn_data().kind, ExpnKind::Macro(MacroKind::Derive, _))
}

/// Gate suggestions that would not be appropriate in a context the user didn't write.
pub fn can_be_used_for_suggestions(self) -> bool {
!self.from_expansion()
// FIXME: If this span comes from a `derive` macro but it points at code the user wrote,
// the callsite span and the span will be pointing at different places. It also means that
// we can safely provide suggestions on this span.
|| (matches!(self.ctxt().outer_expn_data().kind, ExpnKind::Macro(MacroKind::Derive, _))
&& self.parent_callsite().map(|p| (p.lo(), p.hi())) != Some((self.lo(), self.hi())))
}

#[inline]
pub fn with_root_ctxt(lo: BytePos, hi: BytePos) -> Span {
Span::new(lo, hi, SyntaxContext::root(), None)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,16 +262,16 @@ fn suggest_restriction(
match generics
.params
.iter()
.map(|p| p.bounds_span().unwrap_or(p.span))
.filter(|&span| generics.span.contains(span) && span.desugaring_kind().is_none())
.map(|p| p.bounds_span_for_suggestions().unwrap_or(p.span.shrink_to_hi()))
.filter(|&span| generics.span.contains(span) && span.can_be_used_for_suggestions())
.max_by_key(|span| span.hi())
{
// `fn foo(t: impl Trait)`
// ^ suggest `<T: Trait>` here
None => (generics.span, format!("<{}>", type_param)),
// `fn foo<A>(t: impl Trait)`
// ^^^ suggest `<A, T: Trait>` here
Some(span) => (span.shrink_to_hi(), format!(", {}", type_param)),
Some(span) => (span, format!(", {}", type_param)),
},
// `fn foo(t: impl Trait)`
// ^ suggest `where <T as Trait>::A: Bound`
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_typeck/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,9 @@ crate fn placeholder_type_error(
sugg.push((arg.span, (*type_name).to_string()));
} else {
let last = generics.iter().last().unwrap();
sugg.push((
// Account for bounds, we want `fn foo<T: E, K>(_: K)` not `fn foo<T, K: E>(_: K)`.
last.bounds_span().unwrap_or(last.span).shrink_to_hi(),
format!(", {}", type_name),
));
// Account for bounds, we want `fn foo<T: E, K>(_: K)` not `fn foo<T, K: E>(_: K)`.
let span = last.bounds_span_for_suggestions().unwrap_or(last.span.shrink_to_hi());
sugg.push((span, format!(", {}", type_name)));
}

let mut err = bad_placeholder_type(tcx, placeholder_types, kind);
Expand Down
4 changes: 4 additions & 0 deletions src/test/ui/issues/issue-38821.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ note: required because of the requirements on the impl of `IntoNullable` for `<C
LL | impl<T: NotNull> IntoNullable for T {
| ^^^^^^^^^^^^ ^
= note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider further restricting the associated type
|
LL | Expr: Expression<SqlType=<Col::SqlType as IntoNullable>::Nullable>, <Col as Expression>::SqlType: NotNull,
| +++++++++++++++++++++++++++++++++++++++

error: aborting due to previous error

Expand Down
11 changes: 10 additions & 1 deletion src/test/ui/issues/issue-50480.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
#[derive(Clone, Copy)]
//~^ ERROR the trait `Copy` may not be implemented for this type
struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
//~^ ERROR cannot find type `NotDefined` in this scope
//~| ERROR cannot find type `NotDefined` in this scope
//~| ERROR cannot find type `N` in this scope
//~| ERROR cannot find type `N` in this scope
//~| ERROR `i32` is not an iterator

#[derive(Clone, Copy)]
//~^ ERROR the trait `Copy` may not be implemented for this type
struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
//~^ ERROR cannot find type `NotDefined` in this scope
//~| ERROR cannot find type `N` in this scope
//~| ERROR `i32` is not an iterator

fn main() {}
89 changes: 76 additions & 13 deletions src/test/ui/issues/issue-50480.stderr
Original file line number Diff line number Diff line change
@@ -1,20 +1,61 @@
error[E0412]: cannot find type `NotDefined` in this scope
error[E0412]: cannot find type `N` in this scope
--> $DIR/issue-50480.rs:3:12
|
LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| ^^^^^^^^^^ not found in this scope
LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| -^ not found in this scope
| |
| help: you might be missing a type parameter: `<N>`

error[E0412]: cannot find type `NotDefined` in this scope
--> $DIR/issue-50480.rs:3:15
|
LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| ^^^^^^^^^^ not found in this scope

error[E0412]: cannot find type `N` in this scope
--> $DIR/issue-50480.rs:3:12
|
LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| ^^^^^^^^^^ not found in this scope
LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| -^ not found in this scope
| |
| help: you might be missing a type parameter: `<N>`

error[E0412]: cannot find type `NotDefined` in this scope
--> $DIR/issue-50480.rs:3:15
|
LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| - ^^^^^^^^^^ not found in this scope
| |
| help: you might be missing a type parameter: `<NotDefined>`

error[E0412]: cannot find type `N` in this scope
--> $DIR/issue-50480.rs:12:18
|
LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| - ^
| |
| similarly named type parameter `T` defined here
|
help: a type parameter with a similar name exists
|
LL | struct Bar<T>(T, T, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| ~
help: you might be missing a type parameter
|
LL | struct Bar<T, N>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| +++

error[E0412]: cannot find type `NotDefined` in this scope
--> $DIR/issue-50480.rs:12:21
|
LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| ^^^^^^^^^^ not found in this scope

error[E0277]: `i32` is not an iterator
--> $DIR/issue-50480.rs:3:24
--> $DIR/issue-50480.rs:3:27
|
LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| ^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator
LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| ^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator
|
= help: the trait `Iterator` is not implemented for `i32`
= note: if you want to iterate between `start` until a value `end`, use the exclusive range syntax `start..end` or the inclusive range syntax `start..=end`
Expand All @@ -25,14 +66,36 @@ error[E0204]: the trait `Copy` may not be implemented for this type
LL | #[derive(Clone, Copy)]
| ^^^^
LL |
LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| -------- ------ this field does not implement `Copy`
| |
| this field does not implement `Copy`
LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| -------- ------ this field does not implement `Copy`
| |
| this field does not implement `Copy`
|
= note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `i32` is not an iterator
--> $DIR/issue-50480.rs:12:33
|
LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| ^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator
|
= help: the trait `Iterator` is not implemented for `i32`
= note: if you want to iterate between `start` until a value `end`, use the exclusive range syntax `start..end` or the inclusive range syntax `start..=end`

error[E0204]: the trait `Copy` may not be implemented for this type
--> $DIR/issue-50480.rs:10:17
|
LL | #[derive(Clone, Copy)]
| ^^^^
LL |
LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| -------- ------ this field does not implement `Copy`
| |
| this field does not implement `Copy`
|
= note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 4 previous errors
error: aborting due to 10 previous errors

Some errors have detailed explanations: E0204, E0277, E0412.
For more information about an error, try `rustc --explain E0204`.
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ LL | a: &'b str,
error[E0261]: use of undeclared lifetime name `'b`
--> $DIR/undeclared-lifetime-used-in-debug-macro-issue-70152.rs:3:9
|
LL | #[derive(Eq, PartialEq)]
| -- lifetime `'b` is missing in item created through this procedural macro
LL | struct Test {
| - help: consider introducing lifetime `'b` here: `<'b>`
LL | a: &'b str,
| ^^ undeclared lifetime
|
Expand Down
Loading

0 comments on commit 0311cfa

Please sign in to comment.