Reject outer attributes on `cfg_select` branches by qaijuang · Pull Request #155734 · rust-lang/rust

qaijuang · 2026-04-24T12:44:12Z

View all comments

Fixes #155701.

Co-authored-by: Copilot <copilot@github.com>

rustbot · 2026-04-24T14:42:13Z

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

rustbot · 2026-04-24T14:42:16Z

r? @JonathanBrouwer

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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 73 candidates
Random selection from 17 candidates

rustbot · 2026-04-25T10:44:36Z

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

JonathanBrouwer · 2026-04-25T12:30:34Z

@bors r+ rollup

rust-bors · 2026-04-25T12:30:37Z

📌 Commit 4bb2b07 has been approved by JonathanBrouwer

It is now in the queue for this repository.

…=JonathanBrouwer Lint doc comments in cfg_select branches Fixes rust-lang#155701.

…uwer Rollup of 6 pull requests Successful merges: - #154803 (Fix ICE from cfg_attr_trace ) - #155485 (Add an edge-case test for `--remap-path-prefix` for `rustc` & `rustdoc`) - #155659 (cleanup, restructure and merge `tests/ui/deriving` into `tests/ui/derives`) - #155696 (Add a higher-level API for parsing attributes) - #155734 (Lint doc comments in cfg_select branches) - #155769 (triagebot.toml: Ping Enselic when tests/debuginfo/basic-stepping.rs changes)

fmease · 2026-04-25T16:09:11Z

This PR allows more code to compile, so it's a language change and technically needs a T-lang FCP.

cc @rust-lang/lang

fmease · 2026-04-25T16:15:32Z

I'm sorry

@bors r-

traviscross · 2026-04-25T18:45:57Z

This PR allows more code to compile, so it's a language change and technically needs a T-lang FCP... The grammar of cfg_select is literally written down in the Reference, so any extensions are new and separate feature requests.

Thanks @fmease for catching this and navigating it.

rustbot · 2026-04-25T23:42:17Z

The parser was modified, potentially altering the grammar of (stable) Rust
which would be a breaking change.

cc @fmease

Co-authored-by: Copilot <copilot@github.com>

fmease · 2026-04-26T15:10:45Z

+        // `parse_outer_attributes` recovers inner doc comments as outer ones, so collect their
+        // spans first and suppress the follow-up `cfg_select` branch diagnostic for them.
+        let mut inner_doc_comment_spans = Vec::new();
+        let mut snapshot = self.create_snapshot_for_diagnostic();


IINM this is creating a parser snapshot in the happy path, right? this is an expensive operation and should only happen in error paths.

View changes since the review

Why is this using a snapshot in the first place? Can't you just use parse_inner_attributes and reject inner attrs afterwards? That function is smart enough not to bail out when it encounters outer attributes so you don't need to worry about that.

calling parse_inner_attributes first for /// outer followed by //! inner would return empty at the /// and would never notice the later //!

The test case:

cfg_select! { /// outer doc comment //! inner doc comment debug_assertions => {} _ => {} }

IINM this is creating a parser snapshot in the happy path, right? this is an expensive operation and should only happen in error paths.

That’s a good point. I’m thinking of classifying recovered doc comments directly from their source snippet in the error path, so we can avoid the snapshot cost

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 Apr 24, 2026

Lint doc comments in cfg_select branches

9c49112

qaijuang force-pushed the cfg-select-doc-comment branch from fb8cf2e to 9c49112 Compare April 24, 2026 14:12

folkertdev reviewed Apr 24, 2026

View reviewed changes

Comment thread tests/ui/lint/unused/unused-doc-comments-for-macros.rs Outdated

test with multi-line doc comment

4bb2b07

Co-authored-by: Copilot <copilot@github.com>

qaijuang marked this pull request as ready for review April 24, 2026 14:42

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 Apr 24, 2026

rustbot assigned JonathanBrouwer Apr 24, 2026

qaijuang requested a review from folkertdev April 24, 2026 16:59

JonathanBrouwer reviewed Apr 25, 2026

View reviewed changes

Comment thread compiler/rustc_attr_parsing/src/attributes/cfg_select.rs Outdated

JonathanBrouwer requested changes Apr 25, 2026

View reviewed changes

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 Apr 25, 2026

rust-bors Bot 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 Apr 25, 2026

JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Apr 25, 2026

Rollup merge of rust-lang#155734 - qaijuang:cfg-select-doc-comment, r…

054e0eb

…=JonathanBrouwer Lint doc comments in cfg_select branches Fixes rust-lang#155701.

JonathanBrouwer mentioned this pull request Apr 25, 2026

Rollup of 6 pull requests #155773

Closed

rust-bors Bot 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 Apr 25, 2026

Reject outer attributes on cfg_select branches

f0d49fa

qaijuang changed the title ~~Lint doc comments in cfg_select branches~~ Reject outer attributes on cfg_select branches Apr 25, 2026

qaijuang requested review from JonathanBrouwer and fmease April 25, 2026 23:46

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 Apr 25, 2026

qaijuang requested a review from folkertdev April 25, 2026 23:46

JonathanBrouwer requested changes Apr 26, 2026

View reviewed changes

Comment thread tests/ui/macros/cfg_select.stderr Outdated

Comment thread tests/ui/macros/cfg_select.rs

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 Apr 26, 2026

address nits and gotchas

96cb64a

Co-authored-by: Copilot <copilot@github.com>

qaijuang requested a review from JonathanBrouwer April 26, 2026 13:08

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 Apr 26, 2026

JonathanBrouwer requested changes Apr 26, 2026

View reviewed changes

Comment thread tests/ui/macros/cfg_select.stderr

Comment thread compiler/rustc_parse/src/parser/cfg_select.rs Outdated

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 Apr 26, 2026

one more test

40dcc5d

Co-authored-by: Copilot <copilot@github.com>

qaijuang requested a review from JonathanBrouwer April 26, 2026 14:52

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 Apr 26, 2026

fmease reviewed Apr 26, 2026

View reviewed changes

avoid snapshotting and classify recovered doc comments directly

c04993f

qaijuang requested a review from fmease April 26, 2026 16:07

Uh oh!

Conversation

qaijuang commented Apr 24, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

rustbot commented Apr 24, 2026

Uh oh!

rustbot commented Apr 24, 2026

Uh oh!

Uh oh!

rustbot commented Apr 25, 2026

Uh oh!

JonathanBrouwer commented Apr 25, 2026

Uh oh!

rust-bors Bot commented Apr 25, 2026

Uh oh!

fmease commented Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fmease commented Apr 25, 2026

Uh oh!

traviscross commented Apr 25, 2026

Uh oh!

rustbot commented Apr 25, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fmease Apr 26, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

qaijuang Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

qaijuang Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

qaijuang commented Apr 24, 2026 •

edited by rustbot

Loading

fmease commented Apr 25, 2026 •

edited

Loading

fmease Apr 26, 2026 •

edited by rustbot

Loading