Skip to content

Commit fd443fc

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

File tree

27 files changed

+194
-190
lines changed

27 files changed

+194
-190
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

@@ -297,7 +295,7 @@ pub enum AttributeKind {
297295
PubTransparent(Span),
298296

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

302300
/// Represents `#[rustc_layout_scalar_valid_range_end]`.
303301
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
@@ -40,7 +40,7 @@ impl AttributeKind {
4040
Optimize(..) => No,
4141
PassByValue(..) => Yes,
4242
PubTransparent(..) => Yes,
43-
Repr(..) => No,
43+
Repr { .. } => No,
4444
RustcLayoutScalarValidRangeEnd(..) => Yes,
4545
RustcLayoutScalarValidRangeStart(..) => Yes,
4646
RustcObjectLifetimeDefault => 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
@@ -163,6 +163,7 @@ mod private {
163163
#[allow(private_interfaces)]
164164
pub trait Stage: Sized + 'static + Sealed {
165165
type Id: Copy;
166+
const SHOULD_EMIT_LINTS: bool;
166167

167168
fn parsers() -> &'static group_type!(Self);
168169

@@ -173,6 +174,7 @@ pub trait Stage: Sized + 'static + Sealed {
173174
#[allow(private_interfaces)]
174175
impl Stage for Early {
175176
type Id = NodeId;
177+
const SHOULD_EMIT_LINTS: bool = false;
176178

177179
fn parsers() -> &'static group_type!(Self) {
178180
&early::ATTRIBUTE_PARSERS
@@ -186,6 +188,7 @@ impl Stage for Early {
186188
#[allow(private_interfaces)]
187189
impl Stage for Late {
188190
type Id = HirId;
191+
const SHOULD_EMIT_LINTS: bool = true;
189192

190193
fn parsers() -> &'static group_type!(Self) {
191194
&late::ATTRIBUTE_PARSERS
@@ -226,6 +229,9 @@ impl<'f, 'sess: 'f, S: Stage> SharedContext<'f, 'sess, S> {
226229
/// must be delayed until after HIR is built. This method will take care of the details of
227230
/// that.
228231
pub(crate) fn emit_lint(&mut self, lint: AttributeLintKind, span: Span) {
232+
if !S::SHOULD_EMIT_LINTS {
233+
return;
234+
}
229235
let id = self.target_id;
230236
(self.emit_lint)(AttributeLint { id, span, kind: lint });
231237
}
@@ -407,6 +413,10 @@ impl<'f, 'sess: 'f, S: Stage> AcceptContext<'f, 'sess, S> {
407413
},
408414
})
409415
}
416+
417+
pub(crate) fn warn_empty_attribute(&mut self, span: Span) {
418+
self.emit_lint(AttributeLintKind::EmptyAttribute { first_span: span }, span);
419+
}
410420
}
411421

412422
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
@@ -1525,7 +1525,8 @@ impl<'tcx> TyCtxt<'tcx> {
15251525
field_shuffle_seed ^= user_seed;
15261526
}
15271527

1528-
if let Some(reprs) = attr::find_attr!(self.get_all_attrs(did), AttributeKind::Repr(r) => r)
1528+
if let Some(reprs) =
1529+
attr::find_attr!(self.get_all_attrs(did), AttributeKind::Repr { reprs, .. } => reprs)
15291530
{
15301531
for (r, _) in reprs {
15311532
flags.insert(match *r {
@@ -1566,10 +1567,6 @@ impl<'tcx> TyCtxt<'tcx> {
15661567
max_align = max_align.max(Some(align));
15671568
ReprFlags::empty()
15681569
}
1569-
attr::ReprEmpty => {
1570-
/* skip these, they're just for diagnostics */
1571-
ReprFlags::empty()
1572-
}
15731570
});
15741571
}
15751572
}

‎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);
@@ -1945,7 +1945,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
19451945
// #[repr(foo)]
19461946
// #[repr(bar, align(8))]
19471947
// ```
1948-
let reprs = find_attr!(attrs, AttributeKind::Repr(r) => r.as_slice()).unwrap_or(&[]);
1948+
let (reprs, first_attr_span) = find_attr!(attrs, AttributeKind::Repr { reprs, first_span } => (reprs.as_slice(), Some(*first_span))).unwrap_or((&[], None));
19491949

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

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

20722071
// Just point at all repr hints if there are any incompatibilities.
@@ -2321,43 +2320,8 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
23212320
}
23222321

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

25752539
fn check_rustc_pub_transparent(&self, attr_span: Span, span: Span, attrs: &[Attribute]) {
2576-
if !find_attr!(attrs, AttributeKind::Repr(r) => r.iter().any(|(r, _)| r == &ReprAttr::ReprTransparent))
2540+
if !find_attr!(attrs, AttributeKind::Repr { reprs, .. } => reprs.iter().any(|(r, _)| r == &ReprAttr::ReprTransparent))
25772541
.unwrap_or(false)
25782542
{
25792543
self.dcx().emit_err(errors::RustcPubTransparent { span, attr_span });
@@ -2849,8 +2813,12 @@ fn check_invalid_crate_level_attr(tcx: TyCtxt<'_>, attrs: &[Attribute]) {
28492813
ATTRS_TO_CHECK.iter().find(|attr_to_check| attr.has_name(**attr_to_check))
28502814
{
28512815
(attr.span(), *a)
2852-
} else if let Attribute::Parsed(AttributeKind::Repr(r)) = attr {
2853-
(r.first().unwrap().1, sym::repr)
2816+
} else if let Attribute::Parsed(AttributeKind::Repr {
2817+
reprs: _,
2818+
first_span: first_attr_span,
2819+
}) = attr
2820+
{
2821+
(*first_attr_span, sym::repr)
28542822
} else {
28552823
continue;
28562824
};

‎src/librustdoc/clean/types.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -784,7 +784,7 @@ impl Item {
784784
// don't want it it `Item::attrs`.
785785
hir::Attribute::Parsed(AttributeKind::Deprecation { .. }) => None,
786786
// We have separate pretty-printing logic for `#[repr(..)]` attributes.
787-
hir::Attribute::Parsed(AttributeKind::Repr(..)) => None,
787+
hir::Attribute::Parsed(AttributeKind::Repr { .. }) => None,
788788
// target_feature is special-cased because cargo-semver-checks uses it
789789
hir::Attribute::Parsed(AttributeKind::TargetFeature(features, _)) => {
790790
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)