-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
cfg_select!: parse unused branches
#149925
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
cfg_select!: parse unused branches
#149925
Conversation
This comment has been minimized.
This comment has been minimized.
e00ad06 to
a118b81
Compare
| 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"), | ||
| } | ||
| } |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, that sounds reasonable!
This comment has been minimized.
This comment has been minimized.
0496cba to
e91cae2
Compare
| 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]>); | ||
| } |
There was a problem hiding this comment.
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.
|
Some changes occurred in compiler/rustc_attr_parsing |
|
r? @chenyukang rustbot has assigned @chenyukang. Use |
|
r? jdonszelmann (Reviewed similar PRS recently, don't mind taking a look :) |
|
@rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
e91cae2 to
d5ca05a
Compare
|
@rustbot ready |
…-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).
|
maybe this conflicts with #148937 ? 🤔 |
|
Looks like we should just also allow expansion in type position. I'll add that. |
d5ca05a to
12d05e7
Compare
|
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. |
12d05e7 to
82ee8b2
Compare
|
After some discussion with @joshtriplett I added support for types and patterns. I haven't been able to trigger the remainder of the @rustbot ready |
|
@bors r+ |
|
🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened. |
…-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).
…uwer Rollup of 11 pull requests Successful merges: - #145933 (Expand `str_as_str` to more types) - #148849 (Set -Cpanic=abort in windows-msvc stack protector tests) - #149925 (`cfg_select!`: parse unused branches) - #150022 (Generate macro expansion for rust compiler crates docs) - #150024 (Support recursive delegation) - #150048 (std_detect: AArch64 Darwin: expose SME F16F16 and B16B16 features) - #150083 (tests/run-make-cargo/same-crate-name-and-macro-name: New regression test) - #150102 (Fixed ICE for EII with multiple defaults due to duplicate definition in nameres) - #150124 (unstable.rs: fix typos in comments (implementatble -> implementable)) - #150125 (Port `#[rustc_lint_opt_deny_field_access]` to attribute parser) - #150126 (Subtree sync for rustc_codegen_cranelift) Failed merges: - #150127 (Port `#[rustc_lint_untracked_query_information]` and `#[rustc_lint_diagnostics]` to using attribute parsers) r? `@ghost` `@rustbot` modify labels: rollup
…uwer Rollup of 12 pull requests Successful merges: - #145933 (Expand `str_as_str` to more types) - #148849 (Set -Cpanic=abort in windows-msvc stack protector tests) - #149925 (`cfg_select!`: parse unused branches) - #149952 (Suggest struct pattern when destructuring Range with .. syntax) - #150022 (Generate macro expansion for rust compiler crates docs) - #150024 (Support recursive delegation) - #150048 (std_detect: AArch64 Darwin: expose SME F16F16 and B16B16 features) - #150083 (tests/run-make-cargo/same-crate-name-and-macro-name: New regression test) - #150102 (Fixed ICE for EII with multiple defaults due to duplicate definition in nameres) - #150124 (unstable.rs: fix typos in comments (implementatble -> implementable)) - #150125 (Port `#[rustc_lint_opt_deny_field_access]` to attribute parser) - #150126 (Subtree sync for rustc_codegen_cranelift) Failed merges: - #150127 (Port `#[rustc_lint_untracked_query_information]` and `#[rustc_lint_diagnostics]` to using attribute parsers) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #149925 - folkertdev:cfg-select-parse-unused-branches, r=jdonszelmann `cfg_select!`: parse unused branches 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).
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).