Make macro metavars respect (non-)hygiene by matthewjasper · Pull Request #68746 · rust-lang/rust

matthewjasper · 2020-02-01T21:46:51Z

This makes them more consistent with other name resolution while not breaking any code on crater.

matthewjasper · 2020-02-02T09:20:31Z

bors · 2020-02-02T09:20:42Z

⌛ Trying commit 54bcfd60092e9f25afa12deeea9ccccf60fb921c with merge 3738e44c8fffe9e1fdfda9304a4df74ce7b29152...

bors · 2020-02-02T12:06:23Z

☀️ Try build successful - checks-azure
Build commit: 3738e44c8fffe9e1fdfda9304a4df74ce7b29152 (3738e44c8fffe9e1fdfda9304a4df74ce7b29152)

matthewjasper · 2020-02-02T12:10:46Z

@craterbot check

craterbot · 2020-02-02T12:10:50Z

👌 Experiment pr-68746 created and queued.
🤖 Automatically detected try build 3738e44c8fffe9e1fdfda9304a4df74ce7b29152
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2020-02-02T12:10:51Z

🚧 Experiment pr-68746 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2020-02-04T22:12:32Z

🎉 Experiment pr-68746 is completed!
📊 1 regressed and 0 fixed (88601 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

matthewjasper · 2020-02-05T07:17:16Z

Failure looks spurious. Checking perf looks ok
@rust-timer build 3738e44c8fffe9e1fdfda9304a4df74ce7b29152
r? @petrochenkov

rust-timer · 2020-02-05T07:17:18Z

Queued 3738e44c8fffe9e1fdfda9304a4df74ce7b29152 with parent 0cbcb17, future comparison URL.

rust-timer · 2020-02-05T11:56:31Z

Finished benchmarking try commit 3738e44c8fffe9e1fdfda9304a4df74ce7b29152, comparison URL.

petrochenkov · 2020-02-09T13:49:40Z

(This will probably has to wait until the next weekend.)

petrochenkov · 2020-02-16T12:44:24Z

This will have to wait for one more week, unfortunately.
(The change is nontrivial, but not super urgent, so it ends up in in the end of my review queue for the second weekend in a row.)

petrochenkov · 2020-02-23T22:06:09Z

I'd like to run a slightly different experiment through crater first - #69410.
Marking this as blocked for now.

petrochenkov · 2020-02-23T22:10:33Z

I'm generally skeptical about applying the term "name resolution" and name resolution techniques to things in macro_rules.

I don't think the current behavior is deliberate, it's probably mostly a consequence of PartialEq and Hash being implemented for Ident.
Other token comparisons in macro_rules deliberately use unhygienic token matching.
Proc macros are also only able to use token matching and name resolution is not available to them.

petrochenkov · 2020-03-08T23:38:55Z

I'm not sure what to do here.

Comparing full Idents as is (status quo) looks like the worst variant.
Proc macros should be able to generate arbitrary tokens for macro_rules items without interfering with their variable matching. So we certainly need to strip the "transparent" layers from the compared identifiers.
On the other hand, stripping more than only the "transparent" layers breaks code that intentionally relies on this "hygiene" as [experiment] Use unhygienic token matching for identifying macro variables #69410 shows.

Perhaps doing what this PR does is the least evil after all.
By doing it we effectively revert to the status quo before ~Rust 1.30 when we still compared full Idents, but didn't have the transparent layers at all.

petrochenkov · 2020-03-08T23:45:48Z

I have only one concern regarding the implementation - keeping Idents in the Binders map is pretty fragile because it's easy to forget to "normalize" them with modern_and_legacy().
It would be great to have Binders to keep a comparable newtype around Ident that performs the modern_and_legacy() normalization during its construction from Ident.

I wanted to migrate all ident comparisons to such newtypes since ~2015, I guess, but never got to this task.

…henkov Make macro metavars respect (non-)hygiene This makes them more consistent with other name resolution while not breaking any code on crater.

@ghost

Rollup of 8 pull requests Successful merges: - #68746 (Make macro metavars respect (non-)hygiene) - #69189 (Erase regions in writeback) - #69402 (Extend search) - #69403 (Implement `Copy` for `IoSlice`) - #69460 (Move some `build-pass` tests to `check-pass`) - #69802 (fix more clippy findings) - #69809 (remove lifetimes that can be elided (clippy::needless_lifetimes)) - #69949 (triagebot.toml: add ping aliases) Failed merges: - #69589 (ast: `Mac`/`Macro` -> `MacCall`) r? @ghost

bors · 2020-03-15T20:52:07Z

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

matthewjasper · 2020-03-16T11:23:41Z

@bors r=petrochenkov

bors · 2020-03-16T11:23:43Z

📌 Commit 0a9a4153433d99f49d5d6e283c234351a8dff73b has been approved by petrochenkov

matthewjasper · 2020-03-16T21:46:26Z

@bors r=petrochenkov

bors · 2020-03-16T21:46:27Z

📌 Commit ec86270 has been approved by petrochenkov

…henkov Make macro metavars respect (non-)hygiene This makes them more consistent with other name resolution while not breaking any code on crater.

@ghost

Rollup of 7 pull requests Successful merges: - #68746 (Make macro metavars respect (non-)hygiene) - #69688 (Move tidy check to mingw-check) - #69735 (bootstrap: Use hash to determine if sanitizers needs to be rebuilt) - #69922 (implement zeroed and uninitialized with MaybeUninit) - #69956 (Ensure HAS_FREE_LOCAL_NAMES is set for ReFree) - #70061 (Cosmetic fixes in documentation) - #70064 (Update books) Failed merges: r? @ghost

craterbot added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Feb 2, 2020

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Feb 4, 2020

rust-highfive assigned petrochenkov Feb 5, 2020

petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 23, 2020

petrochenkov mentioned this pull request Feb 23, 2020

[experiment] Use unhygienic token matching for identifying macro variables #69410

Closed

petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Mar 8, 2020

petrochenkov 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 8, 2020

matthewjasper force-pushed the metahygiene branch 2 times, most recently from 39824fe to d161124 Compare March 11, 2020 20:19

Centril mentioned this pull request Mar 12, 2020

Rollup of 8 pull requests #69958

Closed

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

matthewjasper force-pushed the metahygiene branch from 2f145f6 to 0a9a415 Compare March 15, 2020 22:14

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 16, 2020

Make macro metavars respect (non-)hygiene

ec86270

matthewjasper force-pushed the metahygiene branch from 0a9a415 to ec86270 Compare March 16, 2020 17:14

Centril mentioned this pull request Mar 17, 2020

Rollup of 8 pull requests #70066

Closed

Centril mentioned this pull request Mar 17, 2020

Rollup of 8 pull requests #70067

Closed

Centril mentioned this pull request Mar 17, 2020

Rollup of 7 pull requests #70072

Merged

bors merged commit 8cf9e9e into rust-lang:master Mar 17, 2020

matthewjasper deleted the metahygiene branch March 17, 2020 18:31

Uh oh!

Conversation

matthewjasper commented Feb 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthewjasper commented Feb 2, 2020

Uh oh!

bors commented Feb 2, 2020

Uh oh!

bors commented Feb 2, 2020

Uh oh!

matthewjasper commented Feb 2, 2020

Uh oh!

craterbot commented Feb 2, 2020

Uh oh!

craterbot commented Feb 2, 2020

Uh oh!

craterbot commented Feb 4, 2020

Uh oh!

matthewjasper commented Feb 5, 2020

Uh oh!

rust-timer commented Feb 5, 2020

Uh oh!

rust-timer commented Feb 5, 2020

Uh oh!

petrochenkov commented Feb 9, 2020

Uh oh!

petrochenkov commented Feb 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Feb 23, 2020

Uh oh!

petrochenkov commented Feb 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Mar 8, 2020

Uh oh!

petrochenkov commented Mar 8, 2020

Uh oh!

bors commented Mar 15, 2020

Uh oh!

matthewjasper commented Mar 16, 2020

Uh oh!

bors commented Mar 16, 2020

Uh oh!

matthewjasper commented Mar 16, 2020

Uh oh!

bors commented Mar 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

matthewjasper commented Feb 1, 2020 •

edited

Loading

petrochenkov commented Feb 16, 2020 •

edited

Loading

petrochenkov commented Feb 23, 2020 •

edited

Loading