Skip to content

Commit

Permalink
Auto merge of #78712 - petrochenkov:visitok, r=Aaron1011
Browse files Browse the repository at this point in the history
rustc_ast: Visit tokens stored in AST nodes in mutable visitor

After #77271 token visiting is enabled only for one visitor in `rustc_expand\src\mbe\transcribe.rs` which applies hygiene marks to tokens produced by declarative macros (`macro_rules` or `macro`), so this change doesn't affect anything else.

When a macro has some interpolated token from an outer macro in its output
```rust
macro inner() {
    $interpolated
}
```
we can use the usual interpretation of interpolated tokens in token-based model - a None-delimited group - to write this macro in an equivalent form
```rust
macro inner() {
    ⟪ a b c d ⟫
}
```

When we are expanding the macro `inner` we need to apply hygiene marks to all tokens produced by it, including the tokens inside the group.

Before this PR we did this by visiting the AST piece inside the interpolated token and applying marks to all spans in it.
I'm not sure this is 100% correct (ideally we should apply the marks to tokens and then re-parse the AST from tokens), but it's a very good approximation at least.
We didn't however apply the marks to actual tokens stored in the nonterminal, so if we used the nonterminal as a token rather than as an AST piece (e.g. passed it to a proc macro), then we got hygiene bugs.
This PR applies the marks to tokens in addition to the AST pieces thus fixing the issue.

r? `@Aaron1011`
  • Loading branch information
bors committed Nov 8, 2020
2 parents b1277d0 + 8def2fc commit 1773f60
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 16 deletions.
51 changes: 35 additions & 16 deletions compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ pub fn noop_visit_ty_constraint<T: MutVisitor>(
}

