Skip to content
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.

Sign up for GitHub

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

incorrect visitor use #91441

Closed
wants to merge 2 commits into from
Closed

incorrect visitor use #91441

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b285e1e
incorrect visitor use
estebank Dec 1, 2021
5d60c57
review comments
estebank Dec 2, 2021
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
64 changes: 54 additions & 10 deletions compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ use std::cell::RefCell;
use std::iter;
use std::vec;

use rustc_ast::mut_visit::MutVisitor;
use rustc_ast::ptr::P;
use rustc_ast::{self as ast, BinOpKind, EnumDef, Expr, Generics, PatKind};
use rustc_ast::{GenericArg, GenericParamKind, VariantData};
Expand Down Expand Up @@ -720,22 +721,32 @@ impl<'a> TraitDef<'a> {
let mut a = vec![attr, unused_qual];
a.extend(self.attributes.iter().cloned());

// for attr in a.iter_mut() {
// SpanMarker { container_id: cx.current_expansion.id, span: self.span }.visit_attribute(attr);
// }

let unsafety = if self.is_unsafe { ast::Unsafe::Yes(self.span) } else { ast::Unsafe::No };

// let mut trait_generics = trait_generics;
// SpanMarker { container_id: cx.current_expansion.id, span: self.span }
// .visit_generics(&mut trait_generics);
let mut item = ast::ItemKind::Impl(Box::new(ast::Impl {
unsafety,
polarity: ast::ImplPolarity::Positive,
defaultness: ast::Defaultness::Final,
constness: ast::Const::No,
generics: trait_generics,
of_trait: opt_trait_ref,
self_ty: self_type,
items: methods.into_iter().chain(associated_types).collect(),
}));
SpanMarker { container_id: cx.current_expansion.id, span: self.span }
.visit_item_kind(&mut item);
cx.item(
self.span,
Ident::empty(),
a,
ast::ItemKind::Impl(Box::new(ast::Impl {
unsafety,
polarity: ast::ImplPolarity::Positive,
defaultness: ast::Defaultness::Final,
constness: ast::Const::No,
generics: trait_generics,
of_trait: opt_trait_ref,
self_ty: self_type,
items: methods.into_iter().chain(associated_types).collect(),
})),
item,
)
}

Expand Down Expand Up @@ -1785,3 +1796,36 @@ pub fn is_type_without_fields(item: &Annotatable) -> bool {
false
}
}

/// A folder used to correctly mark every span coming from a `derive` macro.
struct SpanMarker {
container_id: rustc_span::hygiene::LocalExpnId,
span: Span,
}

// let mut marker = Marker(cx.current_expansion.id, transparency);
impl MutVisitor for SpanMarker {
fn visit_span(&mut self, sp: &mut Span) {
// let x = sp.ctxt();
// let y = self.container_id.to_expn_id();
// tracing::info!(?sp, ?x, ?self.container_id, ?y);
let x = self.span.ctxt().outer_expn();
let y = sp.ctxt().outer_expn();
tracing::info!(?self.span, ?sp, ?self.container_id, ?x, ?y);
// if self.span != *sp {
// *sp = sp.apply_mark(self.container_id.to_expn_id(), rustc_span::hygiene::Transparency::SemiTransparent);
// // *sp = sp.with_ctxt(self.span.ctxt());
// } else {
// *sp = sp.with_ctxt(self.span.ctxt());
// }
if self.container_id.to_expn_id() != sp.ctxt().outer_expn() { // && sp.ctxt().outer_expn() == rustc_span::hygiene::ExpnId::root() {
// println!("{:?} {:?} {:?}", self.ctxt, sp.ctxt(), sp);
// tracing::info!("marked");
// *sp.with_ctxt();
*sp = sp.apply_mark(self.container_id.to_expn_id(), rustc_span::hygiene::Transparency::SemiTransparent);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not what I was talking about in #90519, all the marks are already added in the TraitDef::span span (self.span in fn create_derived_impl).

This visitor only needs to replace the visited span's context with the TraitDef::span context while keeping the location.

fn visit_span(&mut self, span: &mut Span) {
    *span = span.with_ctxt(self.self_span_from_TraitDef);
}

Copy link
Contributor Author

@estebank estebank Dec 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When doing that I get failures during bootstrap when computing derives, so I assumed I was doing something wrong:

error[E0412]: cannot find type `Range` in this scope
   --> library/core/src/ops/range.rs:78:37
    |
78  |   #[derive(Clone, Default, PartialEq, Eq, Hash)] // not Copy -- see #27186
    |                                       ^^
    |                                       |
    |                                       not found in this scope
    |                                       in this derive macro expansion
    |
   ::: library/core/src/cmp.rs:293:1
    |
293 | / pub macro Eq($item:item) {
294 | |     /* compiler built-in */
295 | | }
    | |_- in this expansion of `#[derive(Eq)]`
    |
help: consider importing this struct
    |
1   | use crate::ops::Range;

With the apply_mark approach on the other hand I could get through bootstrap consistently.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, of course, some parts of the output should not be visited, after all.

The Foo parts, specifically, i.e. references to the type for which we are deriving the impl.

#[derive(Default)]
struct Foo;
#[automatically_derived]
#[allow(unused_qualifications)]
impl ::core::default::Default for Foo {
    #[inline]
    fn default() -> Foo { Foo{} }
}

// *sp = sp.apply_mark(self.container_id.to_expn_id(), rustc_span::hygiene::Transparency::Transparent);
// sp.apply_mark(self.ctxt.outer_expn(), rustc_span::hygiene::Transparency::Transparent);
// *sp = sp.apply_mark(self.container_id.to_expn_id(), rustc_span::hygiene::Transparency::Opaque);
}
}
}
6 changes: 6 additions & 0 deletions src/test/ui/const-generics/issues/issue-74950.min.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ LL | struct Outer<const I: Inner>;
error: `Inner` is forbidden as the type of a const generic parameter
--> $DIR/issue-74950.rs:17:23
|
LL | #[derive(PartialEq, Eq)]
| --------- in this derive macro expansion
LL | struct Outer<const I: Inner>;
| ^^^^^
|
= note: the only supported types are integers, `bool` and `char`
= help: more complex types are supported with `#![feature(adt_const_params)]`
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)

