Skip to content

Commit 97e9ab6

Browse files
stepanchegfacebook-github-bot
authored andcommitted
Minor refactoring of strings
Summary: * change `ConstFrozenStringN` from ``` pub struct ConstFrozenStringN<const N: usize> { vtable: AValueHeader, object: StarlarkStr, payload: [u8; N], } ``` to ``` pub struct ConstFrozenStringN<const N: usize> { repr: AValueRepr<StarlarkStr>, payload: [u8; N], } ``` * change `ConstFrozenString` from ``` pub struct ConstFrozenString(&'static AValueHeader); ``` to ``` pub struct ConstFrozenString(&'static AValueRepr<StarlarkStr>); ``` and also * make `AValueRepr` fields `pub(crate)` * remove `AValueRepr::header()` method (because fields are `pub(crate)` now * add `FrozenValue::new_repr` constructor This is mostly for safety/readability. Reviewed By: ndmitchell Differential Revision: D30876688 fbshipit-source-id: 2fdeb457c73e080ca0f6147438a6197988366ee1
1 parent 0568794 commit 97e9ab6

File tree

4 files changed

+20
-18
lines changed

4 files changed

+20
-18
lines changed

starlark/src/values/layout/arena.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ pub(crate) struct AValueHeader(DynMetadata<dyn AValue<'static>>);
5353
/// How object is represented in arena.
5454
#[repr(C)]
5555
pub(crate) struct AValueRepr<T> {
56-
header: AValueHeader,
57-
payload: T,
56+
pub(crate) header: AValueHeader,
57+
pub(crate) payload: T,
5858
}
5959

6060
/// This is object written over [`AValueRepr`] during GC.
@@ -309,10 +309,6 @@ impl<T> AValueRepr<T> {
309309
payload,
310310
}
311311
}
312-
313-
pub(crate) const fn header(&self) -> &AValueHeader {
314-
&self.header
315-
}
316312
}
317313

318314
impl Drop for Arena {

starlark/src/values/layout/avalue.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,23 +41,23 @@ pub(crate) static VALUE_NONE: &AValueHeader = {
4141
const DYN: &dyn AValue<'static> = &PAYLOAD;
4242
static DATA: AValueRepr<Wrapper<Basic, NoneType>> =
4343
AValueRepr::with_metadata(metadata(DYN), PAYLOAD);
44-
DATA.header()
44+
&DATA.header
4545
};
4646

4747
pub(crate) static VALUE_FALSE: &AValueHeader = {
4848
const PAYLOAD: Wrapper<Basic, bool> = Wrapper(Basic, false);
4949
const DYN: &dyn AValue<'static> = &PAYLOAD;
5050
static DATA: AValueRepr<Wrapper<Basic, bool>> =
5151
AValueRepr::with_metadata(metadata(DYN), PAYLOAD);
52-
DATA.header()
52+
&DATA.header
5353
};
5454

5555
pub(crate) static VALUE_TRUE: &AValueHeader = {
5656
const PAYLOAD: Wrapper<Basic, bool> = Wrapper(Basic, true);
5757
const DYN: &dyn AValue<'static> = &PAYLOAD;
5858
static DATA: AValueRepr<Wrapper<Basic, bool>> =
5959
AValueRepr::with_metadata(metadata(DYN), PAYLOAD);
60-
DATA.header()
60+
&DATA.header
6161
};
6262

6363
pub(crate) const VALUE_STR_A_VALUE_PTR: AValueHeader = {

starlark/src/values/layout/constant.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
*/
1717

1818
use crate::values::{
19-
layout::{arena::AValueHeader, avalue::VALUE_STR_A_VALUE_PTR, value::FrozenValue},
19+
layout::{arena::AValueRepr, avalue::VALUE_STR_A_VALUE_PTR, value::FrozenValue},
2020
string::StarlarkStr,
2121
};
2222
use gazebo::prelude::*;
@@ -25,8 +25,7 @@ use std::intrinsics::copy_nonoverlapping;
2525
/// A constant string that can be converted to a [`FrozenValue`].
2626
#[repr(C)] // Must match this layout on the heap
2727
pub struct ConstFrozenStringN<const N: usize> {
28-
vtable: AValueHeader,
29-
object: StarlarkStr,
28+
repr: AValueRepr<StarlarkStr>,
3029
payload: [u8; N],
3130
}
3231

@@ -40,8 +39,10 @@ impl<const N: usize> ConstFrozenStringN<N> {
4039
copy_nonoverlapping(s.as_ptr(), payload.as_mut_ptr(), N)
4140
};
4241
Self {
43-
vtable: VALUE_STR_A_VALUE_PTR,
44-
object: unsafe { StarlarkStr::new(N) },
42+
repr: AValueRepr {
43+
header: VALUE_STR_A_VALUE_PTR,
44+
payload: unsafe { StarlarkStr::new(N) },
45+
},
4546
payload,
4647
}
4748
}
@@ -53,7 +54,7 @@ impl<const N: usize> ConstFrozenStringN<N> {
5354

5455
/// Erase the type parameter, giving a slightly nicer user experience.
5556
pub const fn erase(&'static self) -> ConstFrozenString {
56-
ConstFrozenString(&self.vtable)
57+
ConstFrozenString(&self.repr)
5758
}
5859
}
5960

@@ -70,12 +71,12 @@ impl<const N: usize> ConstFrozenStringN<N> {
7071
/// assert_eq!(Some("magic"), fv.to_value().unpack_str());
7172
/// ```
7273
#[derive(Copy, Clone, Dupe)]
73-
pub struct ConstFrozenString(&'static AValueHeader);
74+
pub struct ConstFrozenString(&'static AValueRepr<StarlarkStr>);
7475

7576
impl ConstFrozenString {
7677
/// Obtain the [`FrozenValue`] for a [`ConstFrozenString`].
7778
pub fn unpack(self) -> FrozenValue {
78-
FrozenValue::new_ptr(self.0)
79+
FrozenValue::new_repr(self.0)
7980
}
8081
}
8182

starlark/src/values/layout/value.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,14 @@
3131

3232
use crate::values::{
3333
layout::{
34-
arena::AValueHeader,
34+
arena::{AValueHeader, AValueRepr},
3535
avalue::{basic_ref, AValue, VALUE_FALSE, VALUE_NONE, VALUE_TRUE},
3636
constant::VALUE_EMPTY_STRING,
3737
pointer::Pointer,
3838
pointer_i32::PointerI32,
3939
},
4040
string::StarlarkStr,
41+
StarlarkValue,
4142
};
4243
use either::Either;
4344
use gazebo::{
@@ -192,6 +193,10 @@ impl FrozenValue {
192193
Self(Pointer::new_frozen(x))
193194
}
194195

196+
pub(crate) fn new_repr<'a, T: StarlarkValue<'a>>(x: &'static AValueRepr<T>) -> Self {
197+
Self::new_ptr(&x.header)
198+
}
199+
195200
pub(crate) fn new_ptr_usize(x: usize) -> Self {
196201
Self(Pointer::new_frozen_usize(x))
197202
}

0 commit comments

Comments
 (0)