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

@@ -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

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()
 

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

test/cxxbridge/cxxbridge_circular/CMakeLists.txt

@@ -0,0 +1,38 @@
+# 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")
+if(MSVC)
+    set_target_properties(cpp_lib PROPERTIES MSVC_RUNTIME_LIBRARY "MultiThreadedDLL")
+endif()
+
+# 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.
+target_link_libraries(rust_lib INTERFACE
+    "$<LINK_GROUP:RESCAN,cxxbridge,rust_lib-static,cpp_lib>"
+)
+
+add_executable(cpp_bin main.cpp)
+target_link_libraries(cpp_bin rust_lib)

test/cxxbridge/cxxbridge_circular/cpplib.cpp

@@ -0,0 +1,27 @@
+#include "cpplib.h"
+#include "cxxbridge/lib.h"
+#include "rust/cxx.h"
+#include <iostream>
+
+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() {
+  RsImage expected{.width = 1,
+                   .height = 1,
+                   .raster = Rgba{
+                       .r = 1.0,
+                       .g = 2.0,
+                       .b = 3.0,
+                       .a = 4.0,
+                   }};
+
+  if (!expected.equal_to("dummy path")) {
+    throw std::runtime_error("equality_check failed");
+  }
+}

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();

test/cxxbridge/cxxbridge_circular/main.cpp

@@ -0,0 +1,17 @@
+#include "cpplib.h"
+
+#include <iostream>
+
+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;
+}

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"

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)
+    }
+}