Skip to content

Commit 2e5e511

Browse files
VeykrilVeykril
authored
Merge pull request #19416 from ShoyuVanilla/issue-15037
fix: Handle multiple `#[repr(..)]` attrs correctly
2 parents 9a3e037 + 602b4f3 commit 2e5e511

File tree

3 files changed

+91
-71
lines changed

3 files changed

+91
-71
lines changed

‎crates/hir-def/src/attr.rs‎

Lines changed: 79 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,12 @@ impl Attrs {
233233
}
234234

235235
pub fn repr(&self) -> Option<ReprOptions> {
236-
self.by_key(&sym::repr).tt_values().find_map(parse_repr_tt)
236+
self.by_key(&sym::repr).tt_values().filter_map(parse_repr_tt).fold(None, |acc, repr| {
237+
acc.map_or(Some(repr), |mut acc| {
238+
merge_repr(&mut acc, repr);
239+
Some(acc)
240+
})
241+
})
237242
}
238243
}
239244

@@ -260,6 +265,19 @@ fn parse_rustc_legacy_const_generics(tt: &crate::tt::TopSubtree) -> Box<[u32]> {
260265
indices.into_boxed_slice()
261266
}
262267

268+
fn merge_repr(this: &mut ReprOptions, other: ReprOptions) {
269+
let ReprOptions { int, align, pack, flags, field_shuffle_seed: _ } = this;
270+
flags.insert(other.flags);
271+
*align = (*align).max(other.align);
272+
*pack = match (*pack, other.pack) {
273+
(Some(pack), None) | (None, Some(pack)) => Some(pack),
274+
_ => (*pack).min(other.pack),
275+
};
276+
if other.int.is_some() {
277+
*int = other.int;
278+
}
279+
}
280+
263281
fn parse_repr_tt(tt: &crate::tt::TopSubtree) -> Option<ReprOptions> {
264282
use crate::builtin_type::{BuiltinInt, BuiltinUint};
265283
use rustc_abi::{Align, Integer, IntegerType, ReprFlags, ReprOptions};
@@ -269,83 +287,76 @@ fn parse_repr_tt(tt: &crate::tt::TopSubtree) -> Option<ReprOptions> {
269287
_ => return None,
270288
}
271289

272-
let mut flags = ReprFlags::empty();
273-
let mut int = None;
274-
let mut max_align: Option<Align> = None;
275-
let mut min_pack: Option<Align> = None;
276-
290+
let mut acc = ReprOptions::default();
277291
let mut tts = tt.iter();
278292
while let Some(tt) = tts.next() {
279-
if let TtElement::Leaf(tt::Leaf::Ident(ident)) = tt {
280-
flags.insert(match &ident.sym {
281-
s if *s == sym::packed => {
282-
let pack = if let Some(TtElement::Subtree(_, mut tt_iter)) = tts.peek() {
283-
tts.next();
284-
if let Some(TtElement::Leaf(tt::Leaf::Literal(lit))) = tt_iter.next() {
285-
lit.symbol.as_str().parse().unwrap_or_default()
286-
} else {
287-
0
288-
}
293+
let TtElement::Leaf(tt::Leaf::Ident(ident)) = tt else {
294+
continue;
295+
};
296+
let repr = match &ident.sym {
297+
s if *s == sym::packed => {
298+
let pack = if let Some(TtElement::Subtree(_, mut tt_iter)) = tts.peek() {
299+
tts.next();
300+
if let Some(TtElement::Leaf(tt::Leaf::Literal(lit))) = tt_iter.next() {
301+
lit.symbol.as_str().parse().unwrap_or_default()
289302
} else {
290303
0
291-
};
292-
let pack = Align::from_bytes(pack).unwrap_or(Align::ONE);
293-
min_pack =
294-
Some(if let Some(min_pack) = min_pack { min_pack.min(pack) } else { pack });
295-
ReprFlags::empty()
296-
}
297-
s if *s == sym::align => {
298-
if let Some(TtElement::Subtree(_, mut tt_iter)) = tts.peek() {
299-
tts.next();
300-
if let Some(TtElement::Leaf(tt::Leaf::Literal(lit))) = tt_iter.next() {
301-
if let Ok(align) = lit.symbol.as_str().parse() {
302-
let align = Align::from_bytes(align).ok();
303-
max_align = max_align.max(align);
304-
}
304+
}
305+
} else {
306+
0
307+
};
308+
let pack = Some(Align::from_bytes(pack).unwrap_or(Align::ONE));
309+
ReprOptions { pack, ..Default::default() }
310+
}
311+
s if *s == sym::align => {
312+
let mut align = None;
313+
if let Some(TtElement::Subtree(_, mut tt_iter)) = tts.peek() {
314+
tts.next();
315+
if let Some(TtElement::Leaf(tt::Leaf::Literal(lit))) = tt_iter.next() {
316+
if let Ok(a) = lit.symbol.as_str().parse() {
317+
align = Align::from_bytes(a).ok();
305318
}
306319
}
307-
ReprFlags::empty()
308320
}
309-
s if *s == sym::C => ReprFlags::IS_C,
310-
s if *s == sym::transparent => ReprFlags::IS_TRANSPARENT,
311-
s if *s == sym::simd => ReprFlags::IS_SIMD,
312-
repr => {
313-
if let Some(builtin) = BuiltinInt::from_suffix_sym(repr)
314-
.map(Either::Left)
315-
.or_else(|| BuiltinUint::from_suffix_sym(repr).map(Either::Right))
316-
{
317-
int = Some(match builtin {
318-
Either::Left(bi) => match bi {
319-
BuiltinInt::Isize => IntegerType::Pointer(true),
320-
BuiltinInt::I8 => IntegerType::Fixed(Integer::I8, true),
321-
BuiltinInt::I16 => IntegerType::Fixed(Integer::I16, true),
322-
BuiltinInt::I32 => IntegerType::Fixed(Integer::I32, true),
323-
BuiltinInt::I64 => IntegerType::Fixed(Integer::I64, true),
324-
BuiltinInt::I128 => IntegerType::Fixed(Integer::I128, true),
325-
},
326-
Either::Right(bu) => match bu {
327-
BuiltinUint::Usize => IntegerType::Pointer(false),
328-
BuiltinUint::U8 => IntegerType::Fixed(Integer::I8, false),
329-
BuiltinUint::U16 => IntegerType::Fixed(Integer::I16, false),
330-
BuiltinUint::U32 => IntegerType::Fixed(Integer::I32, false),
331-
BuiltinUint::U64 => IntegerType::Fixed(Integer::I64, false),
332-
BuiltinUint::U128 => IntegerType::Fixed(Integer::I128, false),
333-
},
334-
});
335-
}
336-
ReprFlags::empty()
321+
ReprOptions { align, ..Default::default() }
322+
}
323+
s if *s == sym::C => ReprOptions { flags: ReprFlags::IS_C, ..Default::default() },
324+
s if *s == sym::transparent => {
325+
ReprOptions { flags: ReprFlags::IS_TRANSPARENT, ..Default::default() }
326+
}
327+
s if *s == sym::simd => ReprOptions { flags: ReprFlags::IS_SIMD, ..Default::default() },
328+
repr => {
329+
let mut int = None;
330+
if let Some(builtin) = BuiltinInt::from_suffix_sym(repr)
331+
.map(Either::Left)
332+
.or_else(|| BuiltinUint::from_suffix_sym(repr).map(Either::Right))
333+
{
334+
int = Some(match builtin {
335+
Either::Left(bi) => match bi {
336+
BuiltinInt::Isize => IntegerType::Pointer(true),
337+
BuiltinInt::I8 => IntegerType::Fixed(Integer::I8, true),
338+
BuiltinInt::I16 => IntegerType::Fixed(Integer::I16, true),
339+
BuiltinInt::I32 => IntegerType::Fixed(Integer::I32, true),
340+
BuiltinInt::I64 => IntegerType::Fixed(Integer::I64, true),
341+
BuiltinInt::I128 => IntegerType::Fixed(Integer::I128, true),
342+
},
343+
Either::Right(bu) => match bu {
344+
BuiltinUint::Usize => IntegerType::Pointer(false),
345+
BuiltinUint::U8 => IntegerType::Fixed(Integer::I8, false),
346+
BuiltinUint::U16 => IntegerType::Fixed(Integer::I16, false),
347+
BuiltinUint::U32 => IntegerType::Fixed(Integer::I32, false),
348+
BuiltinUint::U64 => IntegerType::Fixed(Integer::I64, false),
349+
BuiltinUint::U128 => IntegerType::Fixed(Integer::I128, false),
350+
},
351+
});
337352
}
338-
})
339-
}
353+
ReprOptions { int, ..Default::default() }
354+
}
355+
};
356+
merge_repr(&mut acc, repr);
340357
}
341358

342-
Some(ReprOptions {
343-
int,
344-
align: max_align,
345-
pack: min_pack,
346-
flags,
347-
field_shuffle_seed: rustc_hashes::Hash64::ZERO,
348-
})
359+
Some(acc)
349360
}
350361

351362
#[derive(Debug, Clone, PartialEq, Eq, Hash)]

‎crates/hir-def/src/lib.rs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,6 @@ extern crate rustc_hashes;
2424
#[cfg(not(feature = "in-rust-tree"))]
2525
extern crate ra_ap_rustc_abi as rustc_abi;
2626

27-
#[cfg(not(feature = "in-rust-tree"))]
28-
extern crate ra_ap_rustc_hashes as rustc_hashes;
29-
3027
pub mod db;
3128

3229
pub mod attr;

‎crates/hir-ty/src/layout/tests.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,18 @@ fn repr_packed() {
284284
check_size_and_align("#[repr(Rust, packed(5))] struct Goal(i32);", "", 4, 1);
285285
}
286286

287+
#[test]
288+
fn multiple_repr_attrs() {
289+
size_and_align!(
290+
#[repr(C)]
291+
#[repr(packed)]
292+
struct Goal {
293+
id: i32,
294+
u: u8,
295+
}
296+
)
297+
}
298+
287299
#[test]
288300
fn generic() {
289301
size_and_align! {

0 commit comments

Comments
 (0)