mozilla / cbindgen

A project for generating C bindings from Rust code
Mozilla Public License 2.0
2.33k stars 302 forks source link

Incorrect equality operator generated with `derive_eq=true` #985

Open jkorinth opened 1 month ago

jkorinth commented 1 month ago

When generating C++ with derive_eq=true the implementation of the equality operator is wrong when using type aliases or #[repr(transparent)] structs, e.g.:

pub type Bla = [u8; 12];

#[derive(Eq, PartialEq)]
#[repr(transparent)]
pub struct Bla2([u8; 12]);

#[derive(Eq, PartialEq)]
#[repr(C)]
pub struct Test {
    data: Bla,
    data2: Bla2,
}

#[no_mangle]
pub extern "C" fn do_smth(t: Test) {
    assert!(t == t);
}

With cbindgen.toml:

language = "C++"
[struct]
derive_eq = true

Generates this:

using Bla = uint8_t[12];

using Bla2 = uint8_t[12];

struct Test {
  Bla data;
  Bla2 data2;

  bool operator==(const Test& other) const {
    return data == other.data &&
           data2 == other.data2;
  }
};

This fails a simple test in C++:

Test a, b;
assert(a == b);

Output of cargo tree for reference:

psb v0.1.0 (/home/jk/psb)
└── cbindgen v0.26.0
    ├── clap v3.2.25
    │   ├── atty v0.2.14
    │   │   └── libc v0.2.155
    │   ├── bitflags v1.3.2
    │   ├── clap_lex v0.2.4
    │   │   └── os_str_bytes v6.6.1
    │   ├── indexmap v1.9.3
    │   │   └── hashbrown v0.12.3
    │   │   [build-dependencies]
    │   │   └── autocfg v1.3.0
    │   ├── strsim v0.10.0
    │   ├── termcolor v1.4.1
    │   └── textwrap v0.16.1
    ├── heck v0.4.1
    ├── indexmap v1.9.3 (*)
    ├── log v0.4.22
    ├── proc-macro2 v1.0.86
    │   └── unicode-ident v1.0.12
    ├── quote v1.0.36
    │   └── proc-macro2 v1.0.86 (*)
    ├── serde v1.0.204
    │   └── serde_derive v1.0.204 (proc-macro)
    │       ├── proc-macro2 v1.0.86
    │       │   └── unicode-ident v1.0.12
    │       ├── quote v1.0.36
    │       │   └── proc-macro2 v1.0.86 (*)
    │       └── syn v2.0.71
    │           ├── proc-macro2 v1.0.86 (*)
    │           ├── quote v1.0.36 (*)
    │           └── unicode-ident v1.0.12
    ├── serde_json v1.0.120
    │   ├── itoa v1.0.11
    │   ├── ryu v1.0.18
    │   └── serde v1.0.204 (*)
    ├── syn v1.0.109
    │   ├── proc-macro2 v1.0.86 (*)
    │   ├── quote v1.0.36 (*)
    │   └── unicode-ident v1.0.12
    ├── tempfile v3.10.1
    │   ├── cfg-if v1.0.0
    │   ├── fastrand v2.1.0
    │   └── rustix v0.38.34
    │       ├── bitflags v2.6.0
    │       └── linux-raw-sys v0.4.14
    └── toml v0.5.11
        └── serde v1.0.204 (*)
emilio commented 1 month ago

That C++ code just compares two chunks of uninitialized memory fwiw.

My preference would be to fix this by generating std::array<> rather than plain arrays in C++ mode. But special-casing the comparisons could work too.

jkorinth commented 1 month ago

Then you'd still have to support an approach for C? Like memcmp the blocks? Technically, C++ classes with a single member are also repr[transparent], so you could also transfer the classes and implement Eq on the classes. Then the generated code would work again. Not sure which is preferable?

emilio commented 1 month ago

We don't generate operator== for C because C doesn't support operator overloading.