From 973c5960b7cf2ab930b25334f3ebb25d3dc5265b Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Mon, 24 Jan 2022 12:12:50 +0100 Subject: [PATCH] C++ interpreter: Use std::span instead of the internal Slice in the public API --- CHANGELOG.md | 10 +++++++++ api/sixtyfps-cpp/include/sixtyfps.h | 22 +++++++++---------- .../include/sixtyfps_interpreter.h | 18 +++++++-------- api/sixtyfps-cpp/tests/interpreter.cpp | 7 +++--- sixtyfps_compiler/generator/cpp.rs | 12 +++++----- 5 files changed, 37 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3caf28f1c..adc91ee77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,16 @@ All notable changes to this project will be documented in this file. ## Unreleased +### Changed + + - Minimum rust version is now 1.56 + - C++ compiler requires C++20 + - In the C++ interpreter API `std::span` is used for callbacks arguments, instead of `sixtyfps::Slice` + +### Added + +### Fixed + ## [0.1.6] - 2022-01-21 ### Changed diff --git a/api/sixtyfps-cpp/include/sixtyfps.h b/api/sixtyfps-cpp/include/sixtyfps.h index f682b013c..30c3a5754 100644 --- a/api/sixtyfps-cpp/include/sixtyfps.h +++ b/api/sixtyfps-cpp/include/sixtyfps.h @@ -17,6 +17,7 @@ #include #include #include +#include namespace sixtyfps::cbindgen_private { // Workaround https://github.com/eqrion/cbindgen/issues/43 @@ -48,9 +49,6 @@ using cbindgen_private::ItemWeak; using cbindgen_private::TraversalOrder; } -// FIXME: this should not be public API -using cbindgen_private::Slice; - namespace private_api { using ItemTreeNode = cbindgen_private::ItemTreeNode; using cbindgen_private::KeyboardModifiers; @@ -161,14 +159,14 @@ constexpr inline ItemTreeNode make_dyn_node(std::uintptr_t offset, std::uint32_t parent_index } }; } -inline ItemRef get_item_ref(ComponentRef component, Slice item_tree, int index) +inline ItemRef get_item_ref(ComponentRef component, cbindgen_private::Slice item_tree, int index) { const auto &item = item_tree.ptr[index].item.item; return ItemRef { item.vtable, reinterpret_cast(component.instance) + item.offset }; } inline ItemWeak parent_item(cbindgen_private::ComponentWeak component, - Slice item_tree, int index) + cbindgen_private::Slice item_tree, int index) { const auto &node = item_tree.ptr[index]; if (node.tag == ItemTreeNode::Tag::Item) { @@ -412,10 +410,10 @@ inline bool operator!=(const LayoutInfo &a, const LayoutInfo &b) namespace private_api { inline SharedVector solve_box_layout(const cbindgen_private::BoxLayoutData &data, - Slice repeater_indexes) + cbindgen_private::Slice repeater_indexes) { SharedVector result; - Slice ri { reinterpret_cast(repeater_indexes.ptr), repeater_indexes.len }; + cbindgen_private::Slice ri { reinterpret_cast(repeater_indexes.ptr), repeater_indexes.len }; cbindgen_private::sixtyfps_solve_box_layout(&data, ri, &result); return result; } @@ -428,14 +426,14 @@ inline SharedVector solve_grid_layout(const cbindgen_private::GridLayoutD } inline cbindgen_private::LayoutInfo -grid_layout_info(Slice cells, float spacing, +grid_layout_info(cbindgen_private::Slice cells, float spacing, const cbindgen_private::Padding &padding) { return cbindgen_private::sixtyfps_grid_layout_info(cells, spacing, &padding); } inline cbindgen_private::LayoutInfo -box_layout_info(Slice cells, float spacing, +box_layout_info(cbindgen_private::Slice cells, float spacing, const cbindgen_private::Padding &padding, cbindgen_private::LayoutAlignment alignment) { @@ -443,17 +441,17 @@ box_layout_info(Slice cells, float spacing, } inline cbindgen_private::LayoutInfo -box_layout_info_ortho(Slice cells, +box_layout_info_ortho(cbindgen_private::Slice cells, const cbindgen_private::Padding &padding) { return cbindgen_private::sixtyfps_box_layout_info_ortho(cells, &padding); } inline SharedVector solve_path_layout(const cbindgen_private::PathLayoutData &data, - Slice repeater_indexes) + cbindgen_private::Slice repeater_indexes) { SharedVector result; - Slice ri { reinterpret_cast(repeater_indexes.ptr), repeater_indexes.len }; + cbindgen_private::Slice ri { reinterpret_cast(repeater_indexes.ptr), repeater_indexes.len }; cbindgen_private::sixtyfps_solve_path_layout(&data, ri, &result); return result; } diff --git a/api/sixtyfps-cpp/include/sixtyfps_interpreter.h b/api/sixtyfps-cpp/include/sixtyfps_interpreter.h index f3fa7ed3d..19caf154c 100644 --- a/api/sixtyfps-cpp/include/sixtyfps_interpreter.h +++ b/api/sixtyfps-cpp/include/sixtyfps_interpreter.h @@ -612,7 +612,6 @@ public: return {}; } } - // FIXME! Slice in public API? Should be std::span (c++20) or we need to improve the Slice API /// Invoke the specified callback declared in .60 with the given arguments /// /// Example: imagine the .60 file contains the given callback declaration: @@ -628,10 +627,10 @@ public: /// Returns an null optional if the callback don't exist or if the argument don't match /// Otherwise return the returned value from the callback, which may be an empty Value if /// the callback did not return a value. - std::optional invoke_callback(std::string_view name, Slice args) const + std::optional invoke_callback(std::string_view name, std::span args) const { using namespace cbindgen_private; - Slice args_view { reinterpret_cast(args.ptr), args.len }; + Slice args_view { const_cast(reinterpret_cast(args.data())), args.size() }; ValueOpaque out; if (sixtyfps_interpreter_component_instance_invoke_callback( inner(), sixtyfps::private_api::string_to_slice(name), args_view, &out)) { @@ -666,8 +665,8 @@ public: bool set_callback(std::string_view name, F callback) const { using cbindgen_private::ValueOpaque; - auto actual_cb = [](void *data, Slice arg, ValueOpaque *ret) { - Slice args_view { reinterpret_cast(arg.ptr), arg.len }; + auto actual_cb = [](void *data, cbindgen_private::Slice arg, ValueOpaque *ret) { + std::span args_view { reinterpret_cast(arg.ptr), arg.len }; Value r = (*reinterpret_cast(data))(args_view); new (ret) Value(std::move(r)); }; @@ -730,8 +729,8 @@ public: bool set_global_callback(std::string_view global, std::string_view name, F callback) const { using cbindgen_private::ValueOpaque; - auto actual_cb = [](void *data, Slice arg, ValueOpaque *ret) { - Slice args_view { reinterpret_cast(arg.ptr), arg.len }; + auto actual_cb = [](void *data, cbindgen_private::Slice arg, ValueOpaque *ret) { + std::span args_view { reinterpret_cast(arg.ptr), arg.len }; Value r = (*reinterpret_cast(data))(args_view); new (ret) Value(std::move(r)); }; @@ -741,14 +740,13 @@ public: [](void *data) { delete reinterpret_cast(data); }); } - // FIXME! Slice in public API? Should be std::span (c++20) or we need to improve the Slice API /// Invoke the specified callback declared in an exported global singleton std::optional invoke_global_callback(std::string_view global, std::string_view callback_name, - Slice args) const + std::span args) const { using namespace cbindgen_private; - Slice args_view { reinterpret_cast(args.ptr), args.len }; + Slice args_view { const_cast(reinterpret_cast(args.data())), args.size() }; ValueOpaque out; if (sixtyfps_interpreter_component_instance_invoke_global_callback( inner(), sixtyfps::private_api::string_to_slice(global), diff --git a/api/sixtyfps-cpp/tests/interpreter.cpp b/api/sixtyfps-cpp/tests/interpreter.cpp index f879f9d93..b7793115b 100644 --- a/api/sixtyfps-cpp/tests/interpreter.cpp +++ b/api/sixtyfps-cpp/tests/interpreter.cpp @@ -366,7 +366,7 @@ SCENARIO("Invoke callback") return Value(SharedString(res)); })); Value args[] = { SharedString("Hello"), 42. }; - auto res = instance->invoke_callback("some_callback", Slice { args, 2 }); + auto res = instance->invoke_callback("some_callback", args); REQUIRE(res.has_value()); REQUIRE(*res->to_string() == SharedString("Hello:42")); } @@ -379,7 +379,7 @@ SCENARIO("Invoke callback") auto instance = result->create(); REQUIRE(!instance->set_callback("bar", [](auto) { return Value(); })); Value args[] = { SharedString("Hello"), 42. }; - auto res = instance->invoke_callback("bar", Slice { args, 2 }); + auto res = instance->invoke_callback("bar", args); REQUIRE(!res.has_value()); } } @@ -573,8 +573,7 @@ SCENARIO("Global properties") REQUIRE(result->to_string().value() == "ABC"); Value args[] = { SharedString("Hello") }; - auto res = instance->invoke_global_callback("The_Global", "to-uppercase", - Slice { args, 1 }); + auto res = instance->invoke_global_callback("The_Global", "to-uppercase", args); REQUIRE(res.has_value()); REQUIRE(*res->to_string() == SharedString("HELLO")); } diff --git a/sixtyfps_compiler/generator/cpp.rs b/sixtyfps_compiler/generator/cpp.rs index 900829967..925be00ae 100644 --- a/sixtyfps_compiler/generator/cpp.rs +++ b/sixtyfps_compiler/generator/cpp.rs @@ -934,7 +934,7 @@ fn generate_item_tree( Access::Private, Declaration::Function(Function { name: "item_tree".into(), - signature: "() -> sixtyfps::Slice".into(), + signature: "() -> sixtyfps::cbindgen_private::Slice".into(), is_static: true, statements: Some(vec![ "static const sixtyfps::private_api::ItemTreeNode children[] {".to_owned(), @@ -1953,7 +1953,7 @@ fn compile_expression(expr: &llr::Expression, ctx: &EvaluationContext) -> String crate::expression_tree::ImageReference::EmbeddedData { resource_id, extension } => { let symbol = format!("sfps_embedded_resource_{}", resource_id); format!( - r#"sixtyfps::Image(sixtyfps::cbindgen_private::types::ImageInner::EmbeddedData(sixtyfps::Slice{{std::data({}), std::size({})}}, sixtyfps::Slice{{const_cast(reinterpret_cast(u8"{}")), {}}}))"#, + r#"sixtyfps::Image(sixtyfps::cbindgen_private::types::ImageInner::EmbeddedData(sixtyfps::cbindgen_private::Slice{{std::data({}), std::size({})}}, sixtyfps::cbindgen_private::Slice{{const_cast(reinterpret_cast(u8"{}")), {}}}))"#, symbol, symbol, escape_string(extension), extension.as_bytes().len() ) } @@ -1986,7 +1986,7 @@ fn compile_expression(expr: &llr::Expression, ctx: &EvaluationContext) -> String ) } else { format!( - "sixtyfps::Slice<{ty}>{{ std::array<{ty}, {count}>{{ {val} }}.data(), {count} }}", + "sixtyfps::cbindgen_private::Slice<{ty}>{{ std::array<{ty}, {count}>{{ {val} }}.data(), {count} }}", count = values.len(), ty = ty, val = val.join(", ") @@ -2072,7 +2072,7 @@ fn compile_expression(expr: &llr::Expression, ctx: &EvaluationContext) -> String }; format!("sixtyfps::cbindgen_private::GridLayoutCellData {cv}_array [] = {{ {c} }};\ sixtyfps::cbindgen_private::sixtyfps_reorder_dialog_button_layout({cv}_array, {r});\ - sixtyfps::Slice {cv} {{ std::data({cv}_array), std::size({cv}_array) }}", + sixtyfps::cbindgen_private::Slice {cv} {{ std::data({cv}_array), std::size({cv}_array) }}", r = compile_expression(roles, ctx), cv = cells_variable, c = cells.join(", "), @@ -2271,13 +2271,13 @@ fn box_layout_function( let ri = repeated_indices.as_ref().map_or(String::new(), |ri| { push_code += &format!( - "sixtyfps::Slice {ri}{{ {ri}_array.data(), {ri}_array.size() }};", + "sixtyfps::cbindgen_private::Slice {ri}{{ {ri}_array.data(), {ri}_array.size() }};", ri = ri ); format!("std::array {}_array;", 2 * repeater_idx, ri) }); format!( - "[&]{{ {} {} sixtyfps::Slice{}{{cells_vector.data(), cells_vector.size()}}; return {}; }}()", + "[&]{{ {} {} sixtyfps::cbindgen_private::Slice{}{{cells_vector.data(), cells_vector.size()}}; return {}; }}()", ri, push_code, ident(cells_variable),