Skip to content

Commit b61a59a

Browse files
JakobDegenmeta-codesync[bot]
authored andcommitted
lifetimes: access_owned_frozen_value in a couple places
Summary: Things that stopped compiling later in this stack because they weren't statically proving that the reference to the frozen heap had been added Reviewed By: cjhopman Differential Revision: D90475059 fbshipit-source-id: 17b3cc15209ef72c314ca47d452ea1952589dcfd
1 parent 77792dd commit b61a59a

File tree

4 files changed

+16
-7
lines changed

4 files changed

+16
-7
lines changed

starlark/src/assert/assert.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ static ASSERTS_STAR: Lazy<FrozenModule> = Lazy::new(|| {
5959
let g = GlobalsBuilder::new()
6060
.with_namespace("asserts", asserts_star)
6161
.build();
62-
Module::with_temp_heap(|m| {
63-
m.frozen_heap().add_reference(g.heap());
64-
let asserts = g.get("asserts").unwrap();
62+
Module::with_temp_heap(move |m| {
63+
let asserts = g.get_owned("asserts").unwrap();
64+
let asserts = m.heap().access_owned_frozen_value(&asserts);
6565
m.set("asserts", asserts);
6666
m.set(
6767
"freeze",

starlark/src/environment/globals.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ use crate::values::FrozenHeap;
4040
use crate::values::FrozenHeapRef;
4141
use crate::values::FrozenStringValue;
4242
use crate::values::FrozenValue;
43-
use crate::values::Value;
43+
use crate::values::OwnedFrozenValue;
4444
use crate::values::function::NativeFunc;
4545
use crate::values::function::NativeFuncFn;
4646
use crate::values::function::SpecialBuiltinFunction;
@@ -114,7 +114,8 @@ impl Globals {
114114

115115
/// This function is only safe if you first call `heap` and keep a reference to it.
116116
/// Therefore, don't expose it on the public API.
117-
pub(crate) fn get<'v>(&'v self, name: &str) -> Option<Value<'v>> {
117+
#[cfg(test)]
118+
pub(crate) fn get<'v>(&'v self, name: &str) -> Option<crate::values::Value<'v>> {
118119
self.get_frozen(name).map(FrozenValue::to_value)
119120
}
120121

@@ -124,6 +125,12 @@ impl Globals {
124125
self.0.variables.get_str(name).map(|x| x.value)
125126
}
126127

128+
pub(crate) fn get_owned(&self, name: &str) -> Option<OwnedFrozenValue> {
129+
let v = self.get_frozen(name)?;
130+
// SAFETY: We know the heap this is allocated in
131+
unsafe { Some(OwnedFrozenValue::new(self.heap().dupe(), v)) }
132+
}
133+
127134
/// Get all the names defined in this environment.
128135
pub fn names(&self) -> impl Iterator<Item = FrozenStringValue> + '_ {
129136
self.0.variable_names.iter().copied()

starlark/src/environment/modules.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,7 @@ impl Module {
535535
));
536536
}
537537
match module.get_any_visibility(symbol)? {
538-
(v, Visibility::Public) => Ok(v.owned_value(self.frozen_heap())),
538+
(v, Visibility::Public) => Ok(self.heap().access_owned_frozen_value(&v)),
539539
(_, Visibility::Private) => Err(crate::Error::new_other(
540540
EnvironmentError::ModuleSymbolIsNotExported(symbol.to_owned()),
541541
)),

starlark/src/tests/def.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,10 @@ value = {"test": "hello"}
172172
let f = m.get("function").unwrap();
173173
let x = m.get("value").unwrap();
174174
Module::with_temp_heap(|module| {
175+
let f = module.heap().access_owned_frozen_value(&f);
176+
let x = module.heap().access_owned_frozen_value(&x);
175177
let mut eval = Evaluator::new(&module);
176-
let res = eval.eval_function(f.value(), &[x.value()], &[]).unwrap();
178+
let res = eval.eval_function(f, &[x], &[]).unwrap();
177179
assert_eq!(res.to_str(), "hello");
178180
crate::Result::Ok(())
179181
})

0 commit comments

Comments
 (0)