Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 4ff5353

Browse files
estebankestebank
committedDec 3, 2024
Replace cfg_attr in AST with rustc-cfg-placeholder for accurate span 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 #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 + | ```
1 parent 5e1440a commit 4ff5353

34 files changed

+210
-76
lines changed
 

‎compiler/rustc_ast/src/ast.rs

+4
Original file line numberDiff line numberDiff line change
@@ -3006,6 +3006,10 @@ impl AttrItem {
30063006
|| self.path == sym::allow
30073007
|| self.path == sym::deny
30083008
}
3009+
3010+
pub fn is_cfg_placeholder(&self) -> bool {
3011+
self.path == sym::rustc_cfg_placeholder
3012+
}
30093013
}
30103014

30113015
/// `TraitRef`s appear in impls.

‎compiler/rustc_ast/src/attr/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@ impl Attribute {
226226

227227
pub fn token_trees(&self) -> Vec<TokenTree> {
228228
match self.kind {
229+
AttrKind::Normal(ref normal) if normal.item.is_cfg_placeholder() => vec![],
229230
AttrKind::Normal(ref normal) => normal
230231
.tokens
231232
.as_ref()

‎compiler/rustc_ast_passes/src/ast_validation.rs

+4
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,10 @@ impl<'a> AstValidator<'a> {
340340
sym::deny,
341341
sym::expect,
342342
sym::forbid,
343+
// `cfg` and `cfg_attr` get replaced with an inert `rustc_cfg_placeholder` to
344+
// keep the attribute "spot" in the AST. This allows suggestions to remove an
345+
// item to provide a correct suggestion when `#[cfg_attr]`s are present.
346+
sym::rustc_cfg_placeholder,
343347
sym::warn,
344348
];
345349
!arr.contains(&attr.name_or_empty()) && rustc_attr::is_builtin_attr(attr)

‎compiler/rustc_expand/src/config.rs

+19-1
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,25 @@ impl<'a> StripUnconfigured<'a> {
296296
}
297297

298298
if !attr::cfg_matches(&cfg_predicate, &self.sess, self.lint_node_id, self.features) {
299-
return vec![];
299+
// `cfg` and `cfg_attr` gets replaced with an inert `rustc_cfg_placeholder` to keep the
300+
// attribute "spot" in the AST. This allows suggestions to remove an item to provide a
301+
// correct suggestion when `#[cfg_attr]`s are present.
302+
let item = ast::AttrItem {
303+
unsafety: ast::Safety::Default,
304+
path: ast::Path::from_ident(rustc_span::symbol::Ident::with_dummy_span(
305+
sym::rustc_cfg_placeholder,
306+
)),
307+
args: ast::AttrArgs::Empty,
308+
tokens: None,
309+
};
310+
let kind = ast::AttrKind::Normal(P(ast::NormalAttr { item, tokens: None }));
311+
let attr: ast::Attribute = ast::Attribute {
312+
kind,
313+
id: self.sess.psess.attr_id_generator.mk_attr_id(),
314+
style: ast::AttrStyle::Outer,
315+
span: cfg_attr.span,
316+
};
317+
return vec![attr];
300318
}
301319

302320
if recursive {

‎compiler/rustc_feature/src/builtin_attrs.rs

+6
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,12 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
750750
template!(Word, List: r#""...""#), DuplicatesOk,
751751
EncodeCrossCrate::No, INTERNAL_UNSTABLE
752752
),
753+
// placeholder that replaces `cfg_attr` in the item's attribute list
754+
ungated!(
755+
rustc_cfg_placeholder, Normal, template!(Word /* irrelevant */), DuplicatesOk,
756+
EncodeCrossCrate::No
757+
),
758+
753759

754760
// ==========================================================================
755761
// Internal attributes, Diagnostics related:

‎compiler/rustc_lint/messages.ftl

+3-2
Original file line numberDiff line numberDiff line change
@@ -927,8 +927,9 @@ lint_unused_doc_comment = unused doc comment
927927
.label = rustdoc does not generate documentation for macro invocations
928928
.help = to document an item produced by a macro, the macro must produce the documentation as part of its expansion
929929
930-
lint_unused_extern_crate = unused extern crate
931-
.suggestion = remove it
930+
lint_unused_extern_crate = unused `extern crate`
931+
.label = unused
932+
.suggestion = remove the unused `extern crate`
932933
933934
lint_unused_import_braces = braces around {$node} is unnecessary
934935

‎compiler/rustc_lint/src/context/diagnostics.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,8 @@ pub(super) fn decorate_lint(sess: &Session, diagnostic: BuiltinLintDiag, diag: &
288288
BuiltinLintDiag::ByteSliceInPackedStructWithDerive { ty } => {
289289
lints::ByteSliceInPackedStructWithDerive { ty }.decorate_lint(diag);
290290
}
291-
BuiltinLintDiag::UnusedExternCrate { removal_span } => {
292-
lints::UnusedExternCrate { removal_span }.decorate_lint(diag);
291+
BuiltinLintDiag::UnusedExternCrate { span, removal_span } => {
292+
lints::UnusedExternCrate { span, removal_span }.decorate_lint(diag);
293293
}
294294
BuiltinLintDiag::ExternCrateNotIdiomatic { vis_span, ident_span } => {
295295
let suggestion_span = vis_span.between(ident_span);

‎compiler/rustc_lint/src/lints.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -2907,7 +2907,9 @@ pub(crate) struct ByteSliceInPackedStructWithDerive {
29072907
#[derive(LintDiagnostic)]
29082908
#[diag(lint_unused_extern_crate)]
29092909
pub(crate) struct UnusedExternCrate {
2910-
#[suggestion(code = "", applicability = "machine-applicable")]
2910+
#[label]
2911+
pub span: Span,
2912+
#[suggestion(code = "", applicability = "machine-applicable", style = "verbose")]
29112913
pub removal_span: Span,
29122914
}
29132915

‎compiler/rustc_lint_defs/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -706,6 +706,7 @@ pub enum BuiltinLintDiag {
706706
ty: String,
707707
},
708708
UnusedExternCrate {
709+
span: Span,
709710
removal_span: Span,
710711
},
711712
ExternCrateNotIdiomatic {

‎compiler/rustc_passes/src/check_attr.rs

+1
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
273273
| sym::prelude_import
274274
| sym::panic_handler
275275
| sym::allow_internal_unsafe
276+
| sym::rustc_cfg_placeholder // Inert, it's a placeholder in the AST.
276277
| sym::fundamental
277278
| sym::lang
278279
| sym::needs_allocator

‎compiler/rustc_resolve/src/check_unused.rs

+3
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ impl<'a, 'ra, 'tcx> UnusedImportCheckVisitor<'a, 'ra, 'tcx> {
155155
extern_crate.id,
156156
span,
157157
BuiltinLintDiag::UnusedExternCrate {
158+
span: extern_crate.span,
158159
removal_span: extern_crate.span_with_attributes,
159160
},
160161
);
@@ -221,6 +222,7 @@ impl<'a, 'ra, 'tcx> Visitor<'a> for UnusedImportCheckVisitor<'a, 'ra, 'tcx> {
221222
// compiler and we don't need to consider them.
222223
ast::ItemKind::Use(..) if item.span.is_dummy() => return,
223224
ast::ItemKind::ExternCrate(orig_name) => {
225+
tracing::info!(?item);
224226
self.extern_crate_items.push(ExternCrateToLint {
225227
id: item.id,
226228
span: item.span,
@@ -397,6 +399,7 @@ impl Resolver<'_, '_> {
397399
}
398400
}
399401
ImportKind::ExternCrate { id, .. } => {
402+
tracing::info!("import={import:#?}");
400403
let def_id = self.local_def_id(id);
401404
if self.extern_crate_map.get(&def_id).map_or(true, |&cnum| {
402405
!tcx.is_compiler_builtins(cnum)

‎compiler/rustc_span/src/symbol.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1667,6 +1667,8 @@ symbols! {
16671667
rustc_box,
16681668
rustc_builtin_macro,
16691669
rustc_capture_analysis,
1670+
// We ensure that the attribute can't be written by end users by adding `-` to the name.
1671+
rustc_cfg_placeholder: "rustc-cfg-placeholder",
16701672
rustc_clean,
16711673
rustc_coherence_is_core,
16721674
rustc_coinductive,

‎tests/ui/editions/edition-extern-crate-allowed.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@
55
#![warn(rust_2018_idioms)]
66

77
extern crate edition_extern_crate_allowed;
8-
//~^ WARNING unused extern crate
8+
//~^ WARNING unused `extern crate`
99

1010
fn main() {}

‎tests/ui/editions/edition-extern-crate-allowed.stderr

+6-2
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
1-
warning: unused extern crate
1+
warning: unused `extern crate`
22
--> $DIR/edition-extern-crate-allowed.rs:7:1
33
|
44
LL | extern crate edition_extern_crate_allowed;
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unused
66
|
77
note: the lint level is defined here
88
--> $DIR/edition-extern-crate-allowed.rs:5:9
99
|
1010
LL | #![warn(rust_2018_idioms)]
1111
| ^^^^^^^^^^^^^^^^
1212
= note: `#[warn(unused_extern_crates)]` implied by `#[warn(rust_2018_idioms)]`
13+
help: remove the unused `extern crate`
14+
|
15+
LL - extern crate edition_extern_crate_allowed;
16+
|
1317

1418
warning: 1 warning emitted
1519

‎tests/ui/imports/extern-crate-used.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ extern crate core as iso3;
1515
extern crate core as iso4;
1616

1717
// Doesn't introduce its extern prelude entry, so it's still considered unused.
18-
extern crate core; //~ ERROR unused extern crate
18+
extern crate core; //~ ERROR unused `extern crate`
1919

2020
mod m {
2121
use iso1::any as are_you_okay1;

‎tests/ui/imports/extern-crate-used.stderr

+7-2
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
1-
error: unused extern crate
1+
error: unused `extern crate`
22
--> $DIR/extern-crate-used.rs:18:1
33
|
44
LL | extern crate core;
5-
| ^^^^^^^^^^^^^^^^^^ help: remove it
5+
| ^^^^^^^^^^^^^^^^^^ unused
66
|
77
note: the lint level is defined here
88
--> $DIR/extern-crate-used.rs:6:9
99
|
1010
LL | #![deny(unused_extern_crates)]
1111
| ^^^^^^^^^^^^^^^^^^^^
12+
help: remove the unused `extern crate`
13+
|
14+
LL - extern crate core;
15+
LL +
16+
|
1217

1318
error: aborting due to 1 previous error
1419

‎tests/ui/lint/unnecessary-extern-crate.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
#![feature(test)]
55

66
extern crate core;
7-
//~^ ERROR unused extern crate
7+
//~^ ERROR unused `extern crate`
88
//~| HELP remove
99
extern crate core as x;
10-
//~^ ERROR unused extern crate
10+
//~^ ERROR unused `extern crate`
1111
//~| HELP remove
1212

1313
extern crate proc_macro;
@@ -29,11 +29,11 @@ mod foo {
2929
pub(super) extern crate alloc as d;
3030

3131
extern crate core;
32-
//~^ ERROR unused extern crate
32+
//~^ ERROR unused `extern crate`
3333
//~| HELP remove
3434

3535
extern crate core as x;
36-
//~^ ERROR unused extern crate
36+
//~^ ERROR unused `extern crate`
3737
//~| HELP remove
3838

3939
pub extern crate test;
@@ -42,11 +42,11 @@ mod foo {
4242

4343
mod bar {
4444
extern crate core;
45-
//~^ ERROR unused extern crate
45+
//~^ ERROR unused `extern crate`
4646
//~| HELP remove
4747

4848
extern crate core as x;
49-
//~^ ERROR unused extern crate
49+
//~^ ERROR unused `extern crate`
5050
//~| HELP remove
5151

5252
pub(in crate::foo::bar) extern crate alloc as e;

‎tests/ui/lint/unnecessary-extern-crate.stderr

+41-12
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,73 @@
1-
error: unused extern crate
1+
error: unused `extern crate`
22
--> $DIR/unnecessary-extern-crate.rs:6:1
33
|
44
LL | extern crate core;
5-
| ^^^^^^^^^^^^^^^^^^ help: remove it
5+
| ^^^^^^^^^^^^^^^^^^ unused
66
|
77
note: the lint level is defined here
88
--> $DIR/unnecessary-extern-crate.rs:3:9
99
|
1010
LL | #![deny(unused_extern_crates)]
1111
| ^^^^^^^^^^^^^^^^^^^^
12+
help: remove the unused `extern crate`
13+
|
14+
LL - extern crate core;
15+
|
1216

13-
error: unused extern crate
17+
error: unused `extern crate`
1418
--> $DIR/unnecessary-extern-crate.rs:9:1
1519
|
1620
LL | extern crate core as x;
17-
| ^^^^^^^^^^^^^^^^^^^^^^^ help: remove it
21+
| ^^^^^^^^^^^^^^^^^^^^^^^ unused
22+
|
23+
help: remove the unused `extern crate`
24+
|
25+
LL - extern crate core as x;
26+
|
1827

19-
error: unused extern crate
28+
error: unused `extern crate`
2029
--> $DIR/unnecessary-extern-crate.rs:31:5
2130
|
2231
LL | extern crate core;
23-
| ^^^^^^^^^^^^^^^^^^ help: remove it
32+
| ^^^^^^^^^^^^^^^^^^ unused
33+
|
34+
help: remove the unused `extern crate`
35+
|
36+
LL - extern crate core;
37+
|
2438

25-
error: unused extern crate
39+
error: unused `extern crate`
2640
--> $DIR/unnecessary-extern-crate.rs:35:5
2741
|
2842
LL | extern crate core as x;
29-
| ^^^^^^^^^^^^^^^^^^^^^^^ help: remove it
43+
| ^^^^^^^^^^^^^^^^^^^^^^^ unused
44+
|
45+
help: remove the unused `extern crate`
46+
|
47+
LL - extern crate core as x;
48+
|
3049

31-
error: unused extern crate
50+
error: unused `extern crate`
3251
--> $DIR/unnecessary-extern-crate.rs:44:9
3352
|
3453
LL | extern crate core;
35-
| ^^^^^^^^^^^^^^^^^^ help: remove it
54+
| ^^^^^^^^^^^^^^^^^^ unused
55+
|
56+
help: remove the unused `extern crate`
57+
|
58+
LL - extern crate core;
59+
|
3660

37-
error: unused extern crate
61+
error: unused `extern crate`
3862
--> $DIR/unnecessary-extern-crate.rs:48:9
3963
|
4064
LL | extern crate core as x;
41-
| ^^^^^^^^^^^^^^^^^^^^^^^ help: remove it
65+
| ^^^^^^^^^^^^^^^^^^^^^^^ unused
66+
|
67+
help: remove the unused `extern crate`
68+
|
69+
LL - extern crate core as x;
70+
|
4271

4372
error: aborting due to 6 previous errors
4473

‎tests/ui/lint/unused/lint-unused-extern-crate.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
#![allow(unused_variables)]
99
#![allow(deprecated)]
1010

11-
extern crate lint_unused_extern_crate5; //~ ERROR: unused extern crate
11+
extern crate lint_unused_extern_crate5; //~ ERROR: unused `extern crate`
1212

1313
pub extern crate lint_unused_extern_crate4; // no error, it is re-exported
1414

@@ -26,7 +26,7 @@ use other::*;
2626

2727
mod foo {
2828
// Test that this is unused even though an earlier `extern crate` is used.
29-
extern crate lint_unused_extern_crate2; //~ ERROR unused extern crate
29+
extern crate lint_unused_extern_crate2; //~ ERROR unused `extern crate`
3030
}
3131

3232
fn main() {

‎tests/ui/lint/unused/lint-unused-extern-crate.stderr

+15-4
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,31 @@
1-
error: unused extern crate
1+
error: unused `extern crate`
22
--> $DIR/lint-unused-extern-crate.rs:11:1
33
|
44
LL | extern crate lint_unused_extern_crate5;
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unused
66
|
77
note: the lint level is defined here
88
--> $DIR/lint-unused-extern-crate.rs:7:9
99
|
1010
LL | #![deny(unused_extern_crates)]
1111
| ^^^^^^^^^^^^^^^^^^^^
12+
help: remove the unused `extern crate`
13+
|
14+
LL - extern crate lint_unused_extern_crate5;
15+
LL +
16+
|
1217

13-
error: unused extern crate
18+
error: unused `extern crate`
1419
--> $DIR/lint-unused-extern-crate.rs:29:5
1520
|
1621
LL | extern crate lint_unused_extern_crate2;
17-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it
22+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unused
23+
|
24+
help: remove the unused `extern crate`
25+
|
26+
LL - extern crate lint_unused_extern_crate2;
27+
LL +
28+
|
1829

1930
error: aborting due to 2 previous errors
2031

There was a problem loading the remainder of the diff.

0 commit comments

Comments
 (0)
Failed to load comments.