Skip to content

Commit

Permalink
Auto merge of #49326 - petrochenkov:nteq, r=eddyb
Browse files Browse the repository at this point in the history
macros: Remove matching on "complex" nonterminals requiring AST comparisons

So, you can actually use nonterminals from outer macros in left hand side of nested macros and invocations of nested macros will try to match passed arguments to them.

```rust
macro outer($nt_item: item) {
    macro inner($nt_item) {
        struct S;
    }

    inner!($nt_item); // OK, `$nt_item` matches `$nt_item`
}
```

Why this is bad:
- We can't do this matching correctly. When two nonterminals are compared, the original tokens are lost and we have to compare AST fragments instead. Right now the comparison is done by `PartialEq` impls derived on AST structures.
    - On one hand, AST loses information compared to original tokens (e.g. trailing separators and other simplifications done during parsing to AST), so we can produce matches that are not actually correct.
    - On another hand derived `PartialEq` impls for AST structures don't make much sense in general and compare various auxiliary garbage like spans. For the argument nonterminal to match we should use literally the same token (possibly cloned) as was used in the macro LHS (as in the example above). So we can reject matches that are actually correct.
    - Support for nonterminal matching is the only thing that forces us to derive `PartialEq` for all (!) AST structures. As I mentioned these impls are also mostly nonsensical.

This PR removes support for matching on all nonterminals except for "simple" ones like `ident`, `lifetime` and `tt` for which we have original tokens that can be compared.
After this is done I'll submit another PR removing huge number of `PartialEq` impls from AST and HIR structures.

This is an arcane feature and I don't personally know why would anyone use it, but the change should ideally go through crater.
We'll be able to support this feature again in the future when all nonterminals have original token streams attached to them in addition to (or instead of) AST fragments.
  • Loading branch information
bors committed Apr 14, 2018
2 parents e7252f6 + 7e1f73b commit 5724462
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 1 deletion.
18 changes: 17 additions & 1 deletion src/libsyntax/parse/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ impl Token {
}
}

#[derive(Clone, RustcEncodable, RustcDecodable, PartialEq, Eq, Hash)]
#[derive(Clone, RustcEncodable, RustcDecodable, Eq, Hash)]
/// For interpolation during macro expansion.
pub enum Nonterminal {
NtItem(P<ast::Item>),
Expand All @@ -591,6 +591,22 @@ pub enum Nonterminal {
NtArg(ast::Arg),
}

impl PartialEq for Nonterminal {
fn eq(&self, rhs: &Self) -> bool {
match (self, rhs) {
(NtIdent(ident_lhs, is_raw_lhs), NtIdent(ident_rhs, is_raw_rhs)) =>
ident_lhs == ident_rhs && is_raw_lhs == is_raw_rhs,
(NtLifetime(ident_lhs), NtLifetime(ident_rhs)) => ident_lhs == ident_rhs,
(NtTT(tt_lhs), NtTT(tt_rhs)) => tt_lhs == tt_rhs,
// FIXME: Assume that all "complex" nonterminal are not equal, we can't compare them
// correctly based on data from AST. This will prevent them from matching each other
// in macros. The comparison will become possible only when each nonterminal has an
// attached token stream from which it was parsed.
_ => false,
}
}
}

impl fmt::Debug for Nonterminal {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
Expand Down
36 changes: 36 additions & 0 deletions src/test/ui/macros/nonterminal-matching.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Check that we are refusing to match on complex nonterminals for which tokens are
// unavailable and we'd have to go through AST comparisons.

#![feature(decl_macro, macro_lifetime_matcher)]

macro simple_nonterminal($nt_ident: ident, $nt_lifetime: lifetime, $nt_tt: tt) {
macro n(a $nt_ident b $nt_lifetime c $nt_tt d) {
struct S;
}

n!(a $nt_ident b $nt_lifetime c $nt_tt d);
}

macro complex_nonterminal($nt_item: item) {
macro n(a $nt_item b) {
struct S;
}

n!(a $nt_item b); //~ ERROR no rules expected the token `enum E { }`
}

simple_nonterminal!(a, 'a, (x, y, z)); // OK

complex_nonterminal!(enum E {});

fn main() {}
11 changes: 11 additions & 0 deletions src/test/ui/macros/nonterminal-matching.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: no rules expected the token `enum E { }`
--> $DIR/nonterminal-matching.rs:29:10
|
LL | n!(a $nt_item b); //~ ERROR no rules expected the token `enum E { }`
| ^^^^^^^^
...
LL | complex_nonterminal!(enum E {});
| -------------------------------- in this macro invocation

error: aborting due to previous error

0 comments on commit 5724462

Please sign in to comment.