error: `Inner` is forbidden as the type of a const generic parameter
--> $DIR/issue-74950.rs:17:23
Expand All @@ -37,11 +40,14 @@ LL | struct Outer<const I: Inner>;
error: `Inner` is forbidden as the type of a const generic parameter
--> $DIR/issue-74950.rs:17:23
|
LL | #[derive(PartialEq, Eq)]
| -- in this derive macro expansion
LL | struct Outer<const I: Inner>;
| ^^^^^
|
= note: the only supported types are integers, `bool` and `char`
= help: more complex types are supported with `#![feature(adt_const_params)]`
= note: this error originates in the derive macro `Eq` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 5 previous errors

64 changes: 59 additions & 5 deletions src/test/ui/hygiene/nested_macro_privacy.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,63 @@
error[E0616]: field `i` of struct `S` is private
--> $DIR/nested_macro_privacy.rs:15:18
error[E0412]: cannot find type `S` in this scope
--> $DIR/nested_macro_privacy.rs:5:18
|
LL | #[derive(Default)]
| ^^^^^^^ not found in this scope
...
LL | n!(foo, S, i, m);
| ---------------- in this macro invocation
|
= note: this error originates in the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0412]: cannot find type `S` in this scope
--> $DIR/nested_macro_privacy.rs:5:18
|
LL | #[derive(Default)]
| ^^^^^^^ not found in this scope
...
LL | n!(foo, S, i, m);
| ---------------- in this macro invocation
|
= note: this error originates in the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0422]: cannot find struct, variant or union type `S` in this scope
--> $DIR/nested_macro_privacy.rs:5:18
|
LL | #[derive(Default)]
| ^^^^^^^ not found in this scope
...
LL | n!(foo, S, i, m);
| ---------------- in this macro invocation
|
= note: this error originates in the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0599]: no function or associated item named `default` found for struct `S` in the current scope
--> $DIR/nested_macro_privacy.rs:15:8
|
LL | pub struct $S { $i: u32 }
| ------------- function or associated item `default` not found for this
...
LL | S::default().i;
| ^ private field
| ^^^^^^^ function or associated item not found in `S`
|
= help: items from traits can only be used if the trait is implemented and in scope
= note: the following trait defines an item `default`, perhaps you need to implement it:
candidate #1: `Default`

error[E0599]: no function or associated item named `default` found for struct `S` in the current scope
--> $DIR/nested_macro_privacy.rs:16:11
|
LL | pub struct $S { $i: u32 }
| ------------- function or associated item `default` not found for this
...
LL | m!(S::default()); // ok
| ^^^^^^^ function or associated item not found in `S`
|
= help: items from traits can only be used if the trait is implemented and in scope
= note: the following trait defines an item `default`, perhaps you need to implement it:
candidate #1: `Default`

error: aborting due to previous error
error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0616`.
Some errors have detailed explanations: E0412, E0422, E0599.
For more information about an error, try `rustc --explain E0412`.
5 changes: 5 additions & 0 deletions src/test/ui/issues/issue-50480.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,13 @@ LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
error[E0412]: cannot find type `NotDefined` in this scope
--> $DIR/issue-50480.rs:3:12
|
LL | #[derive(Clone, Copy)]
| ----- in this derive macro expansion
LL |
LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
| ^^^^^^^^^^ not found in this scope
|
= note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `i32` is not an iterator
--> $DIR/issue-50480.rs:3:24
Expand Down
6 changes: 5 additions & 1 deletion src/test/ui/lifetimes/undeclared-lifetime-used-in-debug-macro-issue-70152.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,16 @@ 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
| --
| |
| lifetime `'b` is missing in item created through this procedural macro
| in this derive macro expansion
LL | struct Test {
LL | a: &'b str,
| ^^ undeclared lifetime
|
= help: if you want to experiment with in-band lifetime bindings, add `#![feature(in_band_lifetimes)]` to the crate attributes
= note: this error originates in the derive macro `Eq` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0261]: use of undeclared lifetime name `'b`
--> $DIR/undeclared-lifetime-used-in-debug-macro-issue-70152.rs:13:13
Expand Down