Skip to content

Conversation

@folkertdev
Copy link
Contributor

@folkertdev folkertdev commented Dec 12, 2025

tracking issue: #115585

Emit parse errors that are found in the branches that are not selected, like e.g. how #[cfg(false)] { 1 ++ 2 } still emits a parse error even though the expression is never used by the program. Parsing the unused branches was requested by T-lang #149783 (comment).

@folkertdev folkertdev added the F-cfg_select `#![feature(cfg_select)]` label Dec 12, 2025
@rustbot rustbot added 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. labels Dec 12, 2025
@traviscross traviscross added the I-lang-radar Items that are on lang's radar and will need eventual work or consideration. label Dec 12, 2025
@rust-log-analyzer

This comment has been minimized.

@folkertdev folkertdev force-pushed the cfg-select-parse-unused-branches branch from e00ad06 to a118b81 Compare December 12, 2025 19:07
Comment on lines 24 to 35
fn tts_to_mac_result<'cx, 'sess>(
ecx: &'cx mut ExtCtxt<'sess>,
site_span: Span,
tts: TokenStream,
span: Span,
) -> Box<dyn MacResult + 'cx> {
match ExpandResult::from_tts(ecx, tts, site_span, span, Ident::with_dummy_span(sym::cfg_select))
{
ExpandResult::Ready(x) => x,
_ => unreachable!("from_tts always returns Ready"),
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This works around ParserAnyMacro not being exported from rustc_expand. Maybe it should be exported though?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm trying to understand how much prettier that'd become. I think it'd be fine to export it from rustc_expand, but I wonder if it changes much. Would you not need the macro calls below anymore? I think you still do right? Because then I don't think this is so bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You'd still need the macros. The ParserAnyMacro variant also has an additional argument, which is nicely hidden now. So yeah I think this is the best we can do right now.

Copy link
Contributor

Choose a reason for hiding this comment

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

okay, that sounds reasonable!

@rust-log-analyzer

This comment has been minimized.

@folkertdev folkertdev force-pushed the cfg-select-parse-unused-branches branch 2 times, most recently from 0496cba to e91cae2 Compare December 13, 2025 13:57
Comment on lines 51 to 63
impl<'cx, 'sess> MacResult for CfgSelectResult<'cx, 'sess> {
forward_to_parser_any_macro!(make_expr, Box<Expr>);
forward_to_parser_any_macro!(make_stmts, SmallVec<[ast::Stmt; 1]>);
forward_to_parser_any_macro!(make_items, SmallVec<[Box<ast::Item>; 1]>);

branches.wildcard.map(|(_, tt, span)| (tt, span))
forward_to_parser_any_macro!(make_impl_items, SmallVec<[Box<ast::AssocItem>; 1]>);
forward_to_parser_any_macro!(make_trait_impl_items, SmallVec<[Box<ast::AssocItem>; 1]>);
forward_to_parser_any_macro!(make_trait_items, SmallVec<[Box<ast::AssocItem>; 1]>);
forward_to_parser_any_macro!(make_foreign_items, SmallVec<[Box<ast::ForeignItem>; 1]>);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've started a discussion at #t-lang > where can `cfg_select!` go to decide what we want to support.

expr/stmt/item is the bare minimum I think, but we already had some use of (i believe) impl items in the stdlib test suite, so including the other item flavors seems natural. We can see about e.g. patterns or where-clauses etc.

@folkertdev folkertdev marked this pull request as ready for review December 13, 2025 17:59
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 13, 2025

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Dec 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 13, 2025

r? @chenyukang

rustbot has assigned @chenyukang.
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

@jdonszelmann
Copy link
Contributor

jdonszelmann commented Dec 13, 2025

r? jdonszelmann

(Reviewed similar PRS recently, don't mind taking a look :)

@rustbot rustbot assigned jdonszelmann and unassigned chenyukang Dec 13, 2025
@jdonszelmann
Copy link
Contributor

@rustbot author

@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 Dec 15, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 15, 2025

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

@folkertdev folkertdev force-pushed the cfg-select-parse-unused-branches branch from e91cae2 to d5ca05a Compare December 15, 2025 10:38
@folkertdev
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot 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 Dec 15, 2025
@jdonszelmann
Copy link
Contributor

@rustbot author
Let me know on the question about exporting from rustc_expand. If it becomes a lot prettier here I've nothing against making it pub. But otherwise r=me

@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 Dec 17, 2025
@folkertdev
Copy link
Contributor Author

@bors r=jdonszelmann

@bors
Copy link
Collaborator

bors commented Dec 17, 2025

📌 Commit d5ca05a has been approved by jdonszelmann

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 17, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 17, 2025
…-branches, r=jdonszelmann

`cfg_select!`: parse unused branches

tracking issue: rust-lang#115585

Emit parse errors that are found in the branches that are not selected, like e.g. how `#[cfg(false)] { 1 ++ 2 }` still emits a parse error even though the expression is never used by the program. Parsing the unused branches was requested by T-lang rust-lang#149783 (comment).
@matthiaskrgr
Copy link
Member

@bors r-
#150104 (comment)

@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 Dec 17, 2025
@matthiaskrgr
Copy link
Member

maybe this conflicts with #148937 ? 🤔

@folkertdev
Copy link
Contributor Author

Looks like we should just also allow expansion in type position. I'll add that.

@folkertdev folkertdev force-pushed the cfg-select-parse-unused-branches branch from d5ca05a to 12d05e7 Compare December 17, 2025 20:06
@rustbot
Copy link
Collaborator

rustbot commented Dec 17, 2025

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.

@folkertdev folkertdev force-pushed the cfg-select-parse-unused-branches branch from 12d05e7 to 82ee8b2 Compare December 17, 2025 21:42
@folkertdev
Copy link
Contributor Author

After some discussion with @joshtriplett I added support for types and patterns. I haven't been able to trigger the remainder of the MacResult methods from user code, I always run into parse errors before expansion even runs.

@rustbot ready

@rustbot rustbot 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 Dec 17, 2025
@jdonszelmann
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 18, 2025

📌 Commit 82ee8b2 has been approved by jdonszelmann

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Dec 18, 2025

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

@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 Dec 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) F-cfg_select `#![feature(cfg_select)]` I-lang-radar Items that are on lang's radar and will need eventual work or consideration. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

8 participants