-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
base: master
Are you sure you want to change the base?
Conversation
Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in compiler/rustc_passes/src/check_attr.rs |
@bors try |
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`
☀️ Try build successful - checks-actions |
@craterbot check |
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🎉 Experiment
|
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.
Starting a thread on a random file: Just noticed those changes and had a question:
In Clippy we have this utils function:
rust/src/tools/clippy/clippy_utils/src/attrs.rs
Lines 177 to 198 in b95aac6
/// 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.
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.
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.
This is the same as #138515, but for
cfg(true)
instead ofcfg_attr
.The difference is that
cfg(true)
s already left "traces" after themselves - thecfg
attributes themselves, withexpanded_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