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.

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

syntax: add ast::ItemKind::MacroDef, simplify hygiene info #40220

Merged
merged 3 commits into from
Mar 12, 2017

Conversation

jseyfried
Copy link
Contributor

This PR

  • adds a new variant MacroDef to ast::ItemKind for macro_rules! and eventually macro items,
  • [breaking-change] forbids macro defs without a name (macro_rules! { () => {} } compiles today),
  • removes ast::MacroDef, and
  • no longer uses Mark and Invocation to identify and characterize macro definitions.
    • We used to apply (at least) two Marks to an expanded identifier's SyntaxContext -- the definition mark(s) and the expansion mark(s). We now only apply the latter.

r? @nrc

@jseyfried
Copy link
Contributor Author

jseyfried commented Mar 3, 2017

cc #39412
cc @keeperofdakeys

@bors
Copy link
Contributor

bors commented Mar 3, 2017

☔ The latest upstream changes (presumably #39927) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Mar 4, 2017

☔ The latest upstream changes (presumably #40202) made this pull request unmergeable. Please resolve the merge conflicts.

@jseyfried jseyfried force-pushed the ast_macro_def branch 2 times, most recently from dc42522 to 9057c8d Compare March 7, 2017 00:23
@nrc
Copy link
Member

nrc commented Mar 8, 2017

@bors: r+

@bors
Copy link
Contributor

bors commented Mar 8, 2017

📌 Commit 9057c8d has been approved by nrc

@bors
Copy link
Contributor

bors commented Mar 9, 2017

🔒 Merge conflict

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Mar 9, 2017

🔒 Merge conflict

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Mar 10, 2017

🔒 Merge conflict

@alexcrichton
Copy link
Member

@bors: retry

@alexcrichton
Copy link
Member

(oops)

@bors: r+

@bors
Copy link
Contributor

bors commented Mar 10, 2017

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
  • There's another pull request that is currently being tested, blocking this pull request: rustbuild: Use copies instead of hard links #39518

@bors
Copy link
Contributor

bors commented Mar 10, 2017

📌 Commit 9057c8d has been approved by alexcrichton

@alexcrichton
Copy link
Member

@bors: retry

@alexcrichton
Copy link
Member

@bors: r=nrc

@bors
Copy link
Contributor

bors commented Mar 10, 2017

📌 Commit 8c98996 has been approved by nrc

@bors
Copy link
Contributor

bors commented Mar 11, 2017

⌛ Testing commit 8c98996 with merge 223844a...

@bors
Copy link
Contributor

bors commented Mar 11, 2017

💔 Test failed - status-appveyor

@alexcrichton
Copy link
Member

alexcrichton commented Mar 11, 2017 via email

@bors
Copy link
Contributor

bors commented Mar 11, 2017

⌛ Testing commit 8c98996 with merge 1b19284...

bors added a commit that referenced this pull request Mar 11, 2017
syntax: add `ast::ItemKind::MacroDef`, simplify hygiene info

This PR
 - adds a new variant `MacroDef` to `ast::ItemKind` for `macro_rules!` and eventually `macro` items,
 - [breaking-change] forbids macro defs without a name (`macro_rules! { () => {} }` compiles today),
 - removes `ast::MacroDef`, and
 - no longer uses `Mark` and `Invocation` to identify and characterize macro definitions.
   - We used to apply (at least) two `Mark`s to an expanded identifier's `SyntaxContext` -- the definition mark(s) and the expansion mark(s). We now only apply the latter.

r? @nrc
@bors
Copy link
Contributor

bors commented Mar 12, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: nrc
Pushing 1b19284 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants