-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Replace evaluated cfg_attr
in AST with a placeholder attribute for accurate span tracking
#133823
base: master
Are you sure you want to change the base?
Conversation
help: remove the unused `extern crate` | ||
| | ||
LL - extern crate core; | ||
LL + |
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 thought this was a rendering bug, but it is because of the //~
removed comment.
This comment has been minimized.
This comment has been minimized.
cfg_attr
in AST with a placeholder attribute for accurate span trackingcfg_attr
in AST with a placeholder attribute for accurate span tracking
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Are the placeholder attributes visible to proc macros? Something like #[cfg_eval]
#[my_proc_macro]
#[cfg_attr(all(), my_attr)]
struct S {} can be used for testing, the If it's possible to introduce placeholders like this (on best effort basis) only in AST, without making them observable in token streams in any way, then they should be able to solve some other problems as well, like removing expanded Upd: if these placeholders are visible to proc macros then we cannot add them, because they are internal details that should not be public and stable. |
With a test //@ proc-macro: cfg-placeholder.rs
#![feature(cfg_eval)]
#[macro_use] extern crate cfg_placeholder;
#[cfg_eval]
#[my_proc_macro]
#[cfg_attr(FALSE, my_attr1)]
#[cfg_attr(all(), my_attr2)]
struct S {} and a
I'm honestly surprised it doesn't seem like we need to do anything else to avoid polluting the TTS. I think it is because of the check for |
d2d1ac1
to
78b23f6
Compare
This comment has been minimized.
This comment has been minimized.
…pan tracking Previously, when evaluating a `#[cfg_attr(..)]` to false, the entire attribute was removed from the AST. Afterwards, we insert in its place a `#[rustc-cfg-placeholder]` attribute so that checks for attributes can still know about their placement. This is particularly relevant when we suggest removing items with `cfg_attr`s (fix rust-lang#56328). We use `rustc-cfg-placeholder` as it is an ident that can't be written by the end user to begin with. We tweak the wording of the existing "unused `extern crate`" lint. ``` warning: unused `extern crate` --> $DIR/removing-extern-crate.rs:9:1 | LL | extern crate removing_extern_crate as foo; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unused | note: the lint level is defined here --> $DIR/removing-extern-crate.rs:6:9 | LL | #![warn(rust_2018_idioms)] | ^^^^^^^^^^^^^^^^ = note: `#[warn(unused_extern_crates)]` implied by `#[warn(rust_2018_idioms)]` help: remove the unused `extern crate` | LL - #[cfg_attr(test, macro_use)] LL - extern crate removing_extern_crate as foo; LL + | ```
Avoid "duplicated attribute" lints firing on the cfg placeholder.
Ensure that the cfg-placeholder isn't visible by proc-macros.
78b23f6
to
5ebcd19
Compare
Previously, when evaluating a
#[cfg_attr(..)]
to false, the entire attribute was removed from the AST. Afterwards, we insert in its place a#[rustc-cfg-placeholder]
attribute so that checks for attributes can still know about their placement. This is particularly relevant when we suggest removing items withcfg_attr
s (fix #56328). We userustc-cfg-placeholder
as it is an ident that can't be written by the end user to begin with. We tweak the wording of the existing "unusedextern crate
" lint.r? @petrochenkov