pub fn noop_visit_ty<T: MutVisitor>(ty: &mut P<Ty>, vis: &mut T) {
let Ty { id, kind, span, tokens: _ } = ty.deref_mut();
let Ty { id, kind, span, tokens } = ty.deref_mut();
vis.visit_id(id);
match kind {
TyKind::Infer | TyKind::ImplicitSelf | TyKind::Err | TyKind::Never | TyKind::CVarArgs => {}
Expand Down Expand Up @@ -497,6 +497,7 @@ pub fn noop_visit_ty<T: MutVisitor>(ty: &mut P<Ty>, vis: &mut T) {
TyKind::MacCall(mac) => vis.visit_mac(mac),
}
vis.visit_span(span);
visit_lazy_tts(tokens, vis);
}

pub fn noop_visit_foreign_mod<T: MutVisitor>(foreign_mod: &mut ForeignMod, vis: &mut T) {
Expand All @@ -523,13 +524,14 @@ pub fn noop_visit_ident<T: MutVisitor>(Ident { name: _, span }: &mut Ident, vis:
vis.visit_span(span);
}

pub fn noop_visit_path<T: MutVisitor>(Path { segments, span, tokens: _ }: &mut Path, vis: &mut T) {
pub fn noop_visit_path<T: MutVisitor>(Path { segments, span, tokens }: &mut Path, vis: &mut T) {
vis.visit_span(span);
for PathSegment { ident, id, args } in segments {
vis.visit_ident(ident);
vis.visit_id(id);
visit_opt(args, |args| vis.visit_generic_args(args));
}
visit_lazy_tts(tokens, vis);
}

pub fn noop_visit_qself<T: MutVisitor>(qself: &mut Option<QSelf>, vis: &mut T) {
Expand Down Expand Up @@ -587,15 +589,17 @@ pub fn noop_visit_local<T: MutVisitor>(local: &mut P<Local>, vis: &mut T) {
}

pub fn noop_visit_attribute<T: MutVisitor>(attr: &mut Attribute, vis: &mut T) {
let Attribute { kind, id: _, style: _, span, tokens: _ } = attr;
let Attribute { kind, id: _, style: _, span, tokens } = attr;
match kind {
AttrKind::Normal(AttrItem { path, args, tokens: _ }) => {
AttrKind::Normal(AttrItem { path, args, tokens }) => {
vis.visit_path(path);
visit_mac_args(args, vis);
visit_lazy_tts(tokens, vis);
}
AttrKind::DocComment(..) => {}
}
vis.visit_span(span);
visit_lazy_tts(tokens, vis);
}

pub fn noop_visit_mac<T: MutVisitor>(mac: &mut MacCall, vis: &mut T) {
Expand Down Expand Up @@ -652,12 +656,22 @@ pub fn visit_tt<T: MutVisitor>(tt: &mut TokenTree, vis: &mut T) {

// No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
pub fn visit_tts<T: MutVisitor>(TokenStream(tts): &mut TokenStream, vis: &mut T) {
if vis.token_visiting_enabled() {
if vis.token_visiting_enabled() && !tts.is_empty() {
let tts = Lrc::make_mut(tts);
visit_vec(tts, |(tree, _is_joint)| visit_tt(tree, vis));
}
}

pub fn visit_lazy_tts<T: MutVisitor>(lazy_tts: &mut Option<LazyTokenStream>, vis: &mut T) {
if vis.token_visiting_enabled() {
visit_opt(lazy_tts, |lazy_tts| {
let mut tts = lazy_tts.create_token_stream();
visit_tts(&mut tts, vis);
*lazy_tts = LazyTokenStream::new(tts);
})
}
}

// No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
// Applies ident visitor if it's an ident; applies other visits to interpolated nodes.
// In practice the ident part is not actually used by specific visitors right now,
Expand Down Expand Up @@ -725,9 +739,10 @@ pub fn visit_interpolated<T: MutVisitor>(nt: &mut token::Nonterminal, vis: &mut
token::NtLifetime(ident) => vis.visit_ident(ident),
token::NtLiteral(expr) => vis.visit_expr(expr),
token::NtMeta(item) => {
let AttrItem { path, args, tokens: _ } = item.deref_mut();
let AttrItem { path, args, tokens } = item.deref_mut();
vis.visit_path(path);
visit_mac_args(args, vis);
visit_lazy_tts(tokens, vis);
}
token::NtPath(path) => vis.visit_path(path),
token::NtTT(tt) => visit_tt(tt, vis),
Expand Down Expand Up @@ -887,10 +902,11 @@ pub fn noop_visit_mt<T: MutVisitor>(MutTy { ty, mutbl: _ }: &mut MutTy, vis: &mu
}

pub fn noop_visit_block<T: MutVisitor>(block: &mut P<Block>, vis: &mut T) {
let Block { id, stmts, rules: _, span, tokens: _ } = block.deref_mut();
let Block { id, stmts, rules: _, span, tokens } = block.deref_mut();
vis.visit_id(id);
stmts.flat_map_in_place(|stmt| vis.flat_map_stmt(stmt));
vis.visit_span(span);
visit_lazy_tts(tokens, vis);
}

pub fn noop_visit_item_kind<T: MutVisitor>(kind: &mut ItemKind, vis: &mut T) {
Expand Down Expand Up @@ -955,7 +971,7 @@ pub fn noop_flat_map_assoc_item<T: MutVisitor>(
mut item: P<AssocItem>,
visitor: &mut T,
) -> SmallVec<[P<AssocItem>; 1]> {
let Item { id, ident, vis, attrs, kind, span, tokens: _ } = item.deref_mut();
let Item { id, ident, vis, attrs, kind, span, tokens } = item.deref_mut();
visitor.visit_id(id);
visitor.visit_ident(ident);
visitor.visit_vis(vis);
Expand All @@ -978,6 +994,7 @@ pub fn noop_flat_map_assoc_item<T: MutVisitor>(
AssocItemKind::MacCall(mac) => visitor.visit_mac(mac),
}
visitor.visit_span(span);
visit_lazy_tts(tokens, visitor);
smallvec![item]
}

Expand Down Expand Up @@ -1028,16 +1045,14 @@ pub fn noop_flat_map_item<T: MutVisitor>(
mut item: P<Item>,
visitor: &mut T,
) -> SmallVec<[P<Item>; 1]> {
let Item { ident, attrs, id, kind, vis, span, tokens: _ } = item.deref_mut();
let Item { ident, attrs, id, kind, vis, span, tokens } = item.deref_mut();
visitor.visit_ident(ident);
visit_attrs(attrs, visitor);
visitor.visit_id(id);
visitor.visit_item_kind(kind);
visitor.visit_vis(vis);
visitor.visit_span(span);

// FIXME: if `tokens` is modified with a call to `vis.visit_tts` it causes
// an ICE during resolve... odd!
visit_lazy_tts(tokens, visitor);

smallvec![item]
}
Expand All @@ -1046,7 +1061,7 @@ pub fn noop_flat_map_foreign_item<T: MutVisitor>(
mut item: P<ForeignItem>,
visitor: &mut T,
) -> SmallVec<[P<ForeignItem>; 1]> {
let Item { ident, attrs, id, kind, vis, span, tokens: _ } = item.deref_mut();
let Item { ident, attrs, id, kind, vis, span, tokens } = item.deref_mut();
visitor.visit_id(id);
visitor.visit_ident(ident);
visitor.visit_vis(vis);
Expand All @@ -1069,11 +1084,12 @@ pub fn noop_flat_map_foreign_item<T: MutVisitor>(
ForeignItemKind::MacCall(mac) => visitor.visit_mac(mac),
}
visitor.visit_span(span);
visit_lazy_tts(tokens, visitor);
smallvec![item]
}

pub fn noop_visit_pat<T: MutVisitor>(pat: &mut P<Pat>, vis: &mut T) {
let Pat { id, kind, span, tokens: _ } = pat.deref_mut();
let Pat { id, kind, span, tokens } = pat.deref_mut();
vis.visit_id(id);
match kind {
PatKind::Wild | PatKind::Rest => {}
Expand Down Expand Up @@ -1108,6 +1124,7 @@ pub fn noop_visit_pat<T: MutVisitor>(pat: &mut P<Pat>, vis: &mut T) {
PatKind::MacCall(mac) => vis.visit_mac(mac),
}
vis.visit_span(span);
visit_lazy_tts(tokens, vis);
}

pub fn noop_visit_anon_const<T: MutVisitor>(AnonConst { id, value }: &mut AnonConst, vis: &mut T) {
Expand All @@ -1116,7 +1133,7 @@ pub fn noop_visit_anon_const<T: MutVisitor>(AnonConst { id, value }: &mut AnonCo
}

pub fn noop_visit_expr<T: MutVisitor>(
Expr { kind, id, span, attrs, tokens: _ }: &mut Expr,
Expr { kind, id, span, attrs, tokens }: &mut Expr,
vis: &mut T,
) {
match kind {
Expand Down Expand Up @@ -1295,6 +1312,7 @@ pub fn noop_visit_expr<T: MutVisitor>(
vis.visit_id(id);
vis.visit_span(span);
visit_thin_attrs(attrs, vis);
visit_lazy_tts(tokens, vis);
}

pub fn noop_filter_map_expr<T: MutVisitor>(mut e: P<Expr>, vis: &mut T) -> Option<P<Expr>> {
Expand All @@ -1305,11 +1323,12 @@ pub fn noop_filter_map_expr<T: MutVisitor>(mut e: P<Expr>, vis: &mut T) -> Optio
}

pub fn noop_flat_map_stmt<T: MutVisitor>(
Stmt { kind, mut span, mut id, tokens }: Stmt,
Stmt { kind, mut span, mut id, mut tokens }: Stmt,
vis: &mut T,
) -> SmallVec<[Stmt; 1]> {
vis.visit_id(&mut id);
vis.visit_span(&mut span);
visit_lazy_tts(&mut tokens, vis);
noop_flat_map_stmt_kind(kind, vis)
.into_iter()
.map(|kind| Stmt { id, kind, span, tokens: tokens.clone() })
Expand Down
33 changes: 33 additions & 0 deletions src/test/ui/proc-macro/nonterminal-token-hygiene.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Make sure that marks from declarative macros are applied to tokens in nonterminal.

// check-pass
// compile-flags: -Z span-debug -Z macro-backtrace -Z unpretty=expanded,hygiene
// compile-flags: -Z trim-diagnostic-paths=no
// normalize-stdout-test "\d+#" -> "0#"
// aux-build:test-macros.rs

#![feature(decl_macro)]

#![no_std] // Don't load unnecessary hygiene information from std
extern crate std;

#[macro_use]
extern crate test_macros;

macro_rules! outer {
($item:item) => {
macro inner() {
print_bang! { $item }
}

inner!();
};
}

struct S;

outer! {
struct S; // OK, not a duplicate definition of `S`
}

fn main() {}
88 changes: 88 additions & 0 deletions src/test/ui/proc-macro/nonterminal-token-hygiene.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
PRINT-BANG INPUT (DISPLAY): struct S;
PRINT-BANG RE-COLLECTED (DISPLAY): struct S ;
PRINT-BANG INPUT (DEBUG): TokenStream [
Group {
delimiter: None,
stream: TokenStream [
Ident {
ident: "struct",
span: $DIR/nonterminal-token-hygiene.rs:30:5: 30:11 (#5),
},
Ident {
ident: "S",
span: $DIR/nonterminal-token-hygiene.rs:30:12: 30:13 (#5),
},
Punct {
ch: ';',
spacing: Alone,
span: $DIR/nonterminal-token-hygiene.rs:30:13: 30:14 (#5),
},
],
span: $DIR/nonterminal-token-hygiene.rs:20:27: 20:32 (#6),
},
]
#![feature /* 0#0 */(prelude_import)]
#![no_std /* 0#0 */]
// Make sure that marks from declarative macros are applied to tokens in nonterminal.

// check-pass
// compile-flags: -Z span-debug -Z macro-backtrace -Z unpretty=expanded,hygiene
// compile-flags: -Z trim-diagnostic-paths=no
// normalize-stdout-test "\d+#" -> "0#"
// aux-build:test-macros.rs

#![feature /* 0#0 */(decl_macro)]

#![no_std /* 0#0 */]
#[prelude_import /* 0#1 */]
use ::core /* 0#1 */::prelude /* 0#1 */::v1 /* 0#1 */::*;
#[macro_use /* 0#1 */]
extern crate core /* 0#2 */;
#[macro_use /* 0#1 */]
extern crate compiler_builtins /* 0#2 */;
// Don't load unnecessary hygiene information from std
extern crate std /* 0#0 */;

#[macro_use /* 0#0 */]
extern crate test_macros /* 0#0 */;

macro_rules! outer
/*
0#0
*/ {
($ item : item) =>
{
macro inner() { print_bang ! { $ item } } inner ! () ;

} ;
}

struct S /* 0#0 */;
macro inner /* 0#4 */ { () => { print_bang ! { struct S; } } }

struct S /* 0#5 */;
// OK, not a duplicate definition of `S`

fn main /* 0#0 */() { }

/*
Expansions:
0: parent: ExpnId(0), call_site_ctxt: #0, def_site_ctxt: #0, kind: Root
1: parent: ExpnId(0), call_site_ctxt: #0, def_site_ctxt: #0, kind: AstPass(StdImports)
2: parent: ExpnId(0), call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Bang, "outer")
3: parent: ExpnId(0), call_site_ctxt: #0, def_site_ctxt: #0, kind: AstPass(StdImports)
4: parent: ExpnId(2), call_site_ctxt: #4, def_site_ctxt: #4, kind: Macro(Bang, "inner")
5: parent: ExpnId(4), call_site_ctxt: #6, def_site_ctxt: #0, kind: Macro(Bang, "print_bang")

SyntaxContexts:
#0: parent: #0, outer_mark: (ExpnId(0), Opaque)
#1: parent: #0, outer_mark: (ExpnId(1), Opaque)
#2: parent: #0, outer_mark: (ExpnId(1), Transparent)
#3: parent: #0, outer_mark: (ExpnId(3), Opaque)
#4: parent: #0, outer_mark: (ExpnId(2), SemiTransparent)
#5: parent: #0, outer_mark: (ExpnId(4), Opaque)
#6: parent: #4, outer_mark: (ExpnId(4), Opaque)
#7: parent: #0, outer_mark: (ExpnId(5), Opaque)
#8: parent: #6, outer_mark: (ExpnId(5), Transparent)
#9: parent: #5, outer_mark: (ExpnId(5), SemiTransparent)
*/

0 comments on commit 1773f60

Please sign in to comment.