Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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
Collaborator

bors commented Mar 3, 2017

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

@bors
Copy link
Collaborator

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
Collaborator

bors commented Mar 8, 2017

📌 Commit 9057c8d has been approved by nrc

@bors
Copy link
Collaborator

bors commented Mar 9, 2017

🔒 Merge conflict

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Collaborator

bors commented Mar 9, 2017

🔒 Merge conflict

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Collaborator

bors commented Mar 10, 2017

🔒 Merge conflict

@alexcrichton
Copy link
Member

@bors: retry

@alexcrichton
Copy link
Member

(oops)

@bors: r+

@bors
Copy link
Collaborator

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
Collaborator

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
Collaborator

bors commented Mar 10, 2017

📌 Commit 8c98996 has been approved by nrc

@bors
Copy link
Collaborator

bors commented Mar 11, 2017

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

@bors
Copy link
Collaborator

bors commented Mar 11, 2017

💔 Test failed - status-appveyor

@alexcrichton
Copy link
Member

alexcrichton commented Mar 11, 2017 via email

@bors
Copy link
Collaborator

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
Collaborator

bors commented Mar 12, 2017

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

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

Successfully merging this pull request may close these issues.

4 participants