Skip to content

Commit 017fe2f

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #143252 - JonathanBrouwer:rewrite_empty_attribute, r=jdonszelmann
Rewrite empty attribute lint for new attribute parser cc `@jdonszelmann`
2 parents c83e217 + 33f2cc7 commit 017fe2f

File tree

29 files changed

+201
-199
lines changed

29 files changed

+201
-199
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

@@ -304,7 +302,7 @@ pub enum AttributeKind {
304302
PubTransparent(Span),
305303

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

309307
/// Represents `#[rustc_layout_scalar_valid_range_end]`.
310308
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
@@ -41,7 +41,7 @@ impl AttributeKind {
4141
Optimize(..) => No,
4242
PassByValue(..) => Yes,
4343
PubTransparent(..) => Yes,
44-
Repr(..) => No,
44+
Repr { .. } => No,
4545
RustcLayoutScalarValidRangeEnd(..) => Yes,
4646
RustcLayoutScalarValidRangeStart(..) => Yes,
4747
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
@@ -165,6 +165,7 @@ mod private {
165165
#[allow(private_interfaces)]
166166
pub trait Stage: Sized + 'static + Sealed {
167167
type Id: Copy;
168+
const SHOULD_EMIT_LINTS: bool;
168169

169170
fn parsers() -> &'static group_type!(Self);
170171

@@ -175,6 +176,7 @@ pub trait Stage: Sized + 'static + Sealed {
175176
#[allow(private_interfaces)]
176177
impl Stage for Early {
177178
type Id = NodeId;
179+
const SHOULD_EMIT_LINTS: bool = false;
178180

179181
fn parsers() -> &'static group_type!(Self) {
180182
&early::ATTRIBUTE_PARSERS
@@ -188,6 +190,7 @@ impl Stage for Early {
188190
#[allow(private_interfaces)]
189191
impl Stage for Late {
190192
type Id = HirId;
193+
const SHOULD_EMIT_LINTS: bool = true;
191194

192195
fn parsers() -> &'static group_type!(Self) {
193196
&late::ATTRIBUTE_PARSERS
@@ -228,6 +231,9 @@ impl<'f, 'sess: 'f, S: Stage> SharedContext<'f, 'sess, S> {
228231
/// must be delayed until after HIR is built. This method will take care of the details of
229232
/// that.
230233
pub(crate) fn emit_lint(&mut self, lint: AttributeLintKind, span: Span) {
234+
if !S::SHOULD_EMIT_LINTS {
235+
return;
236+
}
231237
let id = self.target_id;
232238
(self.emit_lint)(AttributeLint { id, span, kind: lint });
233239
}
@@ -409,6 +415,10 @@ impl<'f, 'sess: 'f, S: Stage> AcceptContext<'f, 'sess, S> {
409415
},
410416
})
411417
}
418+
419+
pub(crate) fn warn_empty_attribute(&mut self, span: Span) {
420+
self.emit_lint(AttributeLintKind::EmptyAttribute { first_span: span }, span);
421+
}
412422
}
413423

414424
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_macros/src/print_attribute.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ fn print_fields(name: &Ident, fields: &Fields) -> (TokenStream, TokenStream, Tok
2121
__p.word_space(",");
2222
}
2323
__p.word(#string_name);
24-
__p.word_space(":");
24+
__p.word(":");
25+
__p.nbsp();
2526
__printed_anything = true;
2627
}
2728
#name.print_attribute(__p);

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

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

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

20752074
// Just point at all repr hints if there are any incompatibilities.
@@ -2324,43 +2323,8 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
23242323
}
23252324

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

25782542
fn check_rustc_pub_transparent(&self, attr_span: Span, span: Span, attrs: &[Attribute]) {
2579-
if !find_attr!(attrs, AttributeKind::Repr(r) => r.iter().any(|(r, _)| r == &ReprAttr::ReprTransparent))
2543+
if !find_attr!(attrs, AttributeKind::Repr { reprs, .. } => reprs.iter().any(|(r, _)| r == &ReprAttr::ReprTransparent))
25802544
.unwrap_or(false)
25812545
{
25822546
self.dcx().emit_err(errors::RustcPubTransparent { span, attr_span });
@@ -2852,8 +2816,12 @@ fn check_invalid_crate_level_attr(tcx: TyCtxt<'_>, attrs: &[Attribute]) {
28522816
ATTRS_TO_CHECK.iter().find(|attr_to_check| attr.has_name(**attr_to_check))
28532817
{
28542818
(attr.span(), *a)
2855-
} else if let Attribute::Parsed(AttributeKind::Repr(r)) = attr {
2856-
(r.first().unwrap().1, sym::repr)
2819+
} else if let Attribute::Parsed(AttributeKind::Repr {
2820+
reprs: _,
2821+
first_span: first_attr_span,
2822+
}) = attr
2823+
{
2824+
(*first_attr_span, sym::repr)
28572825
} else {
28582826
continue;
28592827
};

‎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(..)))

0 commit comments

Comments
 (0)