Skip to content

Keep spans for generics in #[derive(_)] desugaring #90519

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

Merged
merged 5 commits into from
Dec 4, 2021
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
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