Skip to content
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

expand: Leave traces when expanding cfg attributes #138844

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

petrochenkov
Copy link
Contributor

This is the same as #138515, but for cfg(true) instead of cfg_attr.

The difference is that cfg(true)s already left "traces" after themselves - the cfg attributes themselves, with expanded_inert_attrs set to true, with full tokens, available to proc macros.
This is not a reasonably expected behavior, but it could not be removed without a replacement, because a major rustdoc feature and a number of clippy lints rely on it. This PR implements a replacement.

This needs a crater run, because it changes observable behavior (in an intended way) - proc macros can no longer see expanded cfg(true) attributes.

(Some minor unnecessary special casing for sym::cfg_attr is also removed in this PR.)

r? @nnethercote

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 22, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 22, 2025

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

@petrochenkov
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Mar 22, 2025

⌛ Trying commit 750814d with merge 411f3ca...

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2025
expand: Leave traces when expanding `cfg` attributes

This is the same as rust-lang#138515, but for `cfg(true)` instead of `cfg_attr`.

The difference is that `cfg(true)`s already left "traces" after themselves - the `cfg` attributes themselves, with `expanded_inert_attrs` set to true, with full tokens, available to proc macros.
This is not a reasonably expected behavior, but it could not be removed without a replacement, because a [major rustdoc feature](rust-lang/rfcs#3631) and a number of clippy lints rely on it. This PR implements a replacement.

This needs a crater run, because it changes observable behavior (in an intended way) - proc macros can no longer see expanded `cfg(true)` attributes.

(Some minor unnecessary special casing for `sym::cfg_attr` is also removed in this PR.)

r? `@nnethercote`
@bors
Copy link
Contributor

bors commented Mar 23, 2025

☀️ Try build successful - checks-actions
Build commit: 411f3ca (411f3ca0d8086d5a27e2aaa79b9ff41f53ab2f51)

@petrochenkov
Copy link
Contributor Author

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-138844 created and queued.
🤖 Automatically detected try build 411f3ca
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 23, 2025
@craterbot
Copy link
Collaborator

🚧 Experiment pr-138844 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-138844 is completed!
📊 6 regressed and 7 fixed (602721 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Mar 24, 2025
Copy link
Member

Choose a reason for hiding this comment

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

Starting a thread on a random file: Just noticed those changes and had a question:

In Clippy we have this utils function:

/// Checks if the given span contains a `#[cfg(..)]` attribute
pub fn span_contains_cfg(cx: &LateContext<'_>, s: Span) -> bool {
s.check_source_text(cx, |src| {
let mut iter = tokenize_with_text(src);
// Search for the token sequence [`#`, `[`, `cfg`]
while iter.any(|(t, ..)| matches!(t, TokenKind::Pound)) {
let mut iter = iter.by_ref().skip_while(|(t, ..)| {
matches!(
t,
TokenKind::Whitespace | TokenKind::LineComment { .. } | TokenKind::BlockComment { .. }
)
});
if matches!(iter.next(), Some((TokenKind::OpenBracket, ..)))
&& matches!(iter.next(), Some((TokenKind::Ident, "cfg", _)))
{
return true;
}
}
false
})
}

This function is there to check after-the-fact if there was a cfg involved in the code that we're currently checking. For example when we're looking at a match:

match x {
    #[cfg(foo)]
    optionalArm => {},
    alwaysArm => {},
}

And then gating linting on it.

IIUC this can help us getting rid of this utils function? If so, I'd open a issue on Clippy for it. Definitely not for this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cfg(true) already stays in code after expansion, and some other logic in clippy checks for it, yet this function still exists.

Maybe it takes a span of some larger node and expects to see #[cfg(false)]s in it as well? In that case this PR won't help, configured out nodes still disappear without a trace.

@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 Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants