macros: improve Span's expansion information by jseyfried · Pull Request #40597 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation44 Commits5 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This PR improves Span's expansion information. More specifically:
- It refactors AST node span construction to preserve expansion information.
- Today, we only use the underlying tokens'
BytePoss, throwing away theExpnIds. - This improves the accuracy of AST nodes' expansion information, fixing If an error is not affected by a macro, don't print macro notes #30506.
- Today, we only use the underlying tokens'
- It refactors
span.expn_id: ExpnIdtospan.ctxt: SyntaxContextand removesExpnId.- This gives all tokens as much hygiene information as
Idents. - This is groundwork for procedural macros 2.0
TokenStreamAPI. - This is also groundwork for declarative macros 2.0, which will need this hygiene information for some non-
Identtokens.
- This gives all tokens as much hygiene information as
- It simplifies processing of spans' expansion information throughout the compiler.
- It fixes macros: The line number that panic! reports can be wrong in macro invocations #40649.
- It fixes Spans for Paths can be incorrect #39450 and fixes part of Macro expansion often produces invalid Span values #23480.
r? @nrc
This was referenced
Mar 17, 2017
| @@ -22,7 +22,7 @@ macro_rules! indirect_line { () => ( line!() ) } |
|---|
| pub fn main() { |
| assert_eq!(line!(), 24); |
| assert_eq!(column!(), 4); |
| assert_eq!(column!(), 15); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c.f. #40649; technically a [breaking-change]
☔ The latest upstream changes (presumably #40346) made this pull request unmergeable. Please resolve the merge conflicts.
☔ The latest upstream changes (presumably #40693) made this pull request unmergeable. Please resolve the merge conflicts.
☔ The latest upstream changes (presumably #40043) made this pull request unmergeable. Please resolve the merge conflicts.
☔ The latest upstream changes (presumably #40752) made this pull request unmergeable. Please resolve the merge conflicts.
nrc approved these changes Mar 24, 2017
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me, assuming losing the command line span thing is fine.
This seems a good step in the right direction. I worry a bit that to is still too easy to create malformed spans, but it is def. better than mk_sp.
| // Generic span to be used for code originating from the command line |
|---|
| pub const COMMAND_LINE_SP: Span = Span { lo: BytePos(0), |
| hi: BytePos(0), |
| expn_id: COMMAND_LINE_EXPN }; |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do we do now for code from the command line?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as before -- DUMMY_SP. The only place COMMAND_LINE_SP was used was here, which only happens when naming a legacy plugin crate through a debugging command line option.
📌 Commit 8c49446 has been approved by nrc
📌 Commit 8ba347e has been approved by nrc
⌛ Testing commit 8ba347e with merge 2731d4d...
bors added a commit that referenced this pull request
macros: improve Span's expansion information
This PR improves Span's expansion information. More specifically:
- It refactors AST node span construction to preserve expansion information.
- Today, we only use the underlying tokens'
BytePoss, throwing away theExpnIds. - This improves the accuracy of AST nodes' expansion information, fixing #30506.
- Today, we only use the underlying tokens'
- It refactors
span.expn_id: ExpnIdtospan.ctxt: SyntaxContextand removesExpnId.- This gives all tokens as much hygiene information as
Idents. - This is groundwork for procedural macros 2.0
TokenStreamAPI. - This is also groundwork for declarative macros 2.0, which will need this hygiene information for some non-
Identtokens.
- This gives all tokens as much hygiene information as
- It simplifies processing of spans' expansion information throughout the compiler.
- It fixes #40649.
- It fixes #39450 and fixes part of #23480.
r? @nrc
📌 Commit 8fde04b has been approved by nrc
This was referenced
Mar 30, 2017
I'm seeing the following, which may be related to this:
warning: {warning message from a syntax extension} --> /a/nice/big/file/path/here.rs:10:21 | | and so on... | = note: this error originates in a macro outside of the current crate
The issue is that the "note:` refers to the warning as an "error". It should say: "this warning originates from a macro outside of the current crate".
This was referenced
May 8, 2017