Implement invoking starlark macros from GN Bug: 528225104 Change-Id: Iee8b19393637c98b491391af6ca0d6616a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/24804 Reviewed-by: Takuto Ikuta <tikuta@google.com> Commit-Queue: Matt Stark <msta@google.com>
diff --git a/build/gen.py b/build/gen.py index 80ca7e9..18f5779 100755 --- a/build/gen.py +++ b/build/gen.py
@@ -403,6 +403,9 @@ ninja_lines = [] def build_source(src_file, settings): + file_cflags = list(settings.get('cflags', cflags)) + if 'source_cflags' in settings and src_file in settings['source_cflags']: + file_cflags.extend(settings['source_cflags'][src_file]) ninja_lines.extend([ 'build %s: cxx %s' % (src_to_obj(src_file), escape_path_ninja( @@ -411,7 +414,7 @@ os.path.dirname(path)))), ' includes = %s' % ' '.join( ['-I' + escape_path_ninja(dirname) for dirname in include_dirs]), - ' cflags = %s' % ' '.join(settings.get('cflags', cflags)), + ' cflags = %s' % ' '.join(file_cflags), ]) for library, settings in static_libraries.items(): @@ -970,7 +973,13 @@ 'src/util/ticks.cc', 'src/util/worker_pool.cc', 'src/gn/test_with_scope.cc', - ] + ], + 'source_cflags': { + # cxxbridge generates helper templates in anonymous namespaces that + # may be unused depending on which bridge features are referenced. + # Note: -Wno-unused-template is unavailable on gcc + 'src/gn/ffi/bridge.cc': ['-Wno-unused-template'] if not is_gcc(cxx) and not platform.is_msvc() else [], + }, }, }
diff --git a/src/gn/ffi/bridge.cc b/src/gn/ffi/bridge.cc index a35ec04..2bef7a7 100644 --- a/src/gn/ffi/bridge.cc +++ b/src/gn/ffi/bridge.cc
@@ -23,6 +23,7 @@ #include <string> #include <type_traits> #include <utility> +#include <vector> #if __cplusplus >= 201703L #include <string_view> #endif @@ -765,12 +766,31 @@ }; namespace { +template <typename T> +void destroy(T *ptr) { + ptr->~T(); +} + template <bool> struct deleter_if { template <typename T> void operator()(T *) {} }; template <> struct deleter_if<true> { template <typename T> void operator()(T *ptr) { ptr->~T(); } }; + +template <typename T, bool = ::std::is_move_constructible<T>::value> +struct if_move_constructible { + static bool reserve(::std::vector<T> &, ::std::size_t) noexcept { + return false; + } +}; +template <typename T> +struct if_move_constructible<T, true> { + static bool reserve(::std::vector<T> &vec, ::std::size_t new_cap) { + vec.reserve(new_cap); + return true; + } +}; } // namespace } // namespace cxxbridge1 } // namespace rust @@ -873,6 +893,7 @@ ::rust::Box<::OwnedFrozenValue> clone() const noexcept; ::rust::String to_string() const noexcept; bool eq(::OwnedFrozenValue const &other) const noexcept; + void invoke(::Session const &session, ::std::vector<::Value> const &args, ::Scope const &kwargs, ::Value &out_val, ::Scope const &scope, ::ParseNodePtr origin, ::Err &err) const noexcept; ~OwnedFrozenValue() = delete; private: @@ -980,6 +1001,11 @@ return (self.*settings_cxx$)(); } +void cxxbridge1$196$Scope$package_cxx(::Scope const &self, ::SourceDir const **return$) noexcept { + ::SourceDir const &(::Scope::*package_cxx$)() const = &::Scope::GetSourceDir; + new (return$) ::SourceDir const *(&(self.*package_cxx$)()); +} + ::TestWithScope *cxxbridge1$196$NewTestWithScope() noexcept { ::std::unique_ptr<::TestWithScope> (*NewTestWithScope$)() = ::NewTestWithScope; return NewTestWithScope$().release(); @@ -1085,6 +1111,8 @@ void cxxbridge1$196$OwnedFrozenValue$to_string_cxx(::OwnedFrozenValue const &self, ::rust::String *return$) noexcept; bool cxxbridge1$196$OwnedFrozenValue$eq_cxx(::OwnedFrozenValue const &self, ::OwnedFrozenValue const &other) noexcept; + +void cxxbridge1$196$OwnedFrozenValue$invoke(::OwnedFrozenValue const &self, ::Session const &session, ::std::vector<::Value> const &args, ::Scope const &kwargs, ::Value &out_val, ::Scope const &scope, ::ParseNodePtr *origin, ::Err &err) noexcept; } // extern "C" ::std::size_t Session::layout::size() noexcept { @@ -1130,6 +1158,11 @@ return cxxbridge1$196$OwnedFrozenValue$eq_cxx(*this, other); } +void OwnedFrozenValue::invoke(::Session const &session, ::std::vector<::Value> const &args, ::Scope const &kwargs, ::Value &out_val, ::Scope const &scope, ::ParseNodePtr origin, ::Err &err) const noexcept { + ::rust::ManuallyDrop<::ParseNodePtr> origin$(::std::move(origin)); + cxxbridge1$196$OwnedFrozenValue$invoke(*this, session, args, kwargs, out_val, scope, &origin$.value, err); +} + extern "C" { static_assert(::rust::detail::is_complete<::std::remove_extent<::Err>::type>::value, "definition of `::Err` is required"); static_assert(sizeof(::std::unique_ptr<::Err>) == sizeof(void *), ""); @@ -1214,6 +1247,40 @@ ::Session *cxxbridge1$box$Session$alloc() noexcept; void cxxbridge1$box$Session$dealloc(::Session *) noexcept; void cxxbridge1$box$Session$drop(::rust::Box<::Session> *ptr) noexcept; + +::std::vector<::Value> *cxxbridge1$std$vector$Value$new() noexcept { + return new ::std::vector<::Value>(); +} +::std::size_t cxxbridge1$std$vector$Value$size(::std::vector<::Value> const &s) noexcept { + return s.size(); +} +::std::size_t cxxbridge1$std$vector$Value$capacity(::std::vector<::Value> const &s) noexcept { + return s.capacity(); +} +::Value *cxxbridge1$std$vector$Value$get_unchecked(::std::vector<::Value> *s, ::std::size_t pos) noexcept { + return &(*s)[pos]; +} +bool cxxbridge1$std$vector$Value$reserve(::std::vector<::Value> *s, ::std::size_t new_cap) noexcept { + return ::rust::if_move_constructible<::Value>::reserve(*s, new_cap); +} +static_assert(::rust::detail::is_complete<::std::remove_extent<::std::vector<::Value>>::type>::value, "definition of `::std::vector<::Value>` is required"); +static_assert(sizeof(::std::unique_ptr<::std::vector<::Value>>) == sizeof(void *), ""); +static_assert(alignof(::std::unique_ptr<::std::vector<::Value>>) == alignof(void *), ""); +void cxxbridge1$unique_ptr$std$vector$Value$null(::std::unique_ptr<::std::vector<::Value>> *ptr) noexcept { + ::new (ptr) ::std::unique_ptr<::std::vector<::Value>>(); +} +void cxxbridge1$unique_ptr$std$vector$Value$raw(::std::unique_ptr<::std::vector<::Value>> *ptr, ::std::unique_ptr<::std::vector<::Value>>::pointer raw) noexcept { + ::new (ptr) ::std::unique_ptr<::std::vector<::Value>>(raw); +} +::std::unique_ptr<::std::vector<::Value>>::element_type const *cxxbridge1$unique_ptr$std$vector$Value$get(::std::unique_ptr<::std::vector<::Value>> const &ptr) noexcept { + return ptr.get(); +} +::std::unique_ptr<::std::vector<::Value>>::pointer cxxbridge1$unique_ptr$std$vector$Value$release(::std::unique_ptr<::std::vector<::Value>> &ptr) noexcept { + return ptr.release(); +} +void cxxbridge1$unique_ptr$std$vector$Value$drop(::std::unique_ptr<::std::vector<::Value>> *ptr) noexcept { + ::rust::deleter_if<::rust::detail::is_complete<::std::vector<::Value>>::value>{}(ptr); +} } // extern "C" namespace rust {
diff --git a/src/gn/ffi/bridge.h b/src/gn/ffi/bridge.h index d49ba36..dbd322d 100644 --- a/src/gn/ffi/bridge.h +++ b/src/gn/ffi/bridge.h
@@ -24,6 +24,7 @@ #include <string> #include <type_traits> #include <utility> +#include <vector> #if __cplusplus >= 201703L #include <string_view> #endif @@ -830,6 +831,7 @@ ::rust::Box<::OwnedFrozenValue> clone() const noexcept; ::rust::String to_string() const noexcept; bool eq(::OwnedFrozenValue const &other) const noexcept; + void invoke(::Session const &session, ::std::vector<::Value> const &args, ::Scope const &kwargs, ::Value &out_val, ::Scope const &scope, ::ParseNodePtr origin, ::Err &err) const noexcept; ~OwnedFrozenValue() = delete; private:
diff --git a/src/gn/functions.cc b/src/gn/functions.cc index 10b41ce..4de707e 100644 --- a/src/gn/functions.cc +++ b/src/gn/functions.cc
@@ -754,6 +754,11 @@ session_load(loader, values[0], std::span(values).subspan(1), *scope, ParseNodePtr{function}, *err); + if (err->has_error()) { + err->AppendSubErr(Err(function, "whence 'load' was called.")); + return Value(); + } + return Value(); } @@ -1654,6 +1659,40 @@ FunctionInfoMap::const_iterator found_function = function_map.find(name.value()); if (found_function == function_map.end()) { + // Check if the function name matches a loaded Starlark macro. + const Value* val = scope->GetValue(name.value(), true); + if (val && val->type() == Value::STARLARK_VALUE) { + Value args = args_list->Execute(scope, err); + if (err->has_error()) + return Value(); + + std::unique_ptr<Scope> block_scope; + if (block) { + NonNestableBlock non_nestable(scope, function, "macro invocation"); + if (!non_nestable.Enter(err)) + return Value(); + + block_scope = std::make_unique<Scope>(scope); + block->Execute(block_scope.get(), err); + if (err->has_error()) + return Value(); + } else { + block_scope = std::make_unique<Scope>(scope); + } + + Value result; + val->starlark_value().invoke( + scope->settings()->build_settings()->starlark_session(), + args.list_value(), *block_scope, result, *scope, + ParseNodePtr{function}, *err); + + if (err->has_error()) { + err->AppendSubErr(Err(function, "whence '" + std::string(name.value()) + + "' was called.")); + return Value(); + } + return result; + } *err = Err(name, "Unknown function."); return Value(); }
diff --git a/src/gn/functions_unittest.cc b/src/gn/functions_unittest.cc index 31418ee..58af881 100644 --- a/src/gn/functions_unittest.cc +++ b/src/gn/functions_unittest.cc
@@ -12,8 +12,10 @@ #include "base/files/scoped_temp_dir.h" #include "gn/filesystem_utils.h" #include "gn/parse_tree.h" +#include "gn/scheduler.h" #include "gn/test_with_scope.h" #include "gn/value.h" +#include "util/msg_loop.h" #include "util/test/test.h" TEST(Functions, Assert) { @@ -715,6 +717,8 @@ } TEST(Functions, Load) { + MsgLoop run_loop; + Scheduler scheduler; TestWithScope setup; setup.build_settings()->SetRootPath(UTF8ToFilePath(".")); setup.scope()->set_source_dir(SourceDir("//")); @@ -753,34 +757,159 @@ my_rule = rule( implementation = my_rule_impl ) + hello = "hello" + +def sum(a, b, c): + return a + b + c + +def sum_wrapper(*args, **kwargs): + return sum(*args, **kwargs) + )scl"; base::FilePath scl_path = temp_dir.GetPath().AppendASCII("rules.scl"); ASSERT_EQ( static_cast<int>(scl_content.size()), base::WriteFile(scl_path, scl_content.c_str(), scl_content.size())); - TestParseInput input(R"gn( -load("//:rules.scl", "hello", "my_rule", "my_rule_impl") + { + TestParseInput input(R"gn( +load("//:rules.scl", "hello", "my_rule", "my_rule_impl", "sum") assert(my_rule == my_rule) assert(my_rule != my_rule_impl) assert("${my_rule}" == "<rule: my_rule>") copy_of_rule = my_rule assert(copy_of_rule == my_rule) +assert(sum(1, 2, 3) == 6) +assert(sum(1, 2, 3) {} == 6) +assert(sum(1, 2) { + c = 3 +} == 6) +assert(sum() { + a = "a" + b = "b" + c = "c" +} == "abc") )gn"); - ASSERT_SUCCESS(input); - Err err; - input.parsed()->Execute(setup.scope(), &err); - ASSERT_FALSE(err.has_error()) << err.message(); + ASSERT_SUCCESS(input); + Err err; + input.parsed()->Execute(setup.scope(), &err); + ASSERT_FALSE(err.has_error()) << err.message(); - const Value* val_hello = setup.scope()->GetValue("hello"); - ASSERT_TRUE(val_hello); - EXPECT_EQ(Value::STRING, val_hello->type()); - EXPECT_EQ("hello", val_hello->string_value()); + const Value* val_hello = setup.scope()->GetValue("hello"); + ASSERT_TRUE(val_hello); + EXPECT_EQ(Value::STRING, val_hello->type()); + EXPECT_EQ("hello", val_hello->string_value()); - const Value* val_my_rule = setup.scope()->GetValue("my_rule"); - ASSERT_TRUE(val_my_rule); - EXPECT_EQ(Value::STARLARK_VALUE, val_my_rule->type()); + const Value* val_my_rule = setup.scope()->GetValue("my_rule"); + ASSERT_TRUE(val_my_rule); + EXPECT_EQ(Value::STARLARK_VALUE, val_my_rule->type()); + } + + // Verify failure when loading a nonexistent variable. + { + TestParseInput fail_input(R"gn( +load("//:rules.scl", "nonexistent") +)gn"); + ASSERT_SUCCESS(fail_input); + Err err; + fail_input.parsed()->Execute(setup.scope(), &err); + ASSERT_TRUE(err.has_error()); + EXPECT_EQ( + "ERROR Key 'nonexistent' not found in module '//:rules.scl'\n" + "See //test:2:1: whence 'load' was called.\n" + "load(\"//:rules.scl\", \"nonexistent\")\n" + "^---------------------------------\n", + err.to_string()); + } + + // Verify failure when calling Starlark function with insufficient + // arguments. + { + TestParseInput fail_input(R"gn( +load("//:rules.scl", "sum") +sum(1, 2) +)gn"); + ASSERT_SUCCESS(fail_input); + Err err; + fail_input.parsed()->Execute(setup.scope(), &err); + ASSERT_TRUE(err.has_error()); + EXPECT_EQ( + "ERROR Missing parameter `c` for call to `//:rules.scl.sum`\n" + "See //test:3:1: whence 'sum' was called.\n" + "sum(1, 2)\n" + "^-------\n", + err.to_string()); + } + + // Verify failure when calling starlark function with too many arguments. + { + TestParseInput fail_input(R"gn( +load("//:rules.scl", "sum") +sum(1, 2, 3) { + d = 4 +} +)gn"); + ASSERT_SUCCESS(fail_input); + Err err; + fail_input.parsed()->Execute(setup.scope(), &err); + ASSERT_TRUE(err.has_error()); + EXPECT_EQ( + "ERROR Found `d` extra named parameter(s) for call to " + "//:rules.scl.sum\n" + "See //test:3:1: whence 'sum' was called.\n" + "sum(1, 2, 3) {\n" + "^-------------\n", + err.to_string()); + } + + // Verify failure when the starlark function itself returns an error. + { + TestParseInput fail_input(R"gn( +load("//:rules.scl", "sum_wrapper") +sum_wrapper(1, 2, 3) { + d = 4 +} +)gn"); + ASSERT_SUCCESS(fail_input); + Err err; + fail_input.parsed()->Execute(setup.scope(), &err); + ASSERT_TRUE(err.has_error()); + EXPECT_EQ( + "ERROR at //:rules.scl:15:10: Found `d` extra named parameter(s) for " + "call to //:rules.scl.sum\n" + " return sum(*args, **kwargs)\n" + " ^-------------------\n" + "See //test:3:1: whence 'sum_wrapper' was called.\n" + "sum_wrapper(1, 2, 3) {\n" + "^---------------------\n", + err.to_string()); + } + + // Verify failure when trying to declare a target inside a macro invocation + // block. + { + TestParseInput fail_input(R"gn( +load("//:rules.scl", "sum") +sum(1, 2) { + executable("nested") { + } +} +)gn"); + ASSERT_SUCCESS(fail_input); + Err err; + fail_input.parsed()->Execute(setup.scope(), &err); + ASSERT_TRUE(err.has_error()); + EXPECT_EQ( + "ERROR at //test:4:3: Can't nest these things.\n" + " executable(\"nested\") {\n" + " ^---------------------\n" + "You are trying to nest a target inside a macro invocation.\n" + "See //test:3:1: The enclosing block.\n" + "sum(1, 2) {\n" + "^----------\n", + err.to_string()); + } } }
diff --git a/src/gn/starlark/crates/ffi/src/bridge.rs b/src/gn/starlark/crates/ffi/src/bridge.rs index 3fb3d5b..1c8bf00 100644 --- a/src/gn/starlark/crates/ffi/src/bridge.rs +++ b/src/gn/starlark/crates/ffi/src/bridge.rs
@@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +use types::EvaluatorContextExt as _; + /// The consolidated cxx FFI bridge defining all shared C++ classes, structs, /// methods, and constructors utilized by the high-level Rust wrappers. /// @@ -28,6 +30,57 @@ pub fn eq_cxx(&self, other: &Self) -> bool { self.0.value() == other.0.value() } + + pub fn invoke( + &self, + session: &'static Session, + args: &cxx::CxxVector<Value>, + kwargs: &Scope, + mut out_val: std::pin::Pin<&mut Value>, + scope: &'static Scope, + origin: ParseNodePtr, + mut err: std::pin::Pin<&mut Err>, + ) { + // Safety: `self` (and thus self.0.owner()) is guaranteed to outlive + // the temp module and thus this cannot be GC'd during this call. + let func_val = unsafe { self.0.unchecked_frozen_value() }; + err.as_mut().handle((|| { + let val = starlark::environment::Module::with_temp_heap( + |module| -> starlark::Result<Self> { + let heap = module.heap(); + let args: Vec<_> = args.iter().map(|arg| arg.to_rust(&heap)).collect(); + let kwargs: Vec<_> = kwargs + .items() + .as_slice() + .iter() + .map(|kw| (kw.key, kw.value.to_rust(&heap))) + .collect(); + + let package = scope.package(); + let eval_context = + crate::eval_context::EvalContext::new_macro(session, package, scope); + + let res = { + let mut eval = starlark::eval::Evaluator::new(&module); + eval.set_context(&eval_context); + eval.eval_function(func_val.to_value(), &args, &kwargs) + }; + + module.set_extra_value(res?); + let frozen_module = module.freeze().map_err(starlark::Error::new_other)?; + Ok(Self(frozen_module.owned_extra_value().unwrap())) + }, + )?; + + out_val.as_mut().assign( + val.0.value(), + Some(val.0.owner()), + scope.settings(), + origin, + )?; + Ok(()) + })()); + } } #[cxx::bridge] @@ -163,6 +216,8 @@ ) -> Pin<&'a mut Value>; #[rust_name = "settings_cxx"] pub(in crate::scope) fn settings(self: &Scope) -> *const Settings; + #[cxx_name = "GetSourceDir"] + pub(in crate::scope) fn package_cxx(self: &Scope) -> &SourceDir; type TestWithScope; pub(in crate::test_with_scope) fn NewTestWithScope() -> UniquePtr<TestWithScope>; @@ -239,6 +294,16 @@ fn to_string(self: &OwnedFrozenValue) -> String; #[rust_name = "eq_cxx"] fn eq(self: &OwnedFrozenValue, other: &OwnedFrozenValue) -> bool; + fn invoke( + self: &OwnedFrozenValue, + session: &'static Session, + args: &CxxVector<Value>, + kwargs: &Scope, + out_val: Pin<&mut Value>, + scope: &'static Scope, + origin: ParseNodePtr, + err: Pin<&mut Err>, + ); } }
diff --git a/src/gn/starlark/crates/ffi/src/err.rs b/src/gn/starlark/crates/ffi/src/err.rs index 1716681..3be3982 100644 --- a/src/gn/starlark/crates/ffi/src/err.rs +++ b/src/gn/starlark/crates/ffi/src/err.rs
@@ -66,6 +66,9 @@ fn fill_frames(mut self: Pin<&mut Self>, err: &starlark::Error) { // Process call stack frames and append them as nested errors. for frame in err.call_stack().frames.iter().rev() { + if err.span() == frame.location.as_ref() { + continue; + } if let Some(loc) = &frame.location { let frame_filename = loc.filename(); let frame_source = loc.file.source();
diff --git a/src/gn/starlark/crates/ffi/src/errors.rs b/src/gn/starlark/crates/ffi/src/errors.rs index d3b1255..9877159 100644 --- a/src/gn/starlark/crates/ffi/src/errors.rs +++ b/src/gn/starlark/crates/ffi/src/errors.rs
@@ -13,6 +13,8 @@ PassingNonFrozenStarlarkValueToGn(String), #[error("This is not allowed while executing a function loaded from a starlark file.")] RequiresBzlFile, + #[error("This is only allowed while executing a macro (function called from GN).")] + RequiresMacro, } impl From<Error> for starlark::Error {
diff --git a/src/gn/starlark/crates/ffi/src/eval_context.rs b/src/gn/starlark/crates/ffi/src/eval_context.rs index 1bd38aa..5a1a604 100644 --- a/src/gn/starlark/crates/ffi/src/eval_context.rs +++ b/src/gn/starlark/crates/ffi/src/eval_context.rs
@@ -6,11 +6,11 @@ use starlark::values::ProvidesStaticType; use types::{LabelRef, PackageRef, PathResolver}; -use crate::errors::Error; +use crate::{errors::Error, Scope}; -#[derive(Allocative)] enum EvalContextKind { BzlFile, + Macro(&'static Scope), } #[derive(Allocative, ProvidesStaticType)] @@ -19,6 +19,7 @@ session: &'static crate::session::Session, #[allocative(skip)] package: &'static PackageRef, + #[allocative(skip)] kind: EvalContextKind, } @@ -33,6 +34,18 @@ kind: EvalContextKind::BzlFile, } } + + pub fn new_macro( + session: &'static crate::session::Session, + package: &'static PackageRef, + scope: &'static Scope, + ) -> Self { + Self { + session, + package, + kind: EvalContextKind::Macro(scope), + } + } } impl types::EvalContext for EvalContext { @@ -56,7 +69,10 @@ } fn require_macro(&self) -> starlark::Result<&Self::Scope> { - todo!() + match &self.kind { + EvalContextKind::Macro(scope) => Ok(*scope), + _ => Err(Error::RequiresMacro.into()), + } } fn require_bzl(&self) -> starlark::Result<()> { @@ -77,7 +93,7 @@ &self, _target_type: Option<types::OutputType>, _target_name: &str, - _scope: &Self::Scope, + _scope: &Scope, _rule: starlark::values::FrozenValue, _attrs: Vec<attr::Attr>, ) -> starlark::Result<<Self::Session as types::Session>::TargetRef> {
diff --git a/src/gn/starlark/crates/ffi/src/scope.rs b/src/gn/starlark/crates/ffi/src/scope.rs index e485517..2555b87 100644 --- a/src/gn/starlark/crates/ffi/src/scope.rs +++ b/src/gn/starlark/crates/ffi/src/scope.rs
@@ -5,7 +5,7 @@ use std::pin::Pin; use starlark::values::{Heap, Value as StarlarkValue}; -use types::intern_string; +use types::{intern_string, PackageRef}; use crate::{bridge::Value, Immutable, OwnedSlice, Scope}; @@ -49,6 +49,10 @@ } items } + + pub fn package(&self) -> &PackageRef { + self.package_cxx().as_rust().unwrap() + } } impl types::Scope for Scope {