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.

Sign up for GitHub

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

Jump to bottom

Cleanup span quoting #87042

Merged
merged 4 commits into from
Jul 11, 2021
Merged

Cleanup span quoting #87042

merged 4 commits into from
Jul 11, 2021

Conversation

petrochenkov
Copy link
Contributor

I finally got to reviewing #84278.
See the individual commit messages.
r? @Aaron1011

@petrochenkov
rustc_span: Revert addition of proc_macro field to ExpnKind::Macro
28f4dba 
The flag has a vague meaning and is used for a single diagnostic change that is low benefit and appears only under `-Z macro_backtrace`.
@petrochenkov
rustc_expand: Remove redundant field from proc macro expander structures
de897f5 
This information is already available from `ExpnData`
@petrochenkov
rustc_expand: Simplify span quoting in proc macro server
ece6f68 
- The `Rustc::expn_id` field kept redundant information
- `SyntaxContext` is no longer thrown away before `save_proc_macro_span` because it's thrown away during metadata encoding anyway
@petrochenkov
rustc_span: Reorder some ExpnData fields in accordance with comments
4ba91a0 
A drive-by change.
@rust-highfive
Copy link
Collaborator

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 10, 2021
@petrochenkov
Copy link
Contributor Author

I still can't say I like how #84278 is done, but with all redundant data removed the implementation is at least isolated enough, and I can't suggest a fundamentally better implementation strategy right now.

@Aaron1011
Copy link
Member

I still think that displaying 'procedural macro' in the backtrace output is useful (especially if we ever stabilize -Z macro-hacktrace at some point). If proc-macros ever start to use quoted spans, then I think it will be useful to make it clear that a particular span is coming from a proc-macro - if it's inside some helper function in a proc-macro crate, it might not be obvious.

However, we're still a ways off from this feature actually being used (since the quote crate still needs to be updated), so I'm okay with reverting it. If it ever becomes an issue in practice, I'll try to find a less-invasive way of tracking the needed information.

@Aaron1011
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jul 11, 2021

📌 Commit 4ba91a0 has been approved by Aaron1011

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 11, 2021
@bors
Copy link
Contributor

bors commented Jul 11, 2021

⌛ Testing commit 4ba91a0 with merge 4581c4e...

@bors
Copy link
Contributor

bors commented Jul 11, 2021

☀️ Test successful - checks-actions
Approved by: Aaron1011
Pushing 4581c4e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 11, 2021
@bors bors merged commit 4581c4e into rust-lang:master Jul 11, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jul 11, 2021
This was referenced Jul 11, 2021
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Aaron1011 Aaron1011

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.55.0
Development

Successfully merging this pull request may close these issues.
5 participants
@petrochenkov @rust-highfive @Aaron1011 @bors @rustbot