Skip to content

Commit c79fc2c

Browse files
committed
fix(ast_tools): fix calculation of niches for Options (oxc-project#9422)
Fix a bug in memory layout calculations in `oxc_ast_tools`. It was determining the niche for `Option`s incorrectly where the `Option`'s discriminants don't start at 0. Rust compiler appears to now prefer a lower niche value, and allows a value to have niches at both the bottom and top of the range.
1 parent b65f8a5 commit c79fc2c

File tree

2 files changed

+62
-29
lines changed

2 files changed

+62
-29
lines changed

tasks/ast_tools/src/generators/assert_layouts.rs

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,16 @@ fn calculate_layout_for_struct(type_id: TypeId, schema: &mut Schema) -> Layout {
162162
layout.align = max(layout.align, field_layout.align);
163163

164164
// Update niche.
165-
// Take the largest niche. Preference for earlier niche if 2 fields have niches of same size.
165+
// Take the largest niche. Preference for (in order):
166+
// * Largest single range of niche values.
167+
// * Largest number of niche values at start of range.
168+
// * Earlier field.
166169
if let Some(field_niche) = &field_layout.niche {
167-
if layout.niche.as_ref().is_none_or(|niche| field_niche.count > niche.count) {
170+
if layout.niche.as_ref().is_none_or(|niche| {
171+
field_niche.count_max() > niche.count_max()
172+
|| (field_niche.count_max() == niche.count_max()
173+
&& field_niche.count_start > niche.count_start)
174+
}) {
168175
let mut niche = field_niche.clone();
169176
niche.offset += offset;
170177
layout.niche = Some(niche);
@@ -258,9 +265,7 @@ fn calculate_layout_for_enum(type_id: TypeId, schema: &mut Schema) -> Layout {
258265
let niches_end = Discriminant::MAX - max_discriminant;
259266

260267
if niches_start != 0 || niches_end != 0 {
261-
let is_range_start = niches_start >= niches_end;
262-
let count = u32::from(if is_range_start { niches_start } else { niches_end });
263-
let niche = Niche::new(0, 1, is_range_start, count);
268+
let niche = Niche::new(0, 1, u32::from(niches_start), u32::from(niches_end));
264269
layout_64.niche = Some(niche.clone());
265270
layout_32.niche = Some(niche);
266271
}
@@ -284,14 +289,18 @@ fn calculate_layout_for_option(type_id: TypeId, schema: &mut Schema) -> Layout {
284289
#[expect(clippy::items_after_statements)]
285290
fn consume_niche(layout: &mut PlatformLayout) {
286291
if let Some(niche) = &mut layout.niche {
287-
if niche.count == 1 {
288-
layout.niche = None;
292+
if niche.count_start == 0 {
293+
niche.count_end -= 1;
289294
} else {
290-
niche.count -= 1;
295+
niche.count_start -= 1;
296+
}
297+
298+
if niche.count_start == 0 && niche.count_end == 0 {
299+
layout.niche = None;
291300
}
292301
} else {
293302
layout.size += layout.align;
294-
layout.niche = Some(Niche::new(0, 1, false, 254));
303+
layout.niche = Some(Niche::new(0, 1, 0, 254));
295304
}
296305
}
297306

@@ -307,8 +316,8 @@ fn calculate_layout_for_option(type_id: TypeId, schema: &mut Schema) -> Layout {
307316
/// `Box`es are pointer-sized, with a single niche (like `NonNull`).
308317
fn calculate_layout_for_box() -> Layout {
309318
Layout {
310-
layout_64: PlatformLayout::from_size_align_niche(8, 8, Niche::new(0, 8, true, 1)),
311-
layout_32: PlatformLayout::from_size_align_niche(4, 4, Niche::new(0, 4, true, 1)),
319+
layout_64: PlatformLayout::from_size_align_niche(8, 8, Niche::new(0, 8, 1, 0)),
320+
layout_32: PlatformLayout::from_size_align_niche(4, 4, Niche::new(0, 4, 1, 0)),
312321
}
313322
}
314323

@@ -319,8 +328,8 @@ fn calculate_layout_for_box() -> Layout {
319328
/// They have a single niche on the first field - the pointer which is `NonNull`.
320329
fn calculate_layout_for_vec() -> Layout {
321330
Layout {
322-
layout_64: PlatformLayout::from_size_align_niche(32, 8, Niche::new(0, 8, true, 1)),
323-
layout_32: PlatformLayout::from_size_align_niche(16, 4, Niche::new(0, 4, true, 1)),
331+
layout_64: PlatformLayout::from_size_align_niche(32, 8, Niche::new(0, 8, 1, 0)),
332+
layout_32: PlatformLayout::from_size_align_niche(16, 4, Niche::new(0, 4, 1, 0)),
324333
}
325334
}
326335

@@ -343,8 +352,8 @@ fn calculate_layout_for_cell(type_id: TypeId, schema: &mut Schema) -> Layout {
343352
fn calculate_layout_for_primitive(primitive_def: &PrimitiveDef) -> Layout {
344353
// `&str` and `Atom` are a `NonNull` pointer + `usize` pair. Niche for 0 on the pointer field
345354
let str_layout = Layout {
346-
layout_64: PlatformLayout::from_size_align_niche(16, 8, Niche::new(0, 8, true, 1)),
347-
layout_32: PlatformLayout::from_size_align_niche(8, 4, Niche::new(0, 4, true, 1)),
355+
layout_64: PlatformLayout::from_size_align_niche(16, 8, Niche::new(0, 8, 1, 0)),
356+
layout_32: PlatformLayout::from_size_align_niche(8, 4, Niche::new(0, 4, 1, 0)),
348357
};
349358
// `usize` and `isize` are pointer-sized, but with no niche
350359
let usize_layout = Layout {
@@ -353,13 +362,13 @@ fn calculate_layout_for_primitive(primitive_def: &PrimitiveDef) -> Layout {
353362
};
354363
// `NonZeroUsize` and `NonZeroIsize` are pointer-sized, with a single niche
355364
let non_zero_usize_layout = Layout {
356-
layout_64: PlatformLayout::from_size_align_niche(8, 8, Niche::new(0, 8, true, 1)),
357-
layout_32: PlatformLayout::from_size_align_niche(4, 4, Niche::new(0, 4, true, 1)),
365+
layout_64: PlatformLayout::from_size_align_niche(8, 8, Niche::new(0, 8, 1, 0)),
366+
layout_32: PlatformLayout::from_size_align_niche(4, 4, Niche::new(0, 4, 1, 0)),
358367
};
359368

360369
#[expect(clippy::match_same_arms)]
361370
match primitive_def.name() {
362-
"bool" => Layout::from_size_align_niche(1, 1, Niche::new(0, 1, false, 254)),
371+
"bool" => Layout::from_size_align_niche(1, 1, Niche::new(0, 1, 0, 254)),
363372
"u8" => Layout::from_type::<u8>(),
364373
"u16" => Layout::from_type::<u16>(),
365374
"u32" => Layout::from_type::<u32>(),

tasks/ast_tools/src/schema/extensions/layout.rs

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
use std::mem::{align_of, size_of};
1+
use std::{
2+
cmp::max,
3+
mem::{align_of, size_of},
4+
};
25

36
/// The layout of a type.
47
#[derive(Clone, Default, Debug)]
@@ -24,7 +27,7 @@ impl Layout {
2427
Self::from_size_align_niche(
2528
size,
2629
u32::try_from(align_of::<T>()).unwrap(),
27-
Niche::new(0, size, true, 1),
30+
Niche::new(0, size, 1, 0),
2831
)
2932
}
3033

@@ -75,20 +78,41 @@ pub struct Niche {
7578
/// Byte offset of the niche from start of type
7679
pub offset: u32,
7780
/// Size of the niche in bytes
78-
#[expect(dead_code)]
7981
pub size: u32,
80-
/// `true` if niche is at start of range (e.g. 0..3).
81-
/// `false` if niche is at end of range (e.g. 2..255).
82-
#[expect(dead_code)]
83-
pub is_range_start: bool,
84-
/// Number of niche values in the niche (e.g. 1 for `&str`, 254 for `bool`)
85-
pub count: u32,
82+
/// Number of values at start of range
83+
pub count_start: u32,
84+
/// Number of values at end of range
85+
pub count_end: u32,
8686
}
8787

8888
impl Niche {
8989
/// Create new [`Niche`].
90-
pub fn new(offset: u32, size: u32, is_range_start: bool, count: u32) -> Self {
91-
Self { offset, size, is_range_start, count }
90+
pub fn new(offset: u32, size: u32, count_start: u32, count_end: u32) -> Self {
91+
Self { offset, size, count_start, count_end }
92+
}
93+
94+
/// Get size of largest niche range (start or end)
95+
pub fn count_max(&self) -> u32 {
96+
max(self.count_start, self.count_end)
97+
}
98+
99+
/// Get value of the [`Niche`].
100+
#[expect(unused)]
101+
pub fn value(&self) -> u128 {
102+
// Prefer to consume niches at start of range over end of range
103+
if self.count_start > 0 {
104+
u128::from(self.count_start - 1)
105+
} else {
106+
let max_value = match self.size {
107+
1 => u128::from(u8::MAX),
108+
2 => u128::from(u16::MAX),
109+
4 => u128::from(u32::MAX),
110+
8 => u128::from(u64::MAX),
111+
16 => u128::MAX,
112+
size => panic!("Invalid niche size: {size}"),
113+
};
114+
max_value - u128::from(self.count_end) + 1
115+
}
92116
}
93117
}
94118

0 commit comments

Comments
 (0)