Skip to content

Rewrite the #[repr] attribute parser #157125

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.

Sign up for GitHub

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

Open
JonathanBrouwer wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Rewrite the #[repr] attribute parser #157125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
525723d
Change `expected_identifier` from error code E0539 to the more specif…
JonathanBrouwer May 29, 2026
c72dc2b
Clean up `extend` function of `ReprParser`
JonathanBrouwer May 29, 2026
e6b237a
Clean up `parse_repr` function of `ReprParser`
JonathanBrouwer May 29, 2026
7aef596
Clean up `parse_repr_align` function of `ReprParser`
JonathanBrouwer May 29, 2026
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
2 changes: 1 addition & 1 deletion compiler/rustc_attr_parsing/src/attributes/prelude.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub(super) use rustc_hir::attrs::AttributeKind;
#[doc(hidden)]
pub(super) use rustc_hir::{MethodKind, Target};
#[doc(hidden)]
pub(super) use rustc_span::{DUMMY_SP, Ident, Span, Symbol, sym};
pub(super) use rustc_span::{Ident, Span, Symbol, sym};
#[doc(hidden)]
pub(super) use thin_vec::ThinVec;

Expand Down
251 changes: 80 additions & 171 deletions compiler/rustc_attr_parsing/src/attributes/repr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,27 @@
use rustc_abi::{Align, Size};
use rustc_ast::{IntTy, LitIntType, LitKind, UintTy};
use rustc_hir::attrs::{IntType, ReprAttr};
use rustc_hir::attrs::IntType::{SignedInt, UnsignedInt};
use rustc_hir::attrs::ReprAttr;

use super::prelude::*;
use crate::session_diagnostics::{self, IncorrectReprFormatGenericCause};
use crate::session_diagnostics;

/// Parse #[repr(...)] forms.
///
/// Valid repr contents: any of the primitive integral type names (see
/// `int_type_of_word`, below) to specify enum discriminant type; `C`, to use
/// the same discriminant size that the corresponding C enum would or C
/// structure layout, `packed` to remove padding, and `transparent` to delegate representation
/// concerns to the only non-ZST field.
// FIXME(jdonszelmann): is a vec the right representation here even? isn't it just a struct?
/// Valid repr contents:
/// * any of the primitive integral type names to specify enum discriminant type
/// * `Rust`, to use the default `Rust` layout of the type
/// * `C`, to use the same layout for the type that C would use
/// * `align(...)`, to change the alignment requirements of the type
/// * `packed`, to remove padding
/// * `transparent`, to delegate representation concerns to the only non-ZST field.
pub(crate) struct ReprParser;

