Skip to content

Rewrite empty attribute lint for new attribute parser #143252

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions compiler/rustc_attr_data_structures/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ pub enum ReprAttr {
ReprSimd,
ReprTransparent,
ReprAlign(Align),
// this one is just so we can emit a lint for it
ReprEmpty,
}
pub use ReprAttr::*;

Expand Down Expand Up @@ -201,7 +199,7 @@ pub enum AttributeKind {
AllowConstFnUnstable(ThinVec<Symbol>),

/// Represents `#[allow_internal_unstable]`.
AllowInternalUnstable(ThinVec<(Symbol, Span)>),
AllowInternalUnstable(ThinVec<(Symbol, Span)>, Span),

/// Represents `#[rustc_as_ptr]` (used by the `dangling_pointers_from_temporaries` lint).
AsPtr(Span),
Expand Down Expand Up @@ -285,7 +283,7 @@ pub enum AttributeKind {
PubTransparent(Span),

/// Represents [`#[repr]`](https://doc.rust-lang.org/stable/reference/type-layout.html#representations).
Repr(ThinVec<(ReprAttr, Span)>),
Repr(ThinVec<(ReprAttr, Span)>, Span),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you call this field "first_span"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need this field at all if the warning always happens in thiscrate (attr_parsing)


/// Represents `#[rustc_skip_during_method_dispatch]`.
SkipDuringMethodDispatch { array: bool, boxed_slice: bool, span: Span },
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_attr_data_structures/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ pub struct AttributeLint<Id> {
pub enum AttributeLintKind {
UnusedDuplicate { this: Span, other: Span, warning: bool },
IllFormedAttributeInput { suggestions: Vec<String> },
EmptyAttribute { span: Span },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

}
4 changes: 4 additions & 0 deletions compiler/rustc_attr_parsing/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ attr_parsing_deprecated_item_suggestion =
.help = add `#![feature(deprecated_suggestion)]` to the crate root
.note = see #94785 for more details

attr_parsing_empty_attribute =
unused attribute
.suggestion = remove this attribute

attr_parsing_empty_confusables =
expected at least one confusable name
attr_parsing_expected_one_cfg_pattern =
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_attr_parsing/src/attributes/allow_unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ pub(crate) struct AllowInternalUnstableParser;
impl<S: Stage> CombineAttributeParser<S> for AllowInternalUnstableParser {
const PATH: &[Symbol] = &[sym::allow_internal_unstable];
type Item = (Symbol, Span);
const CONVERT: ConvertFn<Self::Item> = AttributeKind::AllowInternalUnstable;
const CONVERT: ConvertFn<Self::Item> =
ConvertFn::WithFirstAttributeSpan(AttributeKind::AllowInternalUnstable);
const TEMPLATE: AttributeTemplate = template!(Word, List: "feat1, feat2, ...");

fn extend<'c>(
Expand All @@ -30,7 +31,7 @@ pub(crate) struct AllowConstFnUnstableParser;
impl<S: Stage> CombineAttributeParser<S> for AllowConstFnUnstableParser {
const PATH: &[Symbol] = &[sym::rustc_allow_const_fn_unstable];
type Item = Symbol;
const CONVERT: ConvertFn<Self::Item> = AttributeKind::AllowConstFnUnstable;
const CONVERT: ConvertFn<Self::Item> = ConvertFn::Simple(AttributeKind::AllowConstFnUnstable);
const TEMPLATE: AttributeTemplate = template!(Word, List: "feat1, feat2, ...");

fn extend<'c>(
Expand Down
25 changes: 21 additions & 4 deletions compiler/rustc_attr_parsing/src/attributes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,10 @@ pub(crate) enum AttributeOrder {
KeepLast,
}

type ConvertFn<E> = fn(ThinVec<E>) -> AttributeKind;
pub(crate) enum ConvertFn<E> {
Simple(fn(ThinVec<E>) -> AttributeKind),
WithFirstAttributeSpan(fn(ThinVec<E>, Span) -> AttributeKind),
}

/// Alternative to [`AttributeParser`] that automatically handles state management.
/// If multiple attributes appear on an element, combines the values of each into a
Expand Down Expand Up @@ -262,22 +265,36 @@ pub(crate) trait CombineAttributeParser<S: Stage>: 'static {
pub(crate) struct Combine<T: CombineAttributeParser<S>, S: Stage>(
PhantomData<(S, T)>,
ThinVec<<T as CombineAttributeParser<S>>::Item>,
Option<Span>,
);

impl<T: CombineAttributeParser<S>, S: Stage> Default for Combine<T, S> {
fn default() -> Self {
Self(Default::default(), Default::default())
Self(Default::default(), Default::default(), Default::default())
}
}

impl<T: CombineAttributeParser<S>, S: Stage> AttributeParser<S> for Combine<T, S> {
const ATTRIBUTES: AcceptMapping<Self, S> = &[(
T::PATH,
<T as CombineAttributeParser<S>>::TEMPLATE,
|group: &mut Combine<T, S>, cx, args| group.1.extend(T::extend(cx, args)),
|group: &mut Combine<T, S>, cx, args| {
// Keep track of the span of the first attribute, for diagnostics
if group.2.is_none() {
group.2 = Some(cx.attr_span);
}
group.1.extend(T::extend(cx, args))
},
)];

fn finalize(self, _cx: &FinalizeContext<'_, '_, S>) -> Option<AttributeKind> {
if self.1.is_empty() { None } else { Some(T::CONVERT(self.1)) }
if let Some(first_span) = self.2 {
Some(match T::CONVERT {
ConvertFn::Simple(f) => f(self.1),
ConvertFn::WithFirstAttributeSpan(f) => f(self.1, first_span),
})
} else {
None
}
}
}
6 changes: 3 additions & 3 deletions compiler/rustc_attr_parsing/src/attributes/repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub(crate) struct ReprParser;
impl<S: Stage> CombineAttributeParser<S> for ReprParser {
type Item = (ReprAttr, Span);
const PATH: &[Symbol] = &[sym::repr];
const CONVERT: ConvertFn<Self::Item> = AttributeKind::Repr;
const CONVERT: ConvertFn<Self::Item> = ConvertFn::WithFirstAttributeSpan(AttributeKind::Repr);
// FIXME(jdonszelmann): never used
const TEMPLATE: AttributeTemplate =
template!(List: "C | Rust | align(...) | packed(...) | <integer type> | transparent");
Expand All @@ -40,8 +40,8 @@ impl<S: Stage> CombineAttributeParser<S> for ReprParser {
};

if list.is_empty() {
// this is so validation can emit a lint
reprs.push((ReprAttr::ReprEmpty, cx.attr_span));
cx.warn_empty_attribute(cx.attr_span);
return reprs;
}

for param in list.mixed() {
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_attr_parsing/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ mod private {
#[allow(private_interfaces)]
pub trait Stage: Sized + 'static + Sealed {
type Id: Copy;
const SHOULD_EMIT_LINTS: bool;

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

Expand All @@ -157,6 +158,7 @@ pub trait Stage: Sized + 'static + Sealed {
#[allow(private_interfaces)]
impl Stage for Early {
type Id = NodeId;
const SHOULD_EMIT_LINTS: bool = false;

fn parsers() -> &'static group_type!(Self) {
&early::ATTRIBUTE_PARSERS
Expand All @@ -170,6 +172,7 @@ impl Stage for Early {
#[allow(private_interfaces)]
impl Stage for Late {
type Id = HirId;
const SHOULD_EMIT_LINTS: bool = true;

fn parsers() -> &'static group_type!(Self) {
&late::ATTRIBUTE_PARSERS
Expand Down Expand Up @@ -210,6 +213,9 @@ impl<'f, 'sess: 'f, S: Stage> SharedContext<'f, 'sess, S> {
/// must be delayed until after HIR is built. This method will take care of the details of
/// that.
pub(crate) fn emit_lint(&mut self, lint: AttributeLintKind, span: Span) {
if !S::SHOULD_EMIT_LINTS {
return;
}
let id = self.target_id;
(self.emit_lint)(AttributeLint { id, span, kind: lint });
}
Expand Down Expand Up @@ -381,6 +387,10 @@ impl<'f, 'sess: 'f, S: Stage> AcceptContext<'f, 'sess, S> {
},
})
}

pub(crate) fn warn_empty_attribute(&mut self, span: Span) {
self.emit_lint(AttributeLintKind::EmptyAttribute { span }, span);
}
}

impl<'f, 'sess, S: Stage> Deref for AcceptContext<'f, 'sess, S> {
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_attr_parsing/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,11 @@ pub fn emit_attribute_lint<L: LintEmitter>(lint: &AttributeLint<HirId>, lint_emi
},
);
}
AttributeLintKind::EmptyAttribute { span } => lint_emitter.emit_node_span_lint(
rustc_session::lint::builtin::UNUSED_ATTRIBUTES,
*id,
*span,
session_diagnostics::EmptyAttributeList { attr_span: *span },
),
}
}
7 changes: 7 additions & 0 deletions compiler/rustc_attr_parsing/src/session_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,13 @@ pub(crate) struct EmptyConfusables {
pub span: Span,
}

#[derive(LintDiagnostic)]
#[diag(attr_parsing_empty_attribute)]
pub(crate) struct EmptyAttributeList {
#[suggestion(code = "", applicability = "machine-applicable")]
pub attr_span: Span,
}

#[derive(Diagnostic)]
#[diag(attr_parsing_invalid_alignment_value, code = E0589)]
pub(crate) struct InvalidAlignmentValue {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ impl<'a> TraitDef<'a> {
Annotatable::Item(item) => {
let is_packed = matches!(
AttributeParser::parse_limited(cx.sess, &item.attrs, sym::repr, item.span, item.id),
Some(Attribute::Parsed(AttributeKind::Repr(r))) if r.iter().any(|(x, _)| matches!(x, ReprPacked(..)))
Some(Attribute::Parsed(AttributeKind::Repr(r, _))) if r.iter().any(|(x, _)| matches!(x, ReprPacked(..)))
);

let newitem = match &item.kind {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {

if let hir::Attribute::Parsed(p) = attr {
match p {
AttributeKind::Repr(reprs) => {
AttributeKind::Repr(reprs, _) => {
codegen_fn_attrs.alignment = reprs
.iter()
.filter_map(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,7 @@ impl SyntaxExtension {
is_local: bool,
) -> SyntaxExtension {
let allow_internal_unstable =
find_attr!(attrs, AttributeKind::AllowInternalUnstable(i) => i)
find_attr!(attrs, AttributeKind::AllowInternalUnstable(i, _) => i)
.map(|i| i.as_slice())
.unwrap_or_default();
// FIXME(jdonszelman): allow_internal_unsafe isn't yet new-style
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1224,7 +1224,7 @@ pub(super) fn check_packed(tcx: TyCtxt<'_>, sp: Span, def: ty::AdtDef<'_>) {
let repr = def.repr();
if repr.packed() {
if let Some(reprs) =
attrs::find_attr!(tcx.get_all_attrs(def.did()), attrs::AttributeKind::Repr(r) => r)
attrs::find_attr!(tcx.get_all_attrs(def.did()), attrs::AttributeKind::Repr(r, _) => r)
{
for (r, _) in reprs {
if let ReprPacked(pack) = r
Expand Down Expand Up @@ -1447,7 +1447,7 @@ fn check_enum(tcx: TyCtxt<'_>, def_id: LocalDefId) {
if def.variants().is_empty() {
attrs::find_attr!(
tcx.get_all_attrs(def_id),
attrs::AttributeKind::Repr(rs) => {
attrs::AttributeKind::Repr(rs, _) => {
struct_span_code_err!(
tcx.dcx(),
rs.first().unwrap().1,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/nonstandard_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ impl EarlyLintPass for NonCamelCaseTypes {
fn check_item(&mut self, cx: &EarlyContext<'_>, it: &ast::Item) {
let has_repr_c = matches!(
AttributeParser::parse_limited(cx.sess(), &it.attrs, sym::repr, it.span, it.id),
Some(Attribute::Parsed(AttributeKind::Repr(r))) if r.iter().any(|(r, _)| r == &ReprAttr::ReprC)
Some(Attribute::Parsed(AttributeKind::Repr(r, _))) if r.iter().any(|(r, _)| r == &ReprAttr::ReprC)
);

if has_repr_c {
Expand Down
7 changes: 2 additions & 5 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1521,7 +1521,8 @@ impl<'tcx> TyCtxt<'tcx> {
field_shuffle_seed ^= user_seed;
}

if let Some(reprs) = attr::find_attr!(self.get_all_attrs(did), AttributeKind::Repr(r) => r)
if let Some(reprs) =
attr::find_attr!(self.get_all_attrs(did), AttributeKind::Repr(r, _) => r)
{
for (r, _) in reprs {
flags.insert(match *r {
Expand Down Expand Up @@ -1562,10 +1563,6 @@ impl<'tcx> TyCtxt<'tcx> {
max_align = max_align.max(Some(align));
ReprFlags::empty()
}
attr::ReprEmpty => {
/* skip these, they're just for diagnostics */
ReprFlags::empty()
}
});
}
}
Expand Down
Loading
Loading