Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect equality operator generated with derive_eq=true #985

Open
jkorinth opened this issue Jul 18, 2024 · 3 comments
Open

Incorrect equality operator generated with derive_eq=true #985

jkorinth opened this issue Jul 18, 2024 · 3 comments

Comments

@jkorinth
Copy link

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
Copy link
Collaborator

emilio commented Jul 18, 2024

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
Copy link
Author

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
Copy link
Collaborator

emilio commented Jul 22, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants