From 27cced0977e90d5a3dd66e45308400097ad243d6 Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Wed, 27 Nov 2024 16:20:35 +0100 Subject: [PATCH] Add test for circular CXX bridge This includes a fix in corrosion_add_cxxbridge, to force generation of the header files for all upstream dependencies, which is important in this circular setup. --- .github/workflows/test.yaml | 2 +- cmake/Corrosion.cmake | 19 ++-- test/cxxbridge/CMakeLists.txt | 3 + .../cxxbridge_circular/CMakeLists.txt | 44 +++++++++ test/cxxbridge/cxxbridge_circular/cpplib.cpp | 18 ++++ .../cxxbridge_circular/include/cpplib.h | 6 ++ test/cxxbridge/cxxbridge_circular/main.cpp | 17 ++++ .../cxxbridge_circular/rust/Cargo.lock | 89 +++++++++++++++++++ .../cxxbridge_circular/rust/Cargo.toml | 11 +++ .../cxxbridge_circular/rust/src/lib.rs | 34 +++++++ 10 files changed, 236 insertions(+), 7 deletions(-) create mode 100644 test/cxxbridge/cxxbridge_circular/CMakeLists.txt create mode 100644 test/cxxbridge/cxxbridge_circular/cpplib.cpp create mode 100644 test/cxxbridge/cxxbridge_circular/include/cpplib.h create mode 100644 test/cxxbridge/cxxbridge_circular/main.cpp create mode 100644 test/cxxbridge/cxxbridge_circular/rust/Cargo.lock create mode 100644 test/cxxbridge/cxxbridge_circular/rust/Cargo.toml create mode 100644 test/cxxbridge/cxxbridge_circular/rust/src/lib.rs diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 61a9f219..5dbb58d2 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -340,7 +340,7 @@ jobs: - name: Install CMake uses: lukka/get-cmake@519de0c7b4812477d74976b2523a9417f552d126 with: - cmakeVersion: "~3.22.0" + cmakeVersion: "~3.24.0" ninjaVersion: "~1.10.0" - name: Install Rust uses: dtolnay/rust-toolchain@master diff --git a/cmake/Corrosion.cmake b/cmake/Corrosion.cmake index 8227e9c1..e6f4396c 100644 --- a/cmake/Corrosion.cmake +++ b/cmake/Corrosion.cmake @@ -1707,7 +1707,8 @@ function(corrosion_add_cxxbridge cxx_target) COMMENT "Generating rust/cxx.h header" ) - set(GENERATED_FILES "${generated_dir}/include/rust/cxx.h") + set(GENERATED_SOURCES "") + set(GENERATED_HEADERS "${generated_dir}/include/rust/cxx.h") foreach(filepath ${_arg_FILES}) get_filename_component(filename ${filepath} NAME_WE) @@ -1739,15 +1740,21 @@ function(corrosion_add_cxxbridge cxx_target) COMMENT "Generating cxx bindings for crate ${_arg_CRATE} and file src/${filepath}" ) - list(APPEND GENERATED_FILES - "${header_placement_dir}/${cxx_header}" - "${source_placement_dir}/${cxx_source}") + list(APPEND GENERATED_SOURCES "${source_placement_dir}/${cxx_source}") + list(APPEND GENERATED_HEADERS "${header_placement_dir}/${cxx_header}") endforeach() - target_sources(${cxx_target} PRIVATE ${GENERATED_FILES}) + target_sources(${cxx_target} PRIVATE ${GENERATED_SOURCES}) + # Make sure to export the headers with PUBLIC. + # This ensures that any target that depends on cxx_target also has these files as a dependency + # CMake will then make sure to generate the files before building either target, which is important + # in the presence of circular dependencies + target_sources(${cxx_target} PUBLIC ${GENERATED_HEADERS}) if(DEFINED _arg_REGEN_TARGET) + # Add only the headers to the regen target, as the sources are actually not needed + # For the IDE to pick everything up add_custom_target(${_arg_REGEN_TARGET} - DEPENDS ${GENERATED_FILES} + DEPENDS ${GENERATED_HEADERS} COMMENT "Generated cxx bindings for crate ${_arg_CRATE}") endif() diff --git a/test/cxxbridge/CMakeLists.txt b/test/cxxbridge/CMakeLists.txt index 23c34ca9..89a918bf 100644 --- a/test/cxxbridge/CMakeLists.txt +++ b/test/cxxbridge/CMakeLists.txt @@ -8,6 +8,9 @@ if(CORROSION_TESTS_CXXBRIDGE) PASS_THROUGH_ARGS -DTEST_CXXBRIDGE_VARIANT2=ON ) corrosion_tests_add_test(cxxbridge_rust2cpp "cxxbridge-exe") + if (CMAKE_VERSION VERSION_GREATER_EQUAL "3.24.0") + corrosion_tests_add_test(cxxbridge_circular "cpp_bin") + endif() set_tests_properties("cxxbridge_cpp2rust_1_run_rust_bin" PROPERTIES PASS_REGULAR_EXPRESSION diff --git a/test/cxxbridge/cxxbridge_circular/CMakeLists.txt b/test/cxxbridge/cxxbridge_circular/CMakeLists.txt new file mode 100644 index 00000000..3ce40425 --- /dev/null +++ b/test/cxxbridge/cxxbridge_circular/CMakeLists.txt @@ -0,0 +1,44 @@ +# This CMake project tests the setup for circular dependencies that involve a CXX bridge +# While circular dependencies are usually discouraged, with CXX they are reasonbly natural, +# as ideally, Rust should be able to call C++ freely and vice-versa. +# +# The way this project is set up, the rust_lib target acts as the one target that includes +# the mixed C++ and Rust code with the cpp_lib and cxxbridge targets as implementation details, +# which shouldn't be used individually. +cmake_minimum_required(VERSION 3.24) +project(test_project VERSION 0.1.0 LANGUAGES CXX) +include(../../test_header.cmake) +set(CMAKE_CXX_STANDARD 11) +set(CMAKE_CXX_STANDARD_REQUIRED 1) + +corrosion_import_crate(MANIFEST_PATH rust/Cargo.toml) +corrosion_add_cxxbridge(cxxbridge CRATE rust_lib FILES lib.rs) + +if(MSVC) + set_target_properties(cxxbridge PROPERTIES MSVC_RUNTIME_LIBRARY "MultiThreadedDLL") +endif() + +add_library(cpp_lib STATIC cpplib.cpp) +target_include_directories(cpp_lib PUBLIC "${CMAKE_CURRENT_LIST_DIR}/include") + +# Make sure the cpp library can access the headers from the bridge and vice-versa +target_link_libraries(cpp_lib PRIVATE cxxbridge) +target_link_libraries(cxxbridge PRIVATE cpp_lib) + +# The 3 libraries (rust, cpp, cxx) have a circular dependency, set this up in the linker +# so that the rust_lib target links to everything and circular references are resolved. +if (CMAKE_CXX_LINK_GROUP_USING_RESCAN_SUPPORTED) + target_link_libraries(rust_lib INTERFACE + "$") +else() + target_link_libraries(rust_lib INTERFACE cxxbridge cpp_lib) +endif() + +add_executable(cpp_bin main.cpp) +target_link_libraries(cpp_bin rust_lib) + +if(MSVC) + set_target_properties(cpp_lib PROPERTIES MSVC_RUNTIME_LIBRARY "MultiThreadedDLL") + set_target_properties(cxxbridge PROPERTIES MSVC_RUNTIME_LIBRARY "MultiThreadedDLL") + set_target_properties(cpp_bin PROPERTIES MSVC_RUNTIME_LIBRARY "MultiThreadedDLL") +endif() diff --git a/test/cxxbridge/cxxbridge_circular/cpplib.cpp b/test/cxxbridge/cxxbridge_circular/cpplib.cpp new file mode 100644 index 00000000..15549a42 --- /dev/null +++ b/test/cxxbridge/cxxbridge_circular/cpplib.cpp @@ -0,0 +1,18 @@ +#include "cpplib.h" +#include "cxxbridge/lib.h" +#include "rust/cxx.h" +#include + +RsImage read_image(rust::Str path) { + std::cout << "read_image called" << std::endl; + std::cout << path << std::endl; + Rgba c = {1.0, 2.0, 3.0, 4.0}; + RsImage v = {1, 1, c}; + return v; +} + +void assert_equality() { + if (!read_image("dummy_path").equal_to("dummy path")) { + throw std::runtime_error("equality_check failed"); + } +} diff --git a/test/cxxbridge/cxxbridge_circular/include/cpplib.h b/test/cxxbridge/cxxbridge_circular/include/cpplib.h new file mode 100644 index 00000000..cf3427d6 --- /dev/null +++ b/test/cxxbridge/cxxbridge_circular/include/cpplib.h @@ -0,0 +1,6 @@ +#pragma once +#include "cxxbridge/lib.h" + +::RsImage read_image(::rust::Str path); + +void assert_equality(); diff --git a/test/cxxbridge/cxxbridge_circular/main.cpp b/test/cxxbridge/cxxbridge_circular/main.cpp new file mode 100644 index 00000000..023e5a91 --- /dev/null +++ b/test/cxxbridge/cxxbridge_circular/main.cpp @@ -0,0 +1,17 @@ +#include "cpplib.h" + +#include + +int main(void) { + std::cout << "Testing roundtrip..." << std::endl; + + try { + assert_equality(); + } catch (const std::exception &e) { + std::cerr << "Error: " << e.what() << std::endl; + return 1; + } + + std::cout << "Roundtrip successful!"; + return 0; +} diff --git a/test/cxxbridge/cxxbridge_circular/rust/Cargo.lock b/test/cxxbridge/cxxbridge_circular/rust/Cargo.lock new file mode 100644 index 00000000..aadd14c8 --- /dev/null +++ b/test/cxxbridge/cxxbridge_circular/rust/Cargo.lock @@ -0,0 +1,89 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "cc" +version = "1.0.78" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a20104e2335ce8a659d6dd92a51a767a0c062599c73b343fd152cb401e828c3d" + +[[package]] +name = "cxx" +version = "1.0.86" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51d1075c37807dcf850c379432f0df05ba52cc30f279c5cfc43cc221ce7f8579" +dependencies = [ + "cc", + "cxxbridge-flags", + "cxxbridge-macro", + "link-cplusplus", +] + +[[package]] +name = "cxxbridge-flags" +version = "1.0.86" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61b50bc93ba22c27b0d31128d2d130a0a6b3d267ae27ef7e4fae2167dfe8781c" + +[[package]] +name = "cxxbridge-macro" +version = "1.0.86" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "39e61fda7e62115119469c7b3591fd913ecca96fb766cfd3f2e2502ab7bc87a5" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "link-cplusplus" +version = "1.0.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ecd207c9c713c34f95a097a5b029ac2ce6010530c7b49d7fea24d977dede04f5" +dependencies = [ + "cc", +] + +[[package]] +name = "proc-macro2" +version = "1.0.50" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ef7d57beacfaf2d8aee5937dab7b7f28de3cb8b1828479bb5de2a7106f2bae2" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quote" +version = "1.0.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8856d8364d252a14d474036ea1358d63c9e6965c8e5c1885c18f73d70bff9c7b" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "rust_lib" +version = "0.1.0" +dependencies = [ + "cxx", +] + +[[package]] +name = "syn" +version = "1.0.107" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1f4064b5b16e03ae50984a5a8ed5d4f8803e6bc1fd170a3cda91a1be4b18e3f5" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "unicode-ident" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "84a22b9f218b40614adcb3f4ff08b703773ad44fa9423e4e0d346d5db86e4ebc" diff --git a/test/cxxbridge/cxxbridge_circular/rust/Cargo.toml b/test/cxxbridge/cxxbridge_circular/rust/Cargo.toml new file mode 100644 index 00000000..d5fca1ea --- /dev/null +++ b/test/cxxbridge/cxxbridge_circular/rust/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "rust_lib" +version = "0.1.0" +edition = "2018" + +[lib] +name = "rust_lib" +crate-type = ["staticlib"] + +[dependencies] +cxx = "1.0" diff --git a/test/cxxbridge/cxxbridge_circular/rust/src/lib.rs b/test/cxxbridge/cxxbridge_circular/rust/src/lib.rs new file mode 100644 index 00000000..024e6eae --- /dev/null +++ b/test/cxxbridge/cxxbridge_circular/rust/src/lib.rs @@ -0,0 +1,34 @@ +#[cxx::bridge] +pub mod ffi { + #[derive(Debug, PartialEq)] + pub struct Rgba { + r: f32, + g: f32, + b: f32, + a: f32, + } + + #[derive(Debug, PartialEq)] + pub struct RsImage { + width: usize, + height: usize, + raster: Rgba, + } + unsafe extern "C++" { + include!("cpplib.h"); + pub fn read_image(path: &str) -> RsImage; + } + + extern "Rust" { + pub fn equal_to(self: &RsImage, other: &str) -> bool; + } +} + +use ffi::*; + +impl RsImage { + pub fn equal_to(&self, path: &str) -> bool { + println!("equal_to"); + *self == read_image(path) + } +}