atilaneves / dpp

Directly include C headers in D source code
Boost Software License 1.0
230 stars 31 forks source link

Change EnumBaseType if the enum values overflow int #217

Closed cbecerescu closed 4 years ago

cbecerescu commented 4 years ago
enum Enum1 { A = 68719476704 }; // Compiles fine in both C and D
enum Enum2 { A = 68719476704, B = 1 }; // Still compiles fine in both C and D
enum Enum3 { B = 1, A = 68719476704 }; // Compiles in C, in D => Error: cannot implicitly convert expression 68719476704L of type long to int

In the last case the EnumBaseType is inferred by looking at just the first enum value, which is of type int.

In C, sizeof(enum Enum3) is 8 and sizef(enum Enum1/Enum2) is 4. So changing the EnumBaseType to long if a value from the enum overflows int seems ok to me. I'm not sure if this could impact something else I'm not aware of.

EDIT: The unit test I've added passed locally, but now I see that the AppVeyor build test fails.

atilaneves commented 4 years ago

Appveyor is failing probably because of Windows 32-bit.

cbecerescu commented 4 years ago

Appveyor is failing probably because of Windows 32-bit.

So the assertions fail because what we get from clang for Promoted.B (which is set to LONG_MAX) is actually INT_MAX, and my code doesn't promote the enum.

I'm not sure how to test this code when clang builds the AST for 32-bit (although from the output of the tests I see that the platform is x64).

cbecerescu commented 4 years ago

I've changed the type from long to c_long so that the C and D types correspond (and the tests are passing now).