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

repr(int) enums are currently broken #261

Closed
petertodd opened this issue Dec 16, 2018 · 5 comments
Closed

repr(int) enums are currently broken #261

petertodd opened this issue Dec 16, 2018 · 5 comments

Comments

@petertodd
Copy link

See rust-lang/rust#56619

Here's an example, lib.rs:

#![allow(dead_code)]

#[repr(u8)]
pub enum Foo {
    A { 
        a: u16,
        b: u8, 
    },  
    B,  
}

extern "C" {
    fn foo() -> Foo;
}

Foo is actually laid out as:

$ cargo rustc -- -Z print-type-sizes
   Compiling foo v0.1.0 (/tmp/foo)
print-type-size type: `Foo`: 4 bytes, alignment: 2 bytes
print-type-size     discriminant: 1 bytes
print-type-size     variant `A`: 3 bytes
print-type-size         field `.b`: 1 bytes
print-type-size         field `.a`: 2 bytes
print-type-size     variant `B`: 0 bytes
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s

...with b and a of the A variant reversed. Meanwhile cbindgen happily produces a binding with those fields in the wrong order:

$ cbindgen foo
#include <cstdint>
#include <cstdlib>

union Foo {
  enum class Tag : uint8_t {
    A,
    B,
  };

  struct A_Body {
    Tag tag;
    uint16_t a;
    uint8_t b;
  };

  struct {
    Tag tag;
  };
  A_Body a;
};

extern "C" {

extern Foo foo();

} // extern "C"

A decent temporary fix would be to temporarily disable repr(int) enums with variants containing multiple fields of different types until rustc fixes this; if every field is the same type the ordering isn't changed. Similarly single-field variants are also fine because they can't be reordered.

@petertodd
Copy link
Author

FWIW, this was working in nightly-2017-04-12-x86_64-unknown-linux-gnu and broke in nightly-2017-04-13-x86_64-unknown-linux-gnu

The commit that broke it is almost certainly rust-lang/rust@4f6f4eb, which re-enabled field reordering.

@emilio
Copy link
Collaborator

emilio commented Dec 16, 2018

Assuming rust-lang/rust#56887 is the right fix (which I suspect it is, but I'm not a rustc developer) I'd prefer to keep this for now and try to uplift that patch instead in rust stable / beta instead of disabling them on cbindgen.

@eqrion
Copy link
Collaborator

eqrion commented Dec 17, 2018

So just to clarify, is this is an issue with rustc reordering fields when it shouldn't or in cbindgen misunderstanding #[repr(c)]?

@emilio
Copy link
Collaborator

emilio commented Dec 17, 2018

The former.

@petertodd
Copy link
Author

Update: rust-lang/rust#56887 got merged so this will be working again in newer compilers.

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

No branches or pull requests

3 participants