Skip to content

Conversation

anforowicz
Copy link
Contributor

This PR improves docs (to explain that the type alias has to go into a extern "C++" section, and to point out the need to use #[derive(ExternType)]).

This commit also fixes errors that would be reported if a type alias for a Rust type would be used as a &mut T parameter. The fixes boil down to ensuring that the Rust type is recognized as Rust-movable. More details can be found in the PR discussion.

Description of changes in macro/src/expand.rs

16e2620 has introduced support for #[derive(ExternType)] in extern “Rust” section. I don’t understand why this commit used type Kind = ::cxx::kind::Opaque rather than kind::Trivial.

Docs for cxx::kind::Opaque say that “an opaque type [...] cannot be passed or held by value within Rust” because “Rust’s move semantics are such that every move is equivalent to a memcpy” (which is “incompatible in general with C++’s constructor-based move semantics”).

Docs for cxx::kind::Trivial say that this is “a type with trivial move constructor and no destructor, which can therefore be owned and moved around in Rust code without requiring indirection”.

Based on tests/ui/rust_pinned.rs (and the corresponding expected error message in the .stderr file here I assume that cxx does not allow extern “Rust” { type RustType; } for !Unpin types. So this would mean that all Rust types supported by cxx can be moved by memcpy.

Therefore it seems okay to change macro/src/[expand.rs](http://expand.rs/) to emit type Kind = ::cxx::kind::Trivial

Without the change in macro/src/expand.rs, the new tests would result in the following error:

error[E0271]: type mismatch resolving `<CrossModuleRustType as ExternType>::Kind == Trivial`
   --> tests/ffi/lib.rs:374:14
    |
374 |         type CrossModuleRustType = crate::module::CrossModuleRustType;
    |              ^^^^^^^^^^^^^^^^^^^ type mismatch resolving `<CrossModuleRustType as ExternType>::Kind == Trivial`
    |
note: expected this to be `Trivial`
   --> tests/ffi/module.rs:85:18
    |
 85 |         #[derive(ExternType)]
    |                  ^^^^^^^^^^
note: required by a bound in `verify_extern_kind`
   --> /usr/local/google/home/lukasza/src/github/cxx/src/extern_type.rs:187:41
    |
187 | pub fn verify_extern_kind<T: ExternType<Kind = Kind>, Kind: self::Kind>() {}
    |                                         ^^^^^^^^^^^ required by this bound in `verify_extern_kind`

Description of changes in gen/src/write.rs

Currently C++ bindings for extern “Rust” types do not mark the bindings as relocatable (in the cxx::IsRelocatable<T> sense).

The doc comment of cxx::IsRelocatable says this is an assertion that the type “is soundly relocatable by Rust”.

We already established above that all Rust types supported by cxx can be moved by memcpy. Therefore from that perspective it seems okay to start emitting using IsRelocatable = ::std::true_type inside the struct/bindings emitted for Rust types by fn write_opaque_type in gen/src/[write.rs](http://write.rs/).

Emitting a new public member carries a risk that its name may conflict/clash with another, preexisting member. So from that perspective, it may be safer to directly specialize cxx::IsRelocatable. OTOH, the Rust naming guidelines reserve UpperCamelCase to types, traits, enum variants, and type parameters - this means that a clash with IsRelocatable seems unlikely.

Without the change in gen/src/write.rs, the new tests would result in the following error:

warning: [email protected]: /usr/local/google/home/lukasza/src/github/cxx/target/debug/build/cxx-test-suite-690332097c2cd1c3/out/cxxbridge/sources/tests/ffi/lib.rs.cc:1489:58: error: static assertion failed: type tests::CrossModuleRustType should be trivially move constructible and trivially destructible in C++ to be used as a non-pinned mutable reference in signature of `r_mut_ref_cross_module_rust_type` in Rust
warning: [email protected]:  1489 |     ::rust::IsRelocatable<::tests::CrossModuleRustType>::value,
warning: [email protected]:       |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
error: failed to run custom build command for `cxx-test-suite v0.0.0 (/usr/local/google/home/lukasza/src/github/cxx/tests/ffi)`

/cc @ShabbyX

This commit improves docs (to explain that the type alias has to go into
a `extern "C++"` section, and to point out the need to use
`#[derive(ExternType)]`).

This commit also fixes errors that would be reported if a type alias for
a Rust type would be used as a `&mut T` parameter.  The fixes are to
ensure that the Rust type is recognized as Rust-movable.  More details
can be found in the PR discussion.
Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would allow exposing an ABI mismatch where types in a struct or function signature have different offsets in different languages, which is unsound. cxx::kind::Opaque is required if the Rust and C++ definition are differently sized or aligned.

Example:

// src/module.rs

pub struct Rust(pub [isize; 2]);

#[cxx::bridge]
pub mod ffi {
    extern "Rust" {
        #[derive(ExternType)]
        type Rust;
    }
}
// src/main.rs

pub mod module;

#[cxx::bridge]
mod ffi {
    extern "C++" {
        include!("example/include/header.h");
        include!("example/src/module.rs.h");

        type Rust = crate::module::Rust;
    }

    unsafe extern "C++" {
        fn repro(s: &Struct);
    }

    struct Struct {
        rust: Rust,
        string: String,
    }
}

fn main() {
    let s = ffi::Struct {
        rust: module::Rust([1; 2]),
        string: ".".repeat(100),
    };
    println!("{:p}", &s);
    println!("{:p}", &s.string);
    ffi::repro(&s);
}
// include/header.h

#pragma once
struct Struct;
void repro(Struct const &s) noexcept;
// src/lib.cc

#include <example/src/main.rs.h>
#include <rust/cxx.h>
#include <iostream>

void repro(Struct const &s) noexcept {
  std::cout << static_cast<const void*>(&s);
  std::cout << '\n';
  std::cout << static_cast<const void*>(&s.string);
  std::cout << '\n';
  std::cout << s.string;
  std::cout << '\n';
}
0x7ffc54784ee0
0x7ffc54784ef0
0x7ffc54784ee0
0x7ffc54784ee8
Segmentation fault (core dumped)

@anforowicz
Copy link
Contributor Author

Abandoning this PR, because the right way to support Box<T>, &mut T, and &[T] has landed elsewhere as described in #1539 (comment). Thanks!

@anforowicz anforowicz closed this Sep 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants