Skip to content

Commit b896b59

Browse files
JonathanBrouwerJonathanBrouwer
committed
Rewrite empty attribute lint
Signed-off-by: Jonathan Brouwer <[email protected]>
1 parent 1b61d43 commit b896b59

File tree

25 files changed

+192
-188
lines changed

25 files changed

+192
-188
lines changed

‎compiler/rustc_attr_data_structures/src/attributes.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ pub enum ReprAttr {
6767
ReprSimd,
6868
ReprTransparent,
6969
ReprAlign(Align),
70-
// this one is just so we can emit a lint for it
71-
ReprEmpty,
7270
}
7371
pub use ReprAttr::*;
7472

@@ -291,7 +289,7 @@ pub enum AttributeKind {
291289
PubTransparent(Span),
292290

293291
/// Represents [`#[repr]`](https://doc.rust-lang.org/stable/reference/type-layout.html#representations).
294-
Repr(ThinVec<(ReprAttr, Span)>),
292+
Repr { reprs: ThinVec<(ReprAttr, Span)>, first_span: Span },
295293

296294
/// Represents `#[rustc_layout_scalar_valid_range_end]`.
297295
RustcLayoutScalarValidRangeEnd(Box<u128>, Span),

‎compiler/rustc_attr_data_structures/src/encode_cross_crate.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ impl AttributeKind {
2626
Inline(..) => No,
2727
LinkSection { .. } => No,
2828
MacroTransparency(..) => Yes,
29-
Repr(..) => No,
29+
Repr { .. } => No,
3030
Stability { .. } => Yes,
3131
Cold(..) => No,
3232
ConstContinue(..) => No,

‎compiler/rustc_attr_data_structures/src/lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,5 @@ pub struct AttributeLint<Id> {
1212
pub enum AttributeLintKind {
1313
UnusedDuplicate { this: Span, other: Span, warning: bool },
1414
IllFormedAttributeInput { suggestions: Vec<String> },
15+
EmptyAttribute { first_span: Span },
1516
}

‎compiler/rustc_attr_parsing/messages.ftl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ attr_parsing_deprecated_item_suggestion =
66
.help = add `#![feature(deprecated_suggestion)]` to the crate root
77
.note = see #94785 for more details
88
9+
attr_parsing_empty_attribute =
10+
unused attribute
11+
.suggestion = remove this attribute
12+
913
attr_parsing_empty_confusables =
1014
expected at least one confusable name
1115
attr_parsing_expected_one_cfg_pattern =

‎compiler/rustc_attr_parsing/src/attributes/codegen_attrs.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,10 @@ impl<S: Stage> CombineAttributeParser<S> for TargetFeatureParser {
298298
cx.expected_list(cx.attr_span);
299299
return features;
300300
};
301+
if list.is_empty() {
302+
cx.warn_empty_attribute(cx.attr_span);
303+
return features;
304+
}
301305
for item in list.mixed() {
302306
let Some(name_value) = item.meta_item() else {
303307
cx.expected_name_value(item.span(), Some(sym::enable));

‎compiler/rustc_attr_parsing/src/attributes/repr.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ pub(crate) struct ReprParser;
2323
impl<S: Stage> CombineAttributeParser<S> for ReprParser {
2424
type Item = (ReprAttr, Span);
2525
const PATH: &[Symbol] = &[sym::repr];
26-
const CONVERT: ConvertFn<Self::Item> = |items, _| AttributeKind::Repr(items);
26+
const CONVERT: ConvertFn<Self::Item> =
27+
|items, first_span| AttributeKind::Repr { reprs: items, first_span };
2728
// FIXME(jdonszelmann): never used
2829
const TEMPLATE: AttributeTemplate =
2930
template!(List: "C | Rust | align(...) | packed(...) | <integer type> | transparent");
@@ -40,8 +41,8 @@ impl<S: Stage> CombineAttributeParser<S> for ReprParser {
4041
};
4142

4243
if list.is_empty() {
43-
// this is so validation can emit a lint
44-
reprs.push((ReprAttr::ReprEmpty, cx.attr_span));
44+
cx.warn_empty_attribute(cx.attr_span);
45+
return reprs;
4546
}
4647

4748
for param in list.mixed() {

‎compiler/rustc_attr_parsing/src/context.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ mod private {
160160
#[allow(private_interfaces)]
161161
pub trait Stage: Sized + 'static + Sealed {
162162
type Id: Copy;
163+
const SHOULD_EMIT_LINTS: bool;
163164

164165
fn parsers() -> &'static group_type!(Self);
165166

@@ -170,6 +171,7 @@ pub trait Stage: Sized + 'static + Sealed {
170171
#[allow(private_interfaces)]
171172
impl Stage for Early {
172173
type Id = NodeId;
174+
const SHOULD_EMIT_LINTS: bool = false;
173175

174176
fn parsers() -> &'static group_type!(Self) {
175177
&early::ATTRIBUTE_PARSERS
@@ -183,6 +185,7 @@ impl Stage for Early {
183185
#[allow(private_interfaces)]
184186
impl Stage for Late {
185187
type Id = HirId;
188+
const SHOULD_EMIT_LINTS: bool = true;
186189

187190
fn parsers() -> &'static group_type!(Self) {
188191
&late::ATTRIBUTE_PARSERS
@@ -223,6 +226,9 @@ impl<'f, 'sess: 'f, S: Stage> SharedContext<'f, 'sess, S> {
223226
/// must be delayed until after HIR is built. This method will take care of the details of
224227
/// that.
225228
pub(crate) fn emit_lint(&mut self, lint: AttributeLintKind, span: Span) {
229+
if !S::SHOULD_EMIT_LINTS {
230+
return;
231+
}
226232
let id = self.target_id;
227233
(self.emit_lint)(AttributeLint { id, span, kind: lint });
228234
}
@@ -404,6 +410,10 @@ impl<'f, 'sess: 'f, S: Stage> AcceptContext<'f, 'sess, S> {
404410
},
405411
})
406412
}
413+
414+
pub(crate) fn warn_empty_attribute(&mut self, span: Span) {
415+
self.emit_lint(AttributeLintKind::EmptyAttribute { first_span: span }, span);
416+
}
407417
}
408418

409419
impl<'f, 'sess, S: Stage> Deref for AcceptContext<'f, 'sess, S> {

‎compiler/rustc_attr_parsing/src/lints.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,11 @@ pub fn emit_attribute_lint<L: LintEmitter>(lint: &AttributeLint<HirId>, lint_emi
2828
},
2929
);
3030
}
31+
AttributeLintKind::EmptyAttribute { first_span } => lint_emitter.emit_node_span_lint(
32+
rustc_session::lint::builtin::UNUSED_ATTRIBUTES,
33+
*id,
34+
*first_span,
35+
session_diagnostics::EmptyAttributeList { attr_span: *first_span },
36+
),
3137
}
3238
}

‎compiler/rustc_attr_parsing/src/session_diagnostics.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,13 @@ pub(crate) struct EmptyConfusables {
473473
pub span: Span,
474474
}
475475

476+
#[derive(LintDiagnostic)]
477+
#[diag(attr_parsing_empty_attribute)]
478+
pub(crate) struct EmptyAttributeList {
479+
#[suggestion(code = "", applicability = "machine-applicable")]
480+
pub attr_span: Span,
481+
}
482+
476483
#[derive(Diagnostic)]
477484
#[diag(attr_parsing_invalid_alignment_value, code = E0589)]
478485
pub(crate) struct InvalidAlignmentValue {

‎compiler/rustc_builtin_macros/src/deriving/generic/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ impl<'a> TraitDef<'a> {
485485
Annotatable::Item(item) => {
486486
let is_packed = matches!(
487487
AttributeParser::parse_limited(cx.sess, &item.attrs, sym::repr, item.span, item.id),
488-
Some(Attribute::Parsed(AttributeKind::Repr(r))) if r.iter().any(|(x, _)| matches!(x, ReprPacked(..)))
488+
Some(Attribute::Parsed(AttributeKind::Repr { reprs, .. })) if reprs.iter().any(|(x, _)| matches!(x, ReprPacked(..)))
489489
);
490490

491491
let newitem = match &item.kind {

‎compiler/rustc_codegen_ssa/src/codegen_attrs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
109109

110110
if let hir::Attribute::Parsed(p) = attr {
111111
match p {
112-
AttributeKind::Repr(reprs) => {
112+
AttributeKind::Repr { reprs, first_span: _ } => {
113113
codegen_fn_attrs.alignment = reprs
114114
.iter()
115115
.filter_map(

‎compiler/rustc_hir_analysis/src/check/check.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1395,8 +1395,7 @@ fn check_simd(tcx: TyCtxt<'_>, sp: Span, def_id: LocalDefId) {
13951395
pub(super) fn check_packed(tcx: TyCtxt<'_>, sp: Span, def: ty::AdtDef<'_>) {
13961396
let repr = def.repr();
13971397
if repr.packed() {
1398-
if let Some(reprs) =
1399-
attrs::find_attr!(tcx.get_all_attrs(def.did()), attrs::AttributeKind::Repr(r) => r)
1398+
if let Some(reprs) = attrs::find_attr!(tcx.get_all_attrs(def.did()), attrs::AttributeKind::Repr { reprs, .. } => reprs)
14001399
{
14011400
for (r, _) in reprs {
14021401
if let ReprPacked(pack) = r
@@ -1619,10 +1618,10 @@ fn check_enum(tcx: TyCtxt<'_>, def_id: LocalDefId) {
16191618
if def.variants().is_empty() {
16201619
attrs::find_attr!(
16211620
tcx.get_all_attrs(def_id),
1622-
attrs::AttributeKind::Repr(rs) => {
1621+
attrs::AttributeKind::Repr { reprs, first_span } => {
16231622
struct_span_code_err!(
16241623
tcx.dcx(),
1625-
rs.first().unwrap().1,
1624+
reprs.first().map(|repr| repr.1).unwrap_or(*first_span),
16261625
E0084,
16271626
"unsupported representation for zero-variant enum"
16281627
)

‎compiler/rustc_lint/src/nonstandard_style.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ impl EarlyLintPass for NonCamelCaseTypes {
168168
fn check_item(&mut self, cx: &EarlyContext<'_>, it: &ast::Item) {
169169
let has_repr_c = matches!(
170170
AttributeParser::parse_limited(cx.sess(), &it.attrs, sym::repr, it.span, it.id),
171-
Some(Attribute::Parsed(AttributeKind::Repr(r))) if r.iter().any(|(r, _)| r == &ReprAttr::ReprC)
171+
Some(Attribute::Parsed(AttributeKind::Repr { reprs, ..})) if reprs.iter().any(|(r, _)| r == &ReprAttr::ReprC)
172172
);
173173

174174
if has_repr_c {

‎compiler/rustc_middle/src/ty/mod.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1521,7 +1521,8 @@ impl<'tcx> TyCtxt<'tcx> {
15211521
field_shuffle_seed ^= user_seed;
15221522
}
15231523

1524-
if let Some(reprs) = attr::find_attr!(self.get_all_attrs(did), AttributeKind::Repr(r) => r)
1524+
if let Some(reprs) =
1525+
attr::find_attr!(self.get_all_attrs(did), AttributeKind::Repr { reprs, .. } => reprs)
15251526
{
15261527
for (r, _) in reprs {
15271528
flags.insert(match *r {
@@ -1562,10 +1563,6 @@ impl<'tcx> TyCtxt<'tcx> {
15621563
max_align = max_align.max(Some(align));
15631564
ReprFlags::empty()
15641565
}
1565-
attr::ReprEmpty => {
1566-
/* skip these, they're just for diagnostics */
1567-
ReprFlags::empty()
1568-
}
15691566
});
15701567
}
15711568
}

‎compiler/rustc_passes/src/check_attr.rs

Lines changed: 32 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
160160
}
161161
Attribute::Parsed(AttributeKind::DocComment { .. }) => { /* `#[doc]` is actually a lot more than just doc comments, so is checked below*/
162162
}
163-
Attribute::Parsed(AttributeKind::Repr(_)) => { /* handled below this loop and elsewhere */
163+
Attribute::Parsed(AttributeKind::Repr { .. }) => { /* handled below this loop and elsewhere */
164164
}
165165
Attribute::Parsed(AttributeKind::RustcObjectLifetimeDefault) => {
166166
self.check_object_lifetime_default(hir_id);
@@ -1943,7 +1943,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
19431943
// #[repr(foo)]
19441944
// #[repr(bar, align(8))]
19451945
// ```
1946-
let reprs = find_attr!(attrs, AttributeKind::Repr(r) => r.as_slice()).unwrap_or(&[]);
1946+
let (reprs, first_attr_span) = find_attr!(attrs, AttributeKind::Repr { reprs, first_span } => (reprs.as_slice(), Some(*first_span))).unwrap_or((&[], None));
19471947

19481948
let mut int_reprs = 0;
19491949
let mut is_explicit_rust = false;
@@ -2040,31 +2040,30 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
20402040
continue;
20412041
}
20422042
}
2043-
// FIXME(jdonszelmann): move the diagnostic for unused repr attrs here, I think
2044-
// it's a better place for it.
2045-
ReprAttr::ReprEmpty => {
2046-
// catch `repr()` with no arguments, applied to an item (i.e. not `#![repr()]`)
2047-
if item.is_some() {
2048-
match target {
2049-
Target::Struct | Target::Union | Target::Enum => continue,
2050-
Target::Fn | Target::Method(_) => {
2051-
self.dcx().emit_err(errors::ReprAlignShouldBeAlign {
2052-
span: *repr_span,
2053-
item: target.name(),
2054-
});
2055-
}
2056-
_ => {
2057-
self.dcx().emit_err(errors::AttrApplication::StructEnumUnion {
2058-
hint_span: *repr_span,
2059-
span,
2060-
});
2061-
}
2062-
}
2063-
}
2043+
};
2044+
}
20642045

2065-
return;
2046+
// catch `repr()` with no arguments, applied to an item (i.e. not `#![repr()]`)
2047+
if let Some(first_attr_span) = first_attr_span
2048+
&& reprs.is_empty()
2049+
&& item.is_some()
2050+
{
2051+
match target {
2052+
Target::Struct | Target::Union | Target::Enum => {}
2053+
Target::Fn | Target::Method(_) => {
2054+
self.dcx().emit_err(errors::ReprAlignShouldBeAlign {
2055+
span: first_attr_span,
2056+
item: target.name(),
2057+
});
20662058
}
2067-
};
2059+
_ => {
2060+
self.dcx().emit_err(errors::AttrApplication::StructEnumUnion {
2061+
hint_span: first_attr_span,
2062+
span,
2063+
});
2064+
}
2065+
}
2066+
return;
20682067
}
20692068

20702069
// Just point at all repr hints if there are any incompatibilities.
@@ -2319,43 +2318,8 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
23192318
}
23202319

23212320
fn check_unused_attribute(&self, hir_id: HirId, attr: &Attribute, style: Option<AttrStyle>) {
2322-
// FIXME(jdonszelmann): deduplicate these checks after more attrs are parsed. This is very
2323-
// ugly now but can 100% be removed later.
2324-
if let Attribute::Parsed(p) = attr {
2325-
match p {
2326-
AttributeKind::Repr(reprs) => {
2327-
for (r, span) in reprs {
2328-
if let ReprAttr::ReprEmpty = r {
2329-
self.tcx.emit_node_span_lint(
2330-
UNUSED_ATTRIBUTES,
2331-
hir_id,
2332-
*span,
2333-
errors::Unused {
2334-
attr_span: *span,
2335-
note: errors::UnusedNote::EmptyList { name: sym::repr },
2336-
},
2337-
);
2338-
}
2339-
}
2340-
return;
2341-
}
2342-
AttributeKind::TargetFeature(features, span) if features.len() == 0 => {
2343-
self.tcx.emit_node_span_lint(
2344-
UNUSED_ATTRIBUTES,
2345-
hir_id,
2346-
*span,
2347-
errors::Unused {
2348-
attr_span: *span,
2349-
note: errors::UnusedNote::EmptyList { name: sym::target_feature },
2350-
},
2351-
);
2352-
return;
2353-
}
2354-
_ => {}
2355-
};
2356-
}
2357-
23582321
// Warn on useless empty attributes.
2322+
// FIXME(jdonszelmann): this lint should be moved to attribute parsing, see `AcceptContext::warn_empty_attribute`
23592323
let note = if attr.has_any_name(&[
23602324
sym::macro_use,
23612325
sym::allow,
@@ -2571,7 +2535,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
25712535
}
25722536

25732537
fn check_rustc_pub_transparent(&self, attr_span: Span, span: Span, attrs: &[Attribute]) {
2574-
if !find_attr!(attrs, AttributeKind::Repr(r) => r.iter().any(|(r, _)| r == &ReprAttr::ReprTransparent))
2538+
if !find_attr!(attrs, AttributeKind::Repr { reprs, .. } => reprs.iter().any(|(r, _)| r == &ReprAttr::ReprTransparent))
25752539
.unwrap_or(false)
25762540
{
25772541
self.dcx().emit_err(errors::RustcPubTransparent { span, attr_span });
@@ -2847,8 +2811,12 @@ fn check_invalid_crate_level_attr(tcx: TyCtxt<'_>, attrs: &[Attribute]) {
28472811
ATTRS_TO_CHECK.iter().find(|attr_to_check| attr.has_name(**attr_to_check))
28482812
{
28492813
(attr.span(), *a)
2850-
} else if let Attribute::Parsed(AttributeKind::Repr(r)) = attr {
2851-
(r.first().unwrap().1, sym::repr)
2814+
} else if let Attribute::Parsed(AttributeKind::Repr {
2815+
reprs: _,
2816+
first_span: first_attr_span,
2817+
}) = attr
2818+
{
2819+
(*first_attr_span, sym::repr)
28522820
} else {
28532821
continue;
28542822
};

‎src/librustdoc/clean/types.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -782,7 +782,7 @@ impl Item {
782782
// don't want it it `Item::attrs`.
783783
hir::Attribute::Parsed(AttributeKind::Deprecation { .. }) => None,
784784
// We have separate pretty-printing logic for `#[repr(..)]` attributes.
785-
hir::Attribute::Parsed(AttributeKind::Repr(..)) => None,
785+
hir::Attribute::Parsed(AttributeKind::Repr { .. }) => None,
786786
// target_feature is special-cased because cargo-semver-checks uses it
787787
hir::Attribute::Parsed(AttributeKind::TargetFeature(features, _)) => {
788788
let mut output = String::new();

‎src/tools/clippy/clippy_lints/src/arbitrary_source_item_ordering.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering {
266266
.tcx
267267
.hir_attrs(item.hir_id())
268268
.iter()
269-
.any(|attr| matches!(attr, Attribute::Parsed(AttributeKind::Repr(..))))
269+
.any(|attr| matches!(attr, Attribute::Parsed(AttributeKind::Repr{ .. })))
270270
{
271271
// Do not lint items with a `#[repr]` attribute as their layout may be imposed by an external API.
272272
return;

‎src/tools/clippy/clippy_lints/src/attrs/repr_attributes.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use clippy_utils::msrvs::{self, Msrv};
99
use super::REPR_PACKED_WITHOUT_ABI;
1010

1111
pub(super) fn check(cx: &LateContext<'_>, item_span: Span, attrs: &[Attribute], msrv: Msrv) {
12-
if let Some(reprs) = find_attr!(attrs, AttributeKind::Repr(r) => r) {
12+
if let Some(reprs) = find_attr!(attrs, AttributeKind::Repr { reprs, .. } => reprs) {
1313
let packed_span = reprs
1414
.iter()
1515
.find(|(r, _)| matches!(r, ReprAttr::ReprPacked(..)))

‎src/tools/clippy/clippy_lints/src/default_union_representation.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,5 +99,5 @@ fn is_zst<'tcx>(cx: &LateContext<'tcx>, field: &FieldDef, args: ty::GenericArgsR
9999
fn has_c_repr_attr(cx: &LateContext<'_>, hir_id: HirId) -> bool {
100100
let attrs = cx.tcx.hir_attrs(hir_id);
101101

102-
find_attr!(attrs, AttributeKind::Repr(r) if r.iter().any(|(x, _)| *x == ReprAttr::ReprC))
102+
find_attr!(attrs, AttributeKind::Repr { reprs, .. } if reprs.iter().any(|(x, _)| *x == ReprAttr::ReprC))
103103
}

0 commit comments

Comments
 (0)