impl CombineAttributeParser for ReprParser {
type Item = (ReprAttr, Span);
const PATH: &[Symbol] = &[sym::repr];
const CONVERT: ConvertFn<Self::Item> =
|items, first_span| AttributeKind::Repr { reprs: items, first_span };
// FIXME(jdonszelmann): never used
const TEMPLATE: AttributeTemplate = template!(
List: &["C", "Rust", "transparent", "align(...)", "packed(...)", "<integer type>"],
"https://doc.rust-lang.org/reference/type-layout.html#representations"
Expand All @@ -30,29 +31,24 @@ impl CombineAttributeParser for ReprParser {
cx: &mut AcceptContext<'_, '_>,
args: &ArgParser,
) -> impl IntoIterator<Item = Self::Item> {
let mut reprs = Vec::new();

let Some(list) = cx.expect_list(args, cx.attr_span) else {
return reprs;
return vec![];
};

if list.is_empty() {
let attr_span = cx.attr_span;
cx.adcx().warn_empty_attribute(attr_span);
return reprs;
return vec![];
}

let mut reprs = Vec::new();
for param in list.mixed() {
if let Some(_) = param.as_lit() {
cx.emit_err(session_diagnostics::ReprIdent { span: cx.attr_span });
let Some(item) = param.meta_item() else {
cx.adcx().expected_identifier(param.span());
continue;
}

reprs.extend(
param.meta_item().and_then(|mi| parse_repr(cx, &mi)).map(|r| (r, param.span())),
);
};
reprs.extend(parse_repr(cx, &item).map(|r| (r, param.span())));
}

reprs
}

Expand All @@ -61,122 +57,69 @@ impl CombineAttributeParser for ReprParser {
const ALLOWED_TARGETS: AllowedTargets = AllowedTargets::AllowList(ALL_TARGETS);
}

macro_rules! int_pat {
() => {
sym::i8
| sym::u8
| sym::i16
| sym::u16
| sym::i32
| sym::u32
| sym::i64
| sym::u64
| sym::i128
| sym::u128
| sym::isize
| sym::usize
};
}

fn int_type_of_word(s: Symbol) -> Option<IntType> {
use IntType::*;

match s {
sym::i8 => Some(SignedInt(IntTy::I8)),
sym::u8 => Some(UnsignedInt(UintTy::U8)),
sym::i16 => Some(SignedInt(IntTy::I16)),
sym::u16 => Some(UnsignedInt(UintTy::U16)),
sym::i32 => Some(SignedInt(IntTy::I32)),
sym::u32 => Some(UnsignedInt(UintTy::U32)),
sym::i64 => Some(SignedInt(IntTy::I64)),
sym::u64 => Some(UnsignedInt(UintTy::U64)),
sym::i128 => Some(SignedInt(IntTy::I128)),
sym::u128 => Some(UnsignedInt(UintTy::U128)),
sym::isize => Some(SignedInt(IntTy::Isize)),
sym::usize => Some(UnsignedInt(UintTy::Usize)),
_ => None,
}
}

fn parse_repr(cx: &AcceptContext<'_, '_>, param: &MetaItemParser) -> Option<ReprAttr> {
fn parse_repr(cx: &mut AcceptContext<'_, '_>, param: &MetaItemParser) -> Option<ReprAttr> {
use ReprAttr::*;

// FIXME(jdonszelmann): invert the parsing here to match on the word first and then the
// structure.
let (name, ident_span) = if let Some(ident) = param.path().word() {
(Some(ident.name), ident.span)
} else {
(None, DUMMY_SP)
};

let args = param.args();

match (name, args) {
(Some(sym::align), ArgParser::NoArgs) => {
cx.emit_err(session_diagnostics::InvalidReprAlignNeedArg { span: ident_span });
None
}
(Some(sym::align), ArgParser::List(l)) => {
parse_repr_align(cx, l, param.span(), AlignKind::Align)
}

(Some(sym::packed), ArgParser::NoArgs) => Some(ReprPacked(Align::ONE)),
(Some(sym::packed), ArgParser::List(l)) => {
parse_repr_align(cx, l, param.span(), AlignKind::Packed)
}

(Some(name @ sym::align | name @ sym::packed), ArgParser::NameValue(l)) => {
cx.emit_err(session_diagnostics::IncorrectReprFormatGeneric {
span: param.span(),
// FIXME(jdonszelmann) can just be a string in the diag type
repr_arg: name,
cause: IncorrectReprFormatGenericCause::from_lit_kind(
param.span(),
&l.value_as_lit().kind,
name,
),
});
None
}

(Some(sym::Rust), ArgParser::NoArgs) => Some(ReprRust),
(Some(sym::C), ArgParser::NoArgs) => Some(ReprC),
(Some(sym::simd), ArgParser::NoArgs) => Some(ReprSimd),
(Some(sym::transparent), ArgParser::NoArgs) => Some(ReprTransparent),
(Some(name @ int_pat!()), ArgParser::NoArgs) => {
// int_pat!() should make sure it always parses
Some(ReprInt(int_type_of_word(name).unwrap()))
}
macro_rules! no_args {
($constructor: expr) => {{
cx.expect_no_args(param.args())?;
Some($constructor)
}};
}

(
Some(
name @ sym::Rust
| name @ sym::C
| name @ sym::simd
| name @ sym::transparent
| name @ int_pat!(),
),
ArgParser::NameValue(_),
) => {
cx.emit_err(session_diagnostics::InvalidReprHintNoValue { span: param.span(), name });
None
}
(
Some(
name @ sym::Rust
| name @ sym::C
| name @ sym::simd
| name @ sym::transparent
| name @ int_pat!(),
),
ArgParser::List(_),
) => {
cx.emit_err(session_diagnostics::InvalidReprHintNoParen { span: param.span(), name });
None
match param.path().word_sym() {
Some(sym::align) => {
let l = cx.expect_list(param.args(), param.span())?;
parse_repr_align(cx, l, AlignKind::Align)
}

Some(sym::packed) => match param.args() {
ArgParser::NoArgs => Some(ReprPacked(Align::ONE)),
ArgParser::List(l) => parse_repr_align(cx, l, AlignKind::Packed),
ArgParser::NameValue(_) => {
cx.adcx().expected_list_or_no_args(param.span());
None
}
},
Some(sym::Rust) => no_args!(ReprRust),
Some(sym::C) => no_args!(ReprC),
Some(sym::simd) => no_args!(ReprSimd),
Some(sym::transparent) => no_args!(ReprTransparent),
Some(sym::i8) => no_args!(ReprInt(SignedInt(IntTy::I8))),
Some(sym::u8) => no_args!(ReprInt(UnsignedInt(UintTy::U8))),
Some(sym::i16) => no_args!(ReprInt(SignedInt(IntTy::I16))),
Some(sym::u16) => no_args!(ReprInt(UnsignedInt(UintTy::U16))),
Some(sym::i32) => no_args!(ReprInt(SignedInt(IntTy::I32))),
Some(sym::u32) => no_args!(ReprInt(UnsignedInt(UintTy::U32))),
Some(sym::i64) => no_args!(ReprInt(SignedInt(IntTy::I64))),
Some(sym::u64) => no_args!(ReprInt(UnsignedInt(UintTy::U64))),
Some(sym::i128) => no_args!(ReprInt(SignedInt(IntTy::I128))),
Some(sym::u128) => no_args!(ReprInt(UnsignedInt(UintTy::U128))),
Some(sym::isize) => no_args!(ReprInt(SignedInt(IntTy::Isize))),
Some(sym::usize) => no_args!(ReprInt(UnsignedInt(UintTy::Usize))),
_ => {
cx.emit_err(session_diagnostics::UnrecognizedReprHint { span: param.span() });
cx.adcx().expected_specific_argument(
param.span(),
&[
sym::align,
Copy link
Copy Markdown
Contributor Author

@JonathanBrouwer JonathanBrouwer May 29, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

One thing that still annoys me is that in a lot of attribute parsers have these match statements, and then we need to list all branches of the match statement again for the expected_specific_argument error

View changes since the review

Copy link
Copy Markdown
Contributor

@mejrs mejrs May 29, 2026

Choose a reason for hiding this comment

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

I'd like a solution where these symbols are only listed once.

Maybe make an array of [(Symbol, constructor); _] (or maybe [(Symbol, checker, constructor); _]) and if you don't find the symbol in the array, do something like array.iter().map(|x|x.0).collect() and pass that to expected_specific_argument.

See

rust/compiler/rustc_resolve/src/macros.rs

Line 716 in 6368fd5

const DIAGNOSTIC_ATTRIBUTES: &[(Symbol, Option<Symbol>)] = &[
for inspiration.

Copy link
Copy Markdown
Contributor Author

@JonathanBrouwer JonathanBrouwer May 30, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

(let's leave this for a separate PR tho, since this affects more places that I'd like to fix at the same time)

sym::packed,
sym::Rust,
sym::C,
sym::simd,
sym::transparent,
sym::i8,
sym::u8,
sym::i16,
sym::u16,
sym::i32,
sym::u32,
sym::i64,
sym::u64,
sym::i128,
sym::u128,
sym::isize,
sym::usize,
],
);
None
}
}
Expand All @@ -188,44 +131,17 @@ enum AlignKind {
}

fn parse_repr_align(
cx: &AcceptContext<'_, '_>,
cx: &mut AcceptContext<'_, '_>,
list: &MetaItemListParser,
param_span: Span,
align_kind: AlignKind,
) -> Option<ReprAttr> {
use AlignKind::*;

let Some(align) = list.as_single() else {
match align_kind {
Packed => {
cx.emit_err(session_diagnostics::IncorrectReprFormatPackedOneOrZeroArg {
span: param_span,
});
}
Align => {
cx.emit_err(session_diagnostics::IncorrectReprFormatAlignOneArg {
span: param_span,
});
}
}

cx.adcx().expected_single_argument(list.span, list.len());
return None;
};

let Some(lit) = align.as_lit() else {
match align_kind {
Packed => {
cx.emit_err(session_diagnostics::IncorrectReprFormatPackedExpectInteger {
span: align.span(),
});
}
Align => {
cx.emit_err(session_diagnostics::IncorrectReprFormatExpectInteger {
span: align.span(),
});
}
}

cx.adcx().expected_integer_literal(align.span());
return None;
};

Expand All @@ -235,12 +151,8 @@ fn parse_repr_align(
AlignKind::Align => ReprAttr::ReprAlign(literal),
}),
Err(message) => {
cx.emit_err(session_diagnostics::InvalidReprGeneric {
cx.emit_err(session_diagnostics::InvalidAlignmentValue {
span: lit.span,
repr_arg: match align_kind {
Packed => "packed".to_string(),
Align => "align".to_string(),
},
error_part: message,
});
None
Expand Down Expand Up @@ -294,10 +206,7 @@ impl RustcAlignParser {
};

let Some(lit) = align.as_lit() else {
cx.emit_err(session_diagnostics::IncorrectReprFormatExpectInteger {
span: align.span(),
});

cx.adcx().expected_integer_literal(align.span());
return;
};

Expand Down
Loading
Loading