Skip to content

Make cfg_match! a semitransparent macro #138993

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
Merged
merged 1 commit into from
Apr 9, 2025
Merged

Make cfg_match! a semitransparent macro #138993

merged 1 commit into from
Apr 9, 2025

Conversation

CAD97
Copy link
Contributor

@CAD97 CAD97 commented Mar 26, 2025 •
edited
Loading

IIUC this is preferred when (potentially) stabilizing macro items, to avoid potentially utilizing def-site hygiene instead of mixed-site.

Tracking issue: #115585

@rustbot
Copy link
Collaborator

rustbot commented Mar 26, 2025

r? @workingjubilee

rustbot has assigned @workingjubilee.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 26, 2025
@workingjubilee
Copy link
Member

@CAD97 I'm confused, what's wrong with def-site hygiene?

@CAD97
Copy link
Contributor Author

CAD97 commented Mar 27, 2025

Nothing wrong, per say, just that nothing has yet exposed "macros 2.0" hygiene existing to stable. Even pin!, which was originally formulated to rely on def-site access to a private inner field, has been changed to use semitransparent hygiene.

@joshtriplett
Copy link
Member

We discussed this in today's libs-api meeting. The rationale of not exposing "macros 2.0" hygiene differences made sense, and if we understood correctly, this should be equivalent to if the macro was written using macro_rules! instead. We also expected that if it were exposed any differently, that'd need to be a lang decision to expose any "macros 2.0" functionality, and we didn't expect that there'd be any desire to do so. So, generally speaking 👍 from libs-api.

That said, several of us were interested in understanding this better. Could someone post an example of, in the absence of this PR, what would be possible using cfg_match! that would expose the "macros 2.0" hygiene difference?

@CAD97
Copy link
Contributor Author

CAD97 commented Apr 8, 2025

I believe there's no functional difference in this case as cfg_match! is currently implemented, since the only names used in the expansion are cfg_match! and #[cfg]. The former is made fully hygienic via the $crate:: path, and #[cfg] can't be shadowed.

Thus, using the semitransparent macro_rules! hygiene is just being conservative in this case; the already stable usable hygiene is sufficient, so it's better to use it instead of the macros 2.0 hygiene.

dtolnay
dtolnay approved these changes Apr 8, 2025
View reviewed changes
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks!

@dtolnay
Copy link
Member

dtolnay commented Apr 8, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 8, 2025

📌 Commit 2c70c8a has been approved by dtolnay

It is now in the queue for this repository.

@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 Apr 8, 2025
@dtolnay dtolnay assigned dtolnay and unassigned workingjubilee Apr 8, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 9, 2025
@matthiaskrgr
Rollup merge of rust-lang#138993 - CAD97:cfg_match_semitransparent, r…
136955c 
…=dtolnay

Make `cfg_match!` a semitransparent macro

IIUC this is preferred when (potentially) stabilizing `macro` items, to avoid potentially utilizing def-site hygiene instead of mixed-site.

Tracking issue: rust-lang#115585
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 9, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 9, 2025
@bors
Auto merge of rust-lang#139571 - matthiaskrgr:rollup-3qdo3ud, r=matth…
d572e8b 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#137412 (Ensure `swap_nonoverlapping` is really always untyped)
 - rust-lang#138869 (Try not to use verbatim paths in `Command::current_dir`)
 - rust-lang#138993 (Make `cfg_match!` a semitransparent macro)
 - rust-lang#139099 (Promise `array::from_fn` is generated in order of increasing indices)
 - rust-lang#139364 (Make the compiler suggest actual paths instead of visible paths if the visible paths are through any doc hidden path.)
 - rust-lang#139468 (Don't call `Span::with_parent` on the good path in `has_stashed_diagnostic`)
 - rust-lang#139481 (Add job summary links to post-merge report)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 9, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 9, 2025
@bors
Auto merge of rust-lang#139581 - matthiaskrgr:rollup-d6hph16, r=matth…
7f34062 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138869 (Try not to use verbatim paths in `Command::current_dir`)
 - rust-lang#138993 (Make `cfg_match!` a semitransparent macro)
 - rust-lang#139099 (Promise `array::from_fn` is generated in order of increasing indices)
 - rust-lang#139364 (Make the compiler suggest actual paths instead of visible paths if the visible paths are through any doc hidden path.)
 - rust-lang#139468 (Don't call `Span::with_parent` on the good path in `has_stashed_diagnostic`)
 - rust-lang#139481 (Add job summary links to post-merge report)
 - rust-lang#139573 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 9, 2025
@bors
Auto merge of rust-lang#139581 - matthiaskrgr:rollup-d6hph16, r=matth…
48f89e7 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138869 (Try not to use verbatim paths in `Command::current_dir`)
 - rust-lang#138993 (Make `cfg_match!` a semitransparent macro)
 - rust-lang#139099 (Promise `array::from_fn` is generated in order of increasing indices)
 - rust-lang#139364 (Make the compiler suggest actual paths instead of visible paths if the visible paths are through any doc hidden path.)
 - rust-lang#139468 (Don't call `Span::with_parent` on the good path in `has_stashed_diagnostic`)
 - rust-lang#139481 (Add job summary links to post-merge report)
 - rust-lang#139573 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 19d4d9f into rust-lang:master Apr 9, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 9, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 9, 2025
@rust-timer
Unrolled build for rust-lang#138993
8d5accb 
Rollup merge of rust-lang#138993 - CAD97:cfg_match_semitransparent, r=dtolnay

Make `cfg_match!` a semitransparent macro

IIUC this is preferred when (potentially) stabilizing `macro` items, to avoid potentially utilizing def-site hygiene instead of mixed-site.

Tracking issue: rust-lang#115585
@CAD97 CAD97 deleted the cfg_match_semitransparent branch April 10, 2025 02:43
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Apr 19, 2025
@matthiaskrgr
Rollup merge of rust-lang#138993 - CAD97:cfg_match_semitransparent, r…
a3b67d3 
…=dtolnay

Make `cfg_match!` a semitransparent macro

IIUC this is preferred when (potentially) stabilizing `macro` items, to avoid potentially utilizing def-site hygiene instead of mixed-site.

Tracking issue: rust-lang#115585
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtolnay dtolnay dtolnay approved these changes

Assignees

@dtolnay dtolnay

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.88.0
Development

Successfully merging this pull request may close these issues.
6 participants
@CAD97 @rustbot @workingjubilee @joshtriplett @dtolnay @bors