Skip to content

Commit 3e7b661

Browse files
stepanchegfacebook-github-bot
authored andcommitted
Fix the issue with TypeId alignment on new Rust Apple Silicon
Summary: * `TypeId` is `u128` [in Rust 1.72](rust-lang/rust#109953) * `u128` is 16 bytes aligned on Apple Silicon * we require 8 byte alignment for `StarlarkValue` Maybe we should fix heap allocations: insert padding to allow any alignment. But practically we don't need it and should not do it: generated machine code is identical for aligned and 8-byte aligned `u128`: [compiler explorer](https://rust.godbolt.org/z/96KsnjW77) (note it is identical if we load it from memory not pass by value, but this is what we do). So for now just force 8 byte alignment on `TypeId`. Fixes issue facebook/buck2#408 Reviewed By: ndmitchell, shayne-fletcher Differential Revision: D49019964 fbshipit-source-id: 62c8d7db83e9e4bdbe58fe7a62d5daae41996e61
1 parent 1548298 commit 3e7b661

File tree

2 files changed

+34
-3
lines changed

2 files changed

+34
-3
lines changed

starlark/src/values/starlark_type_id.rs

+30
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,33 @@ impl StarlarkTypeId {
4747
StarlarkTypeId(ConstTypeId::of::<T::StaticType>())
4848
}
4949
}
50+
51+
/// We require alignment 8 for `StarlarkValue`.
52+
/// `TypeId` is 16 bytes aligned on Rust 1.72 on Apple Silicon.
53+
/// Use this struct to put `ConstTypeId` in a `StarlarkValue`.
54+
// TODO(nga): remove alignment requirement from `Heap`.
55+
#[repr(packed(8))]
56+
#[derive(Allocative, Eq, Clone, Copy, Dupe, Debug)]
57+
#[allocative(skip)] // There are no heap allocations in this struct.
58+
pub(crate) struct StarlarkTypeIdAligned {
59+
starlark_type_id: StarlarkTypeId,
60+
}
61+
62+
impl PartialEq for StarlarkTypeIdAligned {
63+
#[inline]
64+
fn eq(&self, other: &Self) -> bool {
65+
self.get() == other.get()
66+
}
67+
}
68+
69+
impl StarlarkTypeIdAligned {
70+
#[inline]
71+
pub(crate) const fn new(starlark_type_id: StarlarkTypeId) -> StarlarkTypeIdAligned {
72+
StarlarkTypeIdAligned { starlark_type_id }
73+
}
74+
75+
#[inline]
76+
pub(crate) const fn get(&self) -> StarlarkTypeId {
77+
self.starlark_type_id
78+
}
79+
}

starlark/src/values/typing/type_compiled/matchers.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use crate::values::dict::DictRef;
2727
use crate::values::list::value::FrozenList;
2828
use crate::values::list::ListRef;
2929
use crate::values::starlark_type_id::StarlarkTypeId;
30+
use crate::values::starlark_type_id::StarlarkTypeIdAligned;
3031
use crate::values::tuple::value::Tuple;
3132
use crate::values::types::int_or_big::StarlarkIntRef;
3233
use crate::values::typing::type_compiled::matcher::TypeMatcher;
@@ -245,20 +246,20 @@ impl TypeMatcher for IsNone {
245246

246247
#[derive(Allocative, Debug, Clone)]
247248
pub(crate) struct StarlarkTypeIdMatcher {
248-
starlark_type_id: StarlarkTypeId,
249+
starlark_type_id: StarlarkTypeIdAligned,
249250
}
250251

251252
impl StarlarkTypeIdMatcher {
252253
pub(crate) fn new(ty: TyStarlarkValue) -> StarlarkTypeIdMatcher {
253254
StarlarkTypeIdMatcher {
254-
starlark_type_id: ty.starlark_type_id(),
255+
starlark_type_id: StarlarkTypeIdAligned::new(ty.starlark_type_id()),
255256
}
256257
}
257258
}
258259

259260
impl TypeMatcher for StarlarkTypeIdMatcher {
260261
fn matches(&self, value: Value) -> bool {
261-
value.starlark_type_id() == self.starlark_type_id
262+
value.starlark_type_id() == self.starlark_type_id.get()
262263
}
263264
}
264265

0 commit comments

Comments
 (0)