Skip to content

Track items behind cfg_select in the same way we do for cfg - #157218

Open
estebank wants to merge 1 commit into
rust-lang:mainfrom
estebank:cfg_select_tracking
Open

Track items behind cfg_select in the same way we do for cfg#157218
estebank wants to merge 1 commit into
rust-lang:mainfrom
estebank:cfg_select_tracking

Conversation

@estebank

@estebank estebank commented May 31, 2026

Copy link
Copy Markdown
Contributor

View all comments

When annotating an item with #[cfg] we track both the item that got annotated (with an inert attr) and the names of items that got directly cfg'd out. Extend this mechanism to also work for items within a cfg_select!.

r? @jdonszelmann
cc @JonathanBrouwer

@rustbot

rustbot commented May 31, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 31, 2026
@rustbot

rustbot commented May 31, 2026

Copy link
Copy Markdown
Collaborator

jdonszelmann is currently at their maximum review capacity.
They may take a while to respond.

@estebank

Copy link
Copy Markdown
Contributor Author

This or something akin to this is needed for #129249. I'll clean up the code shortly, once I get that PR to the point it can run on crater.

@rustbot

rustbot commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

The Cranelift subtree was changed

cc @bjorn3

@estebank
estebank force-pushed the cfg_select_tracking branch from 2262d01 to 97da0c8 Compare June 1, 2026 00:59
@petrochenkov petrochenkov self-assigned this Jun 1, 2026
@petrochenkov

Copy link
Copy Markdown
Contributor

(I'd like to look at this, probably tomorrow, because the PR touches expansion infra.)

Comment thread compiler/rustc_expand/src/expand.rs
@estebank
estebank force-pushed the cfg_select_tracking branch from 97da0c8 to 677634c Compare June 2, 2026 00:23
@rustbot

This comment has been minimized.

Comment thread compiler/rustc_expand/src/expand.rs
Comment thread compiler/rustc_builtin_macros/src/cfg_select.rs Outdated
Comment thread compiler/rustc_builtin_macros/src/cfg_select.rs Outdated
Comment thread compiler/rustc_builtin_macros/src/cfg_select.rs Outdated
@petrochenkov 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 Jun 3, 2026
@rust-log-analyzer

This comment has been minimized.

@estebank
estebank force-pushed the cfg_select_tracking branch from 76ed1df to 34c1685 Compare June 3, 2026 18:22
@estebank estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 3, 2026
@estebank
estebank force-pushed the cfg_select_tracking branch from 34c1685 to a04d12d Compare June 3, 2026 21:46
@petrochenkov petrochenkov removed their assignment Jun 4, 2026
@estebank

Copy link
Copy Markdown
Contributor Author

Gentle ping :)

@estebank
estebank force-pushed the cfg_select_tracking branch from a04d12d to eea0055 Compare June 19, 2026 22:54
@rustbot

This comment has been minimized.

@estebank
estebank force-pushed the cfg_select_tracking branch from eea0055 to 89a11d2 Compare June 19, 2026 22:55
@JonathanBrouwer

Copy link
Copy Markdown
Member

r? me
I can take a look at this this weekend

| | ^^^^^^^^^^^^
... |
LL | | _ => {}
| |_________- the item is gated here

@JonathanBrouwer JonathanBrouwer Aug 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmmm, I find the "the item is gated here" a bit wide, how hard would it be to make it point only to the cfg predicate?

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Bonus point if we can point to the reason_span that is returned when attr::eval_config_entry returns EvalConfigResult::False, I worked hard to make that information available :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@JonathanBrouwer I think we'll need a larger change to make use of reason_span here. I noticed there are quite a few places where we immediately call .as_bool(), which completely discards that information and that might be a good place to look for further improvements.

@JonathanBrouwer JonathanBrouwer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Other than that, this looks good :)
Sorry that this took so long to get reviewed, this is awesome work!

View changes since this review

@rustbot rustbot 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 Aug 20, 2026
@rustbot

rustbot commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rust-bors

This comment has been minimized.

When annotating an item with `#[cfg]` we track both the item that got annotated (with an inert attr) and the names of items that got directly cfg'd out. Extend this mechanism to also work for items within a `cfg_select!`.
@estebank
estebank force-pushed the cfg_select_tracking branch from 89a11d2 to ce74313 Compare August 26, 2026 20:09
@rustbot

rustbot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants