transpile: Translate enums with newtype structs

[Rua]

• Depends on transpile: Add convert_decl_ref and convert_enum_constant_decl_ref #1889
• Depends on transpile: Take CQualTypeId and return WithStmts in enum/pointer cast functions #1891
• Depends on transpile: Deduplicate in/decrement operator handling #1921
• Depends on transpile: Use a single global Renamer with namespacing #1956
• Depends on refactor: Include value namespace as well for tuple/unit structs #1958

[Rua]

It seems that the transpiler is generating working code, but then the refactorer is breaking the imports. I don't know if that's simply a bug in the refactorer, or something else.

Edit: wondering if the refactorer is not handling impl blocks properly, to support associated constants?

[kkysen]

Btw, #1623 should help with the error messages here, which I saw were atrocious (#1621).

[Rua]

The errors from refactor don't seem to have gone away. This PR hasn't changed anything about refactor, so is that a bug in the refactorer?

[kkysen]

The errors from refactor don't seem to have gone away. This PR hasn't changed anything about refactor, so is that a bug in the refactorer?

Oh sorry, I meant #1623 should help make the error messages legible, but not fix them.

This is likely either a bug in the refactorer, or the refactorer is assuming something about what code the transpiler generates for enums and now can't handle a different translation of enums. If it's the latter, we should probably fix it here.

[Rua]

I've not looked at the refactorer code at all yet, so I wouldn't really know where to look for a possible cause. The CI error messages don't give much of a hint.

[kkysen]

I've not looked at the refactorer code at all yet, so I wouldn't really know where to look for a possible cause. The CI error messages don't give much of a hint.

@ahomescu, do you have any idea maybe?

[ahomescu]

Could you share the refactorer errors you're getting?

[Rua]

The errors are happening in CI. I can't run the tests on NixOS myself.

[Rua]

I've removed the second commit from the PR, the one that uses associated constants instead of global ones. That seems to be the one that the refactorer has trouble with.

EDIT: I guess not? Hmm.

[ahomescu]

If you run the transpiler with --reorganize-definitions (we're renaming that btw, but I don't think it has landed), are you maybe missing the c2rust::src_loc attributes on the newtypes?

[Rua]

@ahomescu Doesn't seem so. Enabling it on tests/unit/enums/src/top_enum.c produces

#[derive(Clone, Copy)]
#[repr(transparent)]
#[c2rust::src_loc = "2:1"]
pub struct E(pub ::core::ffi::c_uint);
#[c2rust::src_loc = "4:5"]
pub const B: E = E(1);
#[c2rust::src_loc = "3:5"]
pub const A: E = E(0);

[ahomescu]

I didn't realize this was waiting for my response. I think we should keep EnumMode. The Rust enum newtypes are sometimes clunky to use for some common C patterns like bitflags. For example if you have a x = Foo::A | Foo::B; in C where A and B are enumeration constants, then you need to either implement operations like BitOr for the newtype, use an existing crate like bitflags, or do something clunky like Foo(Foo::A.0 | Foo::B.0).

[ahomescu]

Continuing my previous comment: bindgen has the same issue https://docs.rs/bindgen/latest/bindgen/struct.Builder.html#method.bitfield_enum and interestingly enough one of the modes they support is "bitfield enum" which is "newtype with bitwise operations implemented".

[Rua]

The Rust enum newtypes are sometimes clunky to use for some common C patterns like bitflags.

@ahomescu Bitflags are a case that is better translated with an integer than with a newtype. But then there will also be uses where a newtype fits better. If there is a command-line option to choose the output, then both options are possible, but they affect all enum uses, even the ones where the other representation would be preferred. No matter what you choose, there will be cases where the chosen representation is less ideal.

However, the newtype representation has one very obvious advantage over the integer one: it preserves information about the enum type, like being a real distinct type, and the possibility for associated constants, that is lost with a simple type alias. Specific instances of newtypes can be refactored by the user into integer type aliases in the case of bitflags and the like, but the reverse is not possible. So a user who wants newtypes for some instances and integers for others will opt for the newtype. The integer representation is more lossy.

[ahomescu]

Could we implement "newtype with bitwise operations" then? It's a bit hacky but it seems like it covers the most common use cases.

[Rua]

@ahomescu that would be best done with a separate support crate, similar to the one for bitfields. Do you agree?

[ahomescu]

a separate support crate, similar to the one for bitfields

With a macro that the transpiler would invoke? Sure, that could work.

[ahomescu]

Oh I thought you were removing EnumMode now. I'd be fine with either keeping it, or replacing with "newtype with operations".

[ahomescu]

Do we not have any test (unit or snapshot) with a C bitmask enum?

[Rua]

@ahomescu I've been looking into how to make the bitwise operators work here, but it doesn't seem so trivial. Bitwise operations on enums are emitted in the AST with casts from and back to the enum type, as demonstrated by the currently produced code:

typedef enum { Foo0, Foo1 } Foo;

void test_enums(void) {
    Foo foo = Foo0;
    Foo bar = Foo1;
    foo |= bar;
}

pub type Foo = ::core::ffi::c_uint;
pub const Foo1: Foo = 1;
pub const Foo0: Foo = 0;
#[no_mangle]
pub unsafe extern "C" fn test_enums() {
    let mut foo: Foo = Foo0;
    let mut bar: Foo = Foo1;
    foo = (foo as ::core::ffi::c_uint | bar as ::core::ffi::c_uint) as Foo;
}

The code produced by this PR actually seems like an improvement:

#[derive(Clone, Copy)]
#[repr(transparent)]
pub struct Foo(pub ::core::ffi::c_uint);
pub const Foo1: Foo = Foo(1);
pub const Foo0: Foo = Foo(0);
#[no_mangle]
pub unsafe extern "C" fn rust_test_enums() {
    let mut foo: Foo = Foo0;
    let mut bar: Foo = Foo1;
    foo = Foo(foo.0 | bar.0);
}

So it's really not the newtypes that are causing the code to become less clean here, it's the intermediate casts. And those are part of the AST, so c2rust is really just being faithful here. Making use of bitwise operators here would involve bypassing all these casts, but that applies just as much to the old code as the new, and would add a lot of transpiler complexity.

[ahomescu]

Okay I guess we can land it as is for now, but I'd really like to eventually hide the internal value (not have it pub).

Are you still removing the Consts mode?

[Rua]

This now depends on #1921 to make tests pass.

There also seems to be an import issue with libgit2 in the testsuite now. I don't know what the cause of that is yet.

[ahomescu]

This looks good now, if you resolve the conflicts and it passes the tests then we can land it.

[ahomescu]

Do we get a compilation error here if the value is negative but the underlying type is one of the other unsigned types? E.g. (enum E)-1 where E is a uint8_t.

[Rua]

Oh you're right, that needs a cast. But that seems to be a problem already in master, going back at least to the beginning of 2026. I'll make a fix in a separate PR.

[Rua]

#1965