Support converting starlark::Error to gn's Err type. Bug: 528225104 Change-Id: I00102e3b74f9dcc4f6bf55692e4a8d716a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/24400 Commit-Queue: Matt Stark <msta@google.com> Reviewed-by: Takuto Ikuta <tikuta@google.com> Reviewed-by: Richard Wang <richardwa@google.com>
diff --git a/src/gn/ffi/bridge.cc b/src/gn/ffi/bridge.cc index 792758f..df8eb27 100644 --- a/src/gn/ffi/bridge.cc +++ b/src/gn/ffi/bridge.cc
@@ -1,5 +1,7 @@ // This file is generated by src/gn/ffi/update_bridge.sh. Do not edit manually. // Source: src/gn/ffi/starlark/crates/ffi/src/bridge.rs +#include "gn/err.h" +#include "gn/ffi/err.h" #include "gn/ffi/scope.h" #include "gn/ffi/test_with_scope.h" #include "gn/ffi/value.h" @@ -45,12 +47,13 @@ void panic [[noreturn]](const char *msg); #endif // CXXBRIDGE1_PANIC +struct unsafe_bitcopy_t; + namespace { template <typename T> class impl; } // namespace -class String; class Opaque; template <typename T> @@ -58,6 +61,77 @@ template <typename T> ::std::size_t align_of(); +#ifndef CXXBRIDGE1_RUST_STRING +#define CXXBRIDGE1_RUST_STRING +class String final { +public: + String() noexcept; + String(const String &) noexcept; + String(String &&) noexcept; + ~String() noexcept; + + String(const std::string &); + String(const char *); + String(const char *, std::size_t); + String(const char16_t *); + String(const char16_t *, std::size_t); +#ifdef __cpp_char8_t + String(const char8_t *s); + String(const char8_t *s, std::size_t len); +#endif + + static String lossy(const std::string &) noexcept; + static String lossy(const char *) noexcept; + static String lossy(const char *, std::size_t) noexcept; + static String lossy(const char16_t *) noexcept; + static String lossy(const char16_t *, std::size_t) noexcept; + + String &operator=(const String &) & noexcept; + String &operator=(String &&) & noexcept; + + explicit operator std::string() const; + + const char *data() const noexcept; + std::size_t size() const noexcept; + std::size_t length() const noexcept; + bool empty() const noexcept; + + const char *c_str() noexcept; + + std::size_t capacity() const noexcept; + void reserve(size_t new_cap) noexcept; + + using iterator = char *; + iterator begin() noexcept; + iterator end() noexcept; + + using const_iterator = const char *; + const_iterator begin() const noexcept; + const_iterator end() const noexcept; + const_iterator cbegin() const noexcept; + const_iterator cend() const noexcept; + + bool operator==(const String &) const noexcept; + bool operator!=(const String &) const noexcept; + bool operator<(const String &) const noexcept; + bool operator<=(const String &) const noexcept; + bool operator>(const String &) const noexcept; + bool operator>=(const String &) const noexcept; + + void swap(String &) noexcept; + + String(unsafe_bitcopy_t, const String &) noexcept; + +private: + struct lossy_t; + String(lossy_t, const char *, std::size_t) noexcept; + String(lossy_t, const char16_t *, std::size_t) noexcept; + friend void swap(String &lhs, String &rhs) noexcept { lhs.swap(rhs); } + + std::array<std::uintptr_t, 3> repr; +}; +#endif // CXXBRIDGE1_RUST_STRING + #ifndef CXXBRIDGE1_RUST_STR #define CXXBRIDGE1_RUST_STR class Str final { @@ -519,6 +593,8 @@ struct SliceAny; struct KeyValue; enum class ValueType : ::std::uint8_t; +using Err = ::Err; +using InputFile = ::InputFile; using OutputFile = ::OutputFile; using SourceDir = ::SourceDir; using Label = ::Label; @@ -570,6 +646,41 @@ #endif // CXXBRIDGE1_ENUM_ValueType extern "C" { +bool cxxbridge1$196$Err$has_error(::Err const &self) noexcept { + bool (::Err::*has_error$)() const = &::Err::has_error; + return (self.*has_error$)(); +} + +::Err *cxxbridge1$196$NewErr() noexcept { + ::std::unique_ptr<::Err> (*NewErr$)() = ::NewErr; + return NewErr$().release(); +} + +void cxxbridge1$196$ErrToString(::Err const &err, ::rust::String *return$) noexcept { + ::rust::String (*ErrToString$)(::Err const &) = ::ErrToString; + new (return$) ::rust::String(ErrToString$(err)); +} + +void cxxbridge1$196$PopulateErrWithLocation(::Err &err, ::rust::Str message, ::rust::Str help, ::InputFile const &file, ::std::int32_t start_line, ::std::int32_t start_column, ::std::int32_t end_line, ::std::int32_t end_column) noexcept { + void (*PopulateErrWithLocation$)(::Err &, ::rust::Str, ::rust::Str, ::InputFile const &, ::std::int32_t, ::std::int32_t, ::std::int32_t, ::std::int32_t) = ::PopulateErrWithLocation; + PopulateErrWithLocation$(err, message, help, file, start_line, start_column, end_line, end_column); +} + +void cxxbridge1$196$PopulateErrWithMessage(::Err &err, ::rust::Str message, ::rust::Str help) noexcept { + void (*PopulateErrWithMessage$)(::Err &, ::rust::Str, ::rust::Str) = ::PopulateErrWithMessage; + PopulateErrWithMessage$(err, message, help); +} + +void cxxbridge1$196$AppendSubErr(::Err &err, ::rust::Str message, ::InputFile const &file, ::std::int32_t start_line, ::std::int32_t start_column, ::std::int32_t end_line, ::std::int32_t end_column) noexcept { + void (*AppendSubErr$)(::Err &, ::rust::Str, ::InputFile const &, ::std::int32_t, ::std::int32_t, ::std::int32_t, ::std::int32_t) = ::AppendSubErr; + AppendSubErr$(err, message, file, start_line, start_column, end_line, end_column); +} + +::InputFile const *cxxbridge1$196$NewInputFile(::rust::Str name, ::rust::Str code) noexcept { + ::InputFile const &(*NewInputFile$)(::rust::Str, ::rust::Str) = ::NewInputFile; + return &NewInputFile$(name, code); +} + void cxxbridge1$196$OutputFile$value(::OutputFile const &self, ::rust::Str *return$) noexcept { std::string_view (::OutputFile::*value$)() const = &::OutputFile::value; new (return$) ::rust::Str(::rust::cxx_to_rust((self.*value$)())); @@ -695,6 +806,25 @@ SetValueScope$(val, origin, ::std::unique_ptr<::Scope>(scope)); } +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 *), ""); +static_assert(alignof(::std::unique_ptr<::Err>) == alignof(void *), ""); +void cxxbridge1$unique_ptr$Err$null(::std::unique_ptr<::Err> *ptr) noexcept { + ::new (ptr) ::std::unique_ptr<::Err>(); +} +void cxxbridge1$unique_ptr$Err$raw(::std::unique_ptr<::Err> *ptr, ::std::unique_ptr<::Err>::pointer raw) noexcept { + ::new (ptr) ::std::unique_ptr<::Err>(raw); +} +::std::unique_ptr<::Err>::element_type const *cxxbridge1$unique_ptr$Err$get(::std::unique_ptr<::Err> const &ptr) noexcept { + return ptr.get(); +} +::std::unique_ptr<::Err>::pointer cxxbridge1$unique_ptr$Err$release(::std::unique_ptr<::Err> &ptr) noexcept { + return ptr.release(); +} +void cxxbridge1$unique_ptr$Err$drop(::std::unique_ptr<::Err> *ptr) noexcept { + ::rust::deleter_if<::rust::detail::is_complete<::Err>::value>{}(ptr); +} + static_assert(::rust::detail::is_complete<::std::remove_extent<::Scope>::type>::value, "definition of `::Scope` is required"); static_assert(sizeof(::std::unique_ptr<::Scope>) == sizeof(void *), ""); static_assert(alignof(::std::unique_ptr<::Scope>) == alignof(void *), "");
diff --git a/src/gn/ffi/bridge.h b/src/gn/ffi/bridge.h index 7ec8e6e..fe645f9 100644 --- a/src/gn/ffi/bridge.h +++ b/src/gn/ffi/bridge.h
@@ -1,6 +1,8 @@ // This file is generated by src/gn/ffi/update_bridge.sh. Do not edit manually. // Source: src/gn/ffi/starlark/crates/ffi/src/bridge.rs #pragma once +#include "gn/err.h" +#include "gn/ffi/err.h" #include "gn/ffi/scope.h" #include "gn/ffi/test_with_scope.h" #include "gn/ffi/value.h" @@ -37,12 +39,13 @@ void panic [[noreturn]](const char *msg); #endif // CXXBRIDGE1_PANIC +struct unsafe_bitcopy_t; + namespace { template <typename T> class impl; } // namespace -class String; class Opaque; template <typename T> @@ -50,6 +53,77 @@ template <typename T> ::std::size_t align_of(); +#ifndef CXXBRIDGE1_RUST_STRING +#define CXXBRIDGE1_RUST_STRING +class String final { +public: + String() noexcept; + String(const String &) noexcept; + String(String &&) noexcept; + ~String() noexcept; + + String(const std::string &); + String(const char *); + String(const char *, std::size_t); + String(const char16_t *); + String(const char16_t *, std::size_t); +#ifdef __cpp_char8_t + String(const char8_t *s); + String(const char8_t *s, std::size_t len); +#endif + + static String lossy(const std::string &) noexcept; + static String lossy(const char *) noexcept; + static String lossy(const char *, std::size_t) noexcept; + static String lossy(const char16_t *) noexcept; + static String lossy(const char16_t *, std::size_t) noexcept; + + String &operator=(const String &) & noexcept; + String &operator=(String &&) & noexcept; + + explicit operator std::string() const; + + const char *data() const noexcept; + std::size_t size() const noexcept; + std::size_t length() const noexcept; + bool empty() const noexcept; + + const char *c_str() noexcept; + + std::size_t capacity() const noexcept; + void reserve(size_t new_cap) noexcept; + + using iterator = char *; + iterator begin() noexcept; + iterator end() noexcept; + + using const_iterator = const char *; + const_iterator begin() const noexcept; + const_iterator end() const noexcept; + const_iterator cbegin() const noexcept; + const_iterator cend() const noexcept; + + bool operator==(const String &) const noexcept; + bool operator!=(const String &) const noexcept; + bool operator<(const String &) const noexcept; + bool operator<=(const String &) const noexcept; + bool operator>(const String &) const noexcept; + bool operator>=(const String &) const noexcept; + + void swap(String &) noexcept; + + String(unsafe_bitcopy_t, const String &) noexcept; + +private: + struct lossy_t; + String(lossy_t, const char *, std::size_t) noexcept; + String(lossy_t, const char16_t *, std::size_t) noexcept; + friend void swap(String &lhs, String &rhs) noexcept { lhs.swap(rhs); } + + std::array<std::uintptr_t, 3> repr; +}; +#endif // CXXBRIDGE1_RUST_STRING + #ifndef CXXBRIDGE1_RUST_STR #define CXXBRIDGE1_RUST_STR class Str final { @@ -502,6 +576,8 @@ struct SliceAny; struct KeyValue; enum class ValueType : ::std::uint8_t; +using Err = ::Err; +using InputFile = ::InputFile; using OutputFile = ::OutputFile; using SourceDir = ::SourceDir; using Label = ::Label;
diff --git a/src/gn/ffi/err.h b/src/gn/ffi/err.h new file mode 100644 index 0000000..87abac2 --- /dev/null +++ b/src/gn/ffi/err.h
@@ -0,0 +1,75 @@ +// Copyright 2026 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef TOOLS_GN_FFI_ERR_H_ +#define TOOLS_GN_FFI_ERR_H_ + +#include <memory> +#include <string> +#include <vector> + +#include "cxx.h" +#include "gn/err.h" +#include "gn/input_file.h" +#include "gn/input_file_manager.h" +#include "gn/scheduler.h" +#include "gn/source_file.h" + +inline const InputFile& NewInputFile(rust::Str name, rust::Str code) { + // Yes, the same file can be created multiple times. + // No, we don't really care, since this is only for error messages. + SourceFile source_file{std::string(name)}; + InputFileManager* manager = g_scheduler->input_file_manager(); + + InputFile* file = nullptr; + std::vector<Token>* tokens = nullptr; + std::unique_ptr<ParseNode>* parse_root = nullptr; + manager->AddDynamicInput(source_file, &file, &tokens, &parse_root); + file->SetContents(std::string(code)); + file->set_friendly_name(std::string(name)); + return *file; +} + +inline void PopulateErrWithLocation(Err& err, + rust::Str message, + rust::Str help, + const InputFile& file, + int start_line, + int start_column, + int end_line, + int end_column) { + Location begin(&file, start_line, start_column); + Location end(&file, end_line, end_column); + LocationRange range(begin, end); + err = Err(range, std::string(message), std::string(help)); +} + +inline void PopulateErrWithMessage(Err& err, + rust::Str message, + rust::Str help) { + err = Err(Location(), std::string(message), std::string(help)); +} + +inline void AppendSubErr(Err& err, + rust::Str message, + const InputFile& file, + int start_line, + int start_column, + int end_line, + int end_column) { + Location begin(&file, start_line, start_column); + Location end(&file, end_line, end_column); + LocationRange range(begin, end); + err.AppendSubErr(Err(range, std::string(message))); +} + +inline std::unique_ptr<Err> NewErr() { + return std::make_unique<Err>(); +} + +inline rust::String ErrToString(const Err& err) { + return rust::String(err.to_string()); +} + +#endif // TOOLS_GN_FFI_ERR_H_
diff --git a/src/gn/ffi/test_with_scope.h b/src/gn/ffi/test_with_scope.h index d101ead..0fe2344 100644 --- a/src/gn/ffi/test_with_scope.h +++ b/src/gn/ffi/test_with_scope.h
@@ -7,10 +7,21 @@ #include <memory> +#include "base/command_line.h" +#include "gn/scheduler.h" #include "gn/test_with_scope.h" +struct TestWithScopeAndScheduler : public TestWithScope { + Scheduler scheduler; +}; + inline std::unique_ptr<TestWithScope> NewTestWithScope() { - return std::make_unique<TestWithScope>(); + if (!base::CommandLine::InitializedForCurrentProcess()) { + int argc = 1; + const char* argv[] = {"gn_rust_tests", nullptr}; + base::CommandLine::Init(argc, argv); + } + return std::make_unique<TestWithScopeAndScheduler>(); } #endif // TOOLS_GN_FFI_TEST_WITH_SCOPE_H_
diff --git a/src/gn/starlark/crates/ffi/src/bridge.rs b/src/gn/starlark/crates/ffi/src/bridge.rs index dd32aee..ee7c45f 100644 --- a/src/gn/starlark/crates/ffi/src/bridge.rs +++ b/src/gn/starlark/crates/ffi/src/bridge.rs
@@ -44,6 +44,8 @@ unsafe extern "C++" { // include! simply tells cxxbridge to put the #include in the generated C++ // source code. It does not do anything on the rust side. + include!("gn/err.h"); + include!("gn/ffi/err.h"); include!("gn/ffi/scope.h"); include!("gn/ffi/test_with_scope.h"); include!("gn/ffi/value.h"); @@ -55,6 +57,40 @@ include!("gn/test_with_scope.h"); include!("gn/value.h"); + type Err; + pub fn has_error(self: &Err) -> bool; + // Dead code for production, used in tests only + #[allow(dead_code)] + pub fn NewErr() -> UniquePtr<Err>; + // Dead code for production, used in tests only + #[allow(dead_code)] + pub(in crate::err) fn ErrToString(err: &Err) -> String; + + #[allow(clippy::too_many_arguments)] + pub(in crate::err) fn PopulateErrWithLocation( + err: Pin<&mut Err>, + message: &str, + help: &str, + file: &InputFile, + start_line: i32, + start_column: i32, + end_line: i32, + end_column: i32, + ); + pub(in crate::err) fn PopulateErrWithMessage(err: Pin<&mut Err>, message: &str, help: &str); + pub(in crate::err) fn AppendSubErr( + err: Pin<&mut Err>, + message: &str, + file: &InputFile, + start_line: i32, + start_column: i32, + end_line: i32, + end_column: i32, + ); + + type InputFile; + pub(in crate::err) fn NewInputFile<'a, 'b>(name: &'a str, code: &'a str) -> &'b InputFile; + type OutputFile; #[cxx_return_type = "std::string_view"] pub(in crate::output_file) fn value(self: &OutputFile) -> &str;
diff --git a/src/gn/starlark/crates/ffi/src/err.rs b/src/gn/starlark/crates/ffi/src/err.rs new file mode 100644 index 0000000..1716681 --- /dev/null +++ b/src/gn/starlark/crates/ffi/src/err.rs
@@ -0,0 +1,152 @@ +// Copyright 2026 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +use std::pin::Pin; + +use crate::bridge::Err as CxxErr; + +impl CxxErr { + /// Translates a Starlark evaluation result to a C++ GN error if it failed. + /// + /// If the result is Ok, returns `Some(v)`. + /// If the result is Err, populates this C++ `Err` object and returns + /// `None`. + pub fn handle<T>(self: Pin<&mut Self>, result: starlark::Result<T>) -> Option<T> { + match result { + Ok(v) => Some(v), + Err(e) => { + self.fill(&e); + None + }, + } + } + + /// Populates this C++ `Err` object with the details of the given Starlark + /// error. + pub fn fill(mut self: Pin<&mut Self>, err: &starlark::Error) { + // The "diagnostic" contains the location. + // Since the GN error type already deals with the location, instead of + // having the diagnostic in the error message, we just pass the metadata to gn. + let message = err.without_diagnostic().to_string(); + + // This is the root cause of the error. + let help = err + .kind() + .source() + .map(|e| e.to_string()) + .unwrap_or_default(); + + if let Some(span) = err.span() { + let filename = span.filename(); + let source = span.file.source(); + let resolved = span.file.resolve_span(span.span); + + // Ensure the file is registered in C++ InputFileManager. + let file_ref = crate::bridge::NewInputFile(filename, source); + + crate::bridge::PopulateErrWithLocation( + self.as_mut(), + &message, + &help, + file_ref, + // GN is 1-indexed, starlark-rs is 0-indexed. + resolved.begin.line as i32 + 1, + resolved.begin.column as i32 + 1, + resolved.end.line as i32 + 1, + resolved.end.column as i32 + 1, + ); + } else { + crate::bridge::PopulateErrWithMessage(self.as_mut(), &message, &help); + } + + self.fill_frames(err); + } + + 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 let Some(loc) = &frame.location { + let frame_filename = loc.filename(); + let frame_source = loc.file.source(); + let frame_resolved = loc.file.resolve_span(loc.span); + + // Note: This is not de-duplicated. If you have a 10-deep stack trace in the + // same file, you will have the same InputFile duplicated 10 times in + // InputFileManager. We don't really care though, especially because starlark + // bans recursion. + let frame_file_ref = crate::bridge::NewInputFile(frame_filename, frame_source); + // Regular GN writes "whence it was called". In starlark, you can do things + // like: ``` + // def foo(): + // ... + // bar = foo + // bar() + // ``` + // so rather than "it", we actually specify the name of the function that was + // called. + let frame_msg = format!("whence '{}' was called.", frame.name); + + crate::bridge::AppendSubErr( + self.as_mut(), + &frame_msg, + frame_file_ref, + // GN is 1-indexed, starlark-rs is 0-indexed. + frame_resolved.begin.line as i32 + 1, + frame_resolved.begin.column as i32 + 1, + frame_resolved.end.line as i32 + 1, + frame_resolved.end.column as i32 + 1, + ); + } + } + } +} + +#[cfg(test)] +mod tests { + #[track_caller] + fn assert_err_eq(value: starlark::Result<()>, want: &[&str]) { + assert!(value.is_err()); + let mut err = crate::bridge::NewErr(); + err.pin_mut().handle(value); + assert!(err.has_error()); + assert_eq!(crate::bridge::ErrToString(&err), want.join("\n") + "\n"); + } + + fn eval_starlark(code: &str) -> starlark::Result<()> { + let globals = starlark::environment::Globals::standard(); + starlark::environment::Module::with_temp_heap(|module| { + let mut eval = starlark::eval::Evaluator::new(&module); + let ast = starlark::syntax::AstModule::parse( + "//test.scl", + code.to_owned(), + &starlark::syntax::Dialect::Standard, + ) + .unwrap(); + eval.eval_module(ast, &globals).map(|_| ()) + }) + } + + #[test] + fn test_call_stack_error() { + let _setup = crate::TestWithScope::new(); + let code = r#" +def foo(): + 1 + "a" + +foo() +"#; + let res = eval_starlark(code); + assert_err_eq( + res, + &[ + r#"ERROR at //test.scl:3:3: Operation `+` not supported for types `int` and `string`"#, + r#" 1 + "a""#, + r#" ^------"#, + r#"See //test.scl:5:1: whence 'foo' was called."#, + r#"foo()"#, + r#"^----"#, + ], + ); + } +}
diff --git a/src/gn/starlark/crates/ffi/src/lib.rs b/src/gn/starlark/crates/ffi/src/lib.rs index 988080a..a27dbbd 100644 --- a/src/gn/starlark/crates/ffi/src/lib.rs +++ b/src/gn/starlark/crates/ffi/src/lib.rs
@@ -17,6 +17,7 @@ //! Safe APIs for C++ types are then exposed in the impl functions for each of //! these types in their own files. mod bridge; +mod err; mod label; mod mutability; mod opaque; @@ -27,7 +28,7 @@ mod test_with_scope; mod value; -pub use bridge::{KeyValue, Label, OutputFile, Scope, Settings, SourceDir, Value, ValueType}; +pub use bridge::{Err, KeyValue, Label, OutputFile, Scope, Settings, SourceDir, Value, ValueType}; pub use mutability::Immutable; pub use opaque::{NonOpaque, OpaqueSized}; pub use scope::OwnedScope;