Split into two kinds of scopes - scopes and structs. Semantically, a "struct" is just a scope which only supports dot-access. It has no parent scope. On the other hand, a Scope has a parent, and inherits variables from it. Bug: 528225104 Change-Id: I6410068cd8e89b8ae747f1fc2583b9e16a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/24541 Reviewed-by: Richard Wang <richardwa@google.com> Commit-Queue: Matt Stark <msta@google.com> Reviewed-by: Takuto Ikuta <tikuta@google.com>
diff --git a/src/gn/ffi/bridge.cc b/src/gn/ffi/bridge.cc index 63b4b22..5b94043 100644 --- a/src/gn/ffi/bridge.cc +++ b/src/gn/ffi/bridge.cc
@@ -721,6 +721,11 @@ new (return$) ::SliceAny(NewScope$(parent_scope, keys, out_scope)); } +void cxxbridge1$196$NewStruct(::Settings const &settings, ::rust::Slice<::rust::Str const> keys, ::std::unique_ptr<::Scope> &out_scope, ::SliceAny *return$) noexcept { + ::SliceAny (*NewStruct$)(::Settings const &, ::rust::Slice<::rust::Str const>, ::std::unique_ptr<::Scope> &) = ::NewStruct; + new (return$) ::SliceAny(NewStruct$(settings, keys, out_scope)); +} + void cxxbridge1$196$GetScopeItems(::Scope const &scope, ::SliceAny *return$) noexcept { ::SliceAny (*GetScopeItems$)(::Scope const &) = ::GetScopeItems; new (return$) ::SliceAny(GetScopeItems$(scope));
diff --git a/src/gn/ffi/scope.cc b/src/gn/ffi/scope.cc index b564226..6b3ed4d 100644 --- a/src/gn/ffi/scope.cc +++ b/src/gn/ffi/scope.cc
@@ -16,22 +16,34 @@ std::unique_ptr<Scope>& out_scope) { auto new_scope = std::make_unique<Scope>(&parent_scope); new_scope->set_source_dir(parent_scope.GetSourceDir()); - // We detach because GN always detaches when result_mode of the parse tree is - // RETURNS_SCOPE (which is what starlark creates new scopes for). - new_scope->DetachFromContaining(); std::vector<Value*> placeholders; placeholders.reserve(keys.size()); for (const auto& key : keys) { - std::string_view key_sv(key.data(), key.size()); - Value* val = new_scope->SetValue(key_sv, Value(), nullptr); - placeholders.push_back(val); + placeholders.push_back( + new_scope->SetValue(std::string_view(key), Value(), nullptr)); } out_scope = std::move(new_scope); return IntoSlice(std::move(placeholders)); } +SliceAny NewStruct(const Settings& settings, + rust::Slice<const rust::Str> keys, + std::unique_ptr<Scope>& out) { + out = std::make_unique<Scope>(&settings); + + std::vector<Value*> placeholders; + placeholders.reserve(keys.size()); + for (const auto& key : keys) { + placeholders.push_back( + out->SetValue(std::string_view(key), Value(), nullptr)); + } + // Not all fields in a struct need to be used. + out->MarkAllUsed(); + return IntoSlice(std::move(placeholders)); +} + // Unlike regular GN scoping rules, this does not extract from variables defined // in outer scopes. This is because starlark treats scopes as equivalent to // "struct" objects, and as the **kwargs to pass to functions. Thus, accessing
diff --git a/src/gn/ffi/scope.h b/src/gn/ffi/scope.h index fd018b0..c62af06 100644 --- a/src/gn/ffi/scope.h +++ b/src/gn/ffi/scope.h
@@ -10,8 +10,9 @@ #include "cxx.h" class Scope; -class Value; +class Settings; struct SliceAny; +class Value; // Constructs a new child Scope, populates placeholder Values for the given // keys, and returns a "std::vector<Value&>" where vec[i] is the value for @@ -22,6 +23,19 @@ rust::Slice<const rust::Str> keys, std::unique_ptr<Scope>& out_scope); +// Constructs a new "struct", populates placeholder Values for the given +// keys, and returns a "std::vector<Value&>" where vec[i] is the value for +// keys[i]. +// +// A "struct" is a scope which can only be used for dot-lookup. It has no +// parent scope, as that would allow struct.foo to lookup the parent +// scope's foo. +// +// Safety: Rust is required to convert this to an OwnedSlice<&Value>. +SliceAny NewStruct(const Settings& settings, + rust::Slice<const rust::Str> keys, + std::unique_ptr<Scope>& out_scope); + // Returns a "std::vector<KeyValue>"-like object. // // Safety: Rust is required to convert this to an OwnedSlice<KeyValue>.
diff --git a/src/gn/starlark/crates/ffi/src/bridge.rs b/src/gn/starlark/crates/ffi/src/bridge.rs index 337265a..396ed42 100644 --- a/src/gn/starlark/crates/ffi/src/bridge.rs +++ b/src/gn/starlark/crates/ffi/src/bridge.rs
@@ -124,6 +124,11 @@ keys: &[&str], out_scope: &mut UniquePtr<Scope>, ) -> SliceAny; + pub(in crate::scope) fn NewStruct( + settings: &Settings, + keys: &[&str], + out_scope: &mut UniquePtr<Scope>, + ) -> SliceAny; // Returns an OwnedSlice<KeyValue> corresponding to references to each element. pub(in crate::scope) fn GetScopeItems(scope: &Scope) -> SliceAny; pub(in crate::scope) fn GetValue(scope: &Scope, ident: &str) -> *const Value;
diff --git a/src/gn/starlark/crates/ffi/src/scope.rs b/src/gn/starlark/crates/ffi/src/scope.rs index 532346e..09d2682 100644 --- a/src/gn/starlark/crates/ffi/src/scope.rs +++ b/src/gn/starlark/crates/ffi/src/scope.rs
@@ -18,6 +18,15 @@ (nested_scope, values.into()) } + pub(crate) fn new_struct<'b>( + settings: &crate::Settings, + keys: &[&str], + ) -> (cxx::UniquePtr<Self>, OwnedSlice<Pin<&'b mut Value>>) { + let mut nested_scope = cxx::UniquePtr::<Self>::null(); + let values = crate::bridge::NewStruct(settings, keys, &mut nested_scope); + (nested_scope, values.into()) + } + /// Returns the settings for the given scope. pub fn settings(&self) -> &crate::Settings { // Safety: Settings pointer is always valid and non-null. @@ -49,12 +58,11 @@ let (keys, vals): (Vec<&str>, Vec<StarlarkValue<'v>>) = kv.unzip(); let (mut child_scope, mut placeholders) = Scope::new(parent, &keys); - // Safety: The child scope will always be non-null. - let child_ref = unsafe { child_scope.as_mut().unwrap().get_unchecked_mut() }; + let child_pin = child_scope.as_mut().unwrap(); for (placeholder, val) in placeholders.as_slice_mut().iter_mut().zip(vals) { placeholder .as_mut() - .assign(val, child_ref, Default::default()); + .assign(val, child_pin.settings(), Default::default()); } Self(child_scope) @@ -81,7 +89,7 @@ let mut setup = TestWithScope::new(); let parent_scope = setup.scope(); - let (child_ptr, _) = Scope::new(parent_scope, &[]); + let (child_ptr, _) = Scope::new(&*parent_scope, &[]); let owned_scope = OwnedScope(child_ptr); starlark::environment::Module::with_temp_heap(|module| {
diff --git a/src/gn/starlark/crates/ffi/src/value.rs b/src/gn/starlark/crates/ffi/src/value.rs index dd970e4..6ef19f0 100644 --- a/src/gn/starlark/crates/ffi/src/value.rs +++ b/src/gn/starlark/crates/ffi/src/value.rs
@@ -8,7 +8,7 @@ use crate::{ bridge::{SliceAny, Value, ValueType}, - Immutable, Scope, Slice, + Immutable, Scope, Settings, Slice, }; impl Value { @@ -41,7 +41,7 @@ pub fn assign<'v>( mut self: Pin<&mut Self>, val: starlark::values::Value<'v>, - scope: &mut Scope, + settings: &Settings, origin: crate::bridge::ParseNodePtr, ) { if val.is_none() { @@ -59,17 +59,17 @@ } .into(); for (el_pin, src) in slice.iter_mut().zip(l.iter()) { - el_pin.assign(src, scope, origin); + el_pin.assign(src, settings, origin); } } else if let Some(s) = StructRef::from_value(val) { let keys: Vec<&str> = s.iter().map(|(k, _)| k.as_str()).collect(); - let (nested_scope, mut values) = Scope::new(scope, &keys); + let (r#struct, mut values) = Scope::new_struct(settings, &keys); for (v_starlark, v_cxx) in s.iter().map(|(_, v)| v).zip(values.as_slice_mut()) { - v_cxx.as_mut().assign(v_starlark, scope, origin); + v_cxx.as_mut().assign(v_starlark, settings, origin); } - crate::bridge::SetValueScope(self.as_mut(), origin, nested_scope); + crate::bridge::SetValueScope(self.as_mut(), origin, r#struct); } else { todo!("Arbitrary starlark values not (yet) supported"); } @@ -93,7 +93,7 @@ let mut value = crate::bridge::NewValueForTesting(); value.pin_mut().assign( val, - scope, + scope.settings(), crate::bridge::ParseNodePtr { ptr: std::ptr::null(), }, @@ -195,4 +195,44 @@ assert_eq!(get_field("bar").unwrap().unpack_str(), Some("baz")); }); } + + #[test] + fn test_nested_struct_conversion() { + starlark::environment::Module::with_temp_heap(|module| { + let heap = module.heap(); + let inner_struct = + starlark::values::structs::AllocStruct(vec![("inner_foo", heap.alloc(42))]); + let struct_ref = StructRef::from_value(back_and_forth( + &heap, + heap.alloc(starlark::values::structs::AllocStruct(vec![ + ("outer_foo", heap.alloc(100)), + ("nested", heap.alloc(inner_struct)), + ])) + .to_value(), + )) + .unwrap(); + + let outer_foo = struct_ref + .iter() + .find(|(k, _)| k.as_str() == "outer_foo") + .map(|(_, v)| v) + .unwrap() + .unpack_i32(); + assert_eq!(outer_foo, Some(100)); + + let nested_val = struct_ref + .iter() + .find(|(k, _)| k.as_str() == "nested") + .map(|(_, v)| v) + .unwrap(); + let nested_ref = StructRef::from_value(nested_val).unwrap(); + let inner_foo = nested_ref + .iter() + .find(|(k, _)| k.as_str() == "inner_foo") + .map(|(_, v)| v) + .unwrap() + .unpack_i32(); + assert_eq!(inner_foo, Some(42)); + }); + } }