corrosion_add_cxxbridge: Allow circular dependencies between bridge/Rust/C++

.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

@@ -1695,14 +1695,11 @@ function(corrosion_add_cxxbridge cxx_target)
     # cxx generated code is using c++11 features in headers, so propagate c++11 as minimal requirement
     target_compile_features(${cxx_target} PUBLIC cxx_std_11)
 
-    # Todo: target_link_libraries is only necessary for rust2c projects.
-    # It is possible that checking if the rust crate is an executable is a sufficient check,
-    # but some more thought may be needed here.
-    # Maybe we should also let the user do this, since for c2rust, the user also has to call
-    # corrosion_link_libraries() themselves.
-    get_target_property(crate_target_type ${_arg_CRATE} TYPE)
-    if (NOT crate_target_type STREQUAL "EXECUTABLE")
-        target_link_libraries(${cxx_target} PRIVATE ${_arg_CRATE})
+    if (TARGET "${_arg_CRATE}-static")
+        target_link_libraries(${cxx_target} PRIVATE "${_arg_CRATE}-static")
+    endif()
+    if (TARGET "${_arg_CRATE}-shared")
+        target_link_libraries(${cxx_target} PRIVATE "${_arg_CRATE}-shared")
     endif()
 
     file(MAKE_DIRECTORY "${generated_dir}/include/rust")
@@ -1714,7 +1711,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)
@@ -1746,15 +1744,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,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
+        "$<LINK_GROUP:RESCAN,cxxbridge,rust_lib-static,cpp_lib>")
+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()

test/cxxbridge/cxxbridge_circular/cpplib.cpp

@@ -0,0 +1,18 @@
+#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() {
+  if (!read_image("dummy_path").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)
+    }
+}