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

resolve/hygiene: macro_rules are not "legacy" #69989

Merged
merged 4 commits into from
Mar 16, 2020

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Mar 13, 2020

The "modern" vs "legacy" naming was introduced by jseyfried during initial implementation of macros 2.0.
At this point it's clear that macro_rules are not going anywhere and won't be deprecated in the near future.
So this PR changes the naming "legacy" (when it implies "macro_rules") to "macro_rules".
This should also help people reading this code because it's wasn't obvious that "legacy" actually meant "macro_rules" in these contexts.

The most contentious renaming here is probably

fn modern -> fn normalize_to_macros_2_0
fn modern_and_legacy -> fn normalize_to_macro_rules

Other alternatives that I could think of are normalize_to_opaque/normalize_to_semitransparent, or strip_non_opaque/strip_transparent, but they seemed less intuitive.
The documentation to these functions can be found in symbol.rs.

r? @matthewjasper

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 13, 2020
@Centril
Copy link
Contributor

Centril commented Mar 14, 2020

The most contentious renaming here is probably

This seems like a good idea, much clearer!

Copy link
Member

@eddyb eddyb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me on the renaming diff if nobody disagrees on the choices themselves

@matthewjasper
Copy link
Contributor

@bors r=eddyb,matthewjasper

@bors
Copy link
Contributor

bors commented Mar 14, 2020

📌 Commit 7a1ecace75e29b8145ce8642b7b70e87728b74f3 has been approved by eddyb,matthewjasper

@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 Mar 14, 2020
@bors
Copy link
Contributor

bors commented Mar 15, 2020

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

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 15, 2020
@petrochenkov
Copy link
Contributor Author

@bors r=eddyb,matthewjasper

@bors
Copy link
Contributor

bors commented Mar 15, 2020

📌 Commit db638bd has been approved by eddyb,matthewjasper

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 15, 2020
bors added a commit that referenced this pull request Mar 16, 2020
Rollup of 7 pull requests

Successful merges:

 - #67335 (Refactor the `Qualif` trait)
 - #69122 (Backtrace Debug tweaks)
 - #69520 (Make error message clearer about creating new module)
 - #69738 (More Method -> AssocFn renaming)
 - #69867 (Add long error explanation for E0628 )
 - #69989 (resolve/hygiene: `macro_rules` are not "legacy")
 - #70036 (Make article_and_description primarily use def_kind)

Failed merges:

r? @ghost
@bors bors merged commit 8872d90 into rust-lang:master Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants