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

Trouble using C enums #3472

Closed
s-ol opened this issue Oct 18, 2019 · 12 comments
Closed

Trouble using C enums #3472

s-ol opened this issue Oct 18, 2019 · 12 comments
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. frontend Tokenization, parsing, AstGen, Sema, and Liveness. translate-c C to Zig source translation feature (@cImport)
Milestone

Comments

@s-ol
Copy link
Contributor

s-ol commented Oct 18, 2019

I'm trying to import/cInclude a header file that defines an enum type, and a struct with an enum member:

enum ImGuiConfigFlags_
{
    ImGuiConfigFlags_None = 0,
    ImGuiConfigFlags_NavEnableKeyboard = 1 << 0,
    ImGuiConfigFlags_NavEnableGamepad = 1 << 1,
    ImGuiConfigFlags_NavEnableSetMousePos = 1 << 2,
    ImGuiConfigFlags_NavNoCaptureKeyboard = 1 << 3,
    ImGuiConfigFlags_NoMouse = 1 << 4,
    ImGuiConfigFlags_NoMouseCursorChange = 1 << 5,
    ImGuiConfigFlags_DockingEnable = 1 << 6,
    ImGuiConfigFlags_ViewportsEnable = 1 << 10,
    ImGuiConfigFlags_DpiEnableScaleViewports= 1 << 14,
    ImGuiConfigFlags_DpiEnableScaleFonts = 1 << 15,
    ImGuiConfigFlags_IsSRGB = 1 << 20,
    ImGuiConfigFlags_IsTouchScreen = 1 << 21
};
// ...
struct ImGuiIO
{
    enum ImGuiConfigFlags_ ConfigFlags;
// ...
};

when assigning the values I get a type mismatch:

  io.*.ConfigFlags |= c.ImGuiConfigFlags_NavEnableKeyboard;
/home/s-ol/zig/src/main.zig:48:20: error: incompatible types: '.cimport:1:11.enum:734:18' and '.cimport:1:11.enum_ImGuiConfigFlags_'
  io.*.ConfigFlags |= c.ImGuiConfigFlags_NavEnableKeyboard;
                   ^
/home/s-ol/zig/src/main.zig:48:7: note: type '.cimport:1:11.enum:734:18' here
  io.*.ConfigFlags |= c.ImGuiConfigFlags_NavEnableKeyboard;
      ^
/home/s-ol/zig/src/main.zig:48:24: note: type '.cimport:1:11.enum_ImGuiConfigFlags_' here
  io.*.ConfigFlags |= c.ImGuiConfigFlags_NavEnableKeyboard;

Shouldn't this be working? I also tried wrapping the enum in a typedef enum ImGuiConfigFlags_ {...} ImGuiConfigFlags and then using it as ImGuiConfigFlags (no underscore or enum) in the struct but the same problem occurs.

@s-ol
Copy link
Contributor Author

s-ol commented Oct 18, 2019

reproducable example:

test.h

enum MyEnum {
 MyEnum_A = 1,
 MyEnum_B = 2,
};

struct MyStruct {
 enum MyEnum theEnum;
};

test.zig

const std = @import("std");

const c = @cImport({
  @cInclude("test.h");
});

pub fn main() void {
  const theStruct = c.MyStruct({ .theEnum = c.MyEnum_A; });
}

error:

/home/s-ol/zig/test.zig:8:46: error: expected type '(enum literal)', found '.cimport:3:11.enum_MyEnum'
  const theStruct = c.MyStruct({ .theEnum = c.MyEnum_A; });
                                             ^
/home/s-ol/zig/zig-cache/o/-TWxiQ4APXP5XQdFvXJH0ZAeZAdEkgu0pO32LAJY_YZA4sI7vaylevI5O9hf0xzD/cimport.zig:3:25: note: .cimport:3:11.enum_MyEnum declared here
pub const enum_MyEnum = extern enum {

@giann
Copy link
Contributor

giann commented Oct 18, 2019

Shouldn't it be c.MyEnum.MyEnum_A ?

@s-ol
Copy link
Contributor Author

s-ol commented Oct 18, 2019

@giann then I get:

/home/s-ol/zigtest/src/main.zig:8:53: error: container '.cimport:3:11.enum_MyEnum' has no member called 'MyEnum_A'
  const theStruct = c.MyStruct({ .theEnum = c.MyEnum.MyEnum_A; });

@s-ol
Copy link
Contributor Author

s-ol commented Oct 18, 2019

the C wrapper I am importing shipped with this hack:

typedef int ImGuiConfigFlags;
enum ImGuiConfigFlags_ {
  // ...
};
struct ImGuiIO {
  ImGuiConfigFlags ConfigFlags;
  // ...
};

so at the moment I am leaving that in place and wrapping the enum values in @enumToInt(), but it seems like there was meant to be a better way there.

@andrewrk andrewrk added this to the 0.7.0 milestone Oct 23, 2019
@andrewrk andrewrk added the translate-c C to Zig source translation feature (@cImport) label Oct 23, 2019
@s-ol
Copy link
Contributor Author

s-ol commented Oct 24, 2019

@andrewrk: I don't think the translate-c label is correct, I am @cImporting only, not translating.

@FireFox317
Copy link
Contributor

@cImport internally uses translate-c to translate the c-header file to a Zig file. --verbose-cimport can be used to check the generated Zig file.

@FireFox317
Copy link
Contributor

FireFox317 commented Oct 24, 2019

The smaller example that you provided works fine, you are just incorrectly initializing the struct, and the error messages are not telling you that.

Correct test.zig file:

const std = @import("std");

const c = @cImport({
  @cInclude("test.h");
});

pub fn main() void {
  const theStruct = c.MyStruct{ .theEnum = c.MyEnum_A };
}

Edit: I think there should be a compile error telling you that you are trying to use the function call syntax on a struct type.

@s-ol
Copy link
Contributor Author

s-ol commented Oct 24, 2019

ah, thanks for the clarification!
an excerpt from the generated zig file shows the problem:

pub const struct_ImGuiIO = extern struct {
    ConfigFlags: extern enum {
        _None = 0,
        _NavEnableKeyboard = 1,
        _NavEnableGamepad = 2,
        _NavEnableSetMousePos = 4,
        _NavNoCaptureKeyboard = 8,
        _NoMouse = 16,
        _NoMouseCursorChange = 32,
        _DockingEnable = 64,
        _ViewportsEnable = 1024,
        _DpiEnableScaleViewports = 16384,
        _DpiEnableScaleFonts = 32768,
        _IsSRGB = 1048576,
        _IsTouchScreen = 2097152,
    },
    BackendFlags: ImGuiBackendFlags,
    DisplaySize: ImVec2,
    DeltaTime: f32,
    IniSavingRate: f32,
    IniFilename: [*c]const u8,

the enum type is inlined in the struct, so while the two structs are identical they are different nominal types and so the error messages above can be explained.
interestingly this doesn't happen with the simple example at the moment:

pub const MyEnum_A = enum_MyEnum._A;
pub const MyEnum_B = enum_MyEnum._B;
pub const enum_MyEnum = extern enum {
    _A = 1,
    _B = 2,
};
pub const struct_MyStruct = extern struct {
    theEnum: enum_MyEnum,
};

the error can be reproduced in my full repo by modifying cimgui/cimgui.h:617 like so:

 struct ImGuiIO
 {
-    ImGuiConfigFlags ConfigFlags;
+    enum ImGuiConfigFlags_ ConfigFlags;
     ImGuiBackendFlags BackendFlags;
     ImVec2 DisplaySize;

(this breaks the build, but with --verbose-cimport you can find the cImport output and investigate)

@FireFox317
Copy link
Contributor

The ConfigFlags enum gets inlined into the struct because of your patch. Without the patch it gets correctly translated into:

pub const ImGuiConfigFlags_None = enum_ImGuiConfigFlags_.None;
pub const ImGuiConfigFlags_NavEnableKeyboard = enum_ImGuiConfigFlags_.NavEnableKeyboard;
...

pub const enum_ImGuiConfigFlags_ = extern enum {
    None = 0,
    NavEnableKeyboard = 1,
    NavEnableGamepad = 2,
    NavEnableSetMousePos = 4,
    NavNoCaptureKeyboard = 8,
    NoMouse = 16,
    NoMouseCursorChange = 32,
    DockingEnable = 64,
    ViewportsEnable = 1024,
    DpiEnableScaleViewports = 16384,
    DpiEnableScaleFonts = 32768,
    IsSRGB = 1048576,
    IsTouchScreen = 2097152,
};

pub const struct_ImGuiIO = extern struct {
    ConfigFlags: ImGuiConfigFlags,
    BackendFlags: ImGuiBackendFlags,
    DisplaySize: ImVec2,
    DeltaTime: f32,
...

I had to add a -DCIMGUI_DEFINE_ENUMS_AND_STRUCTS to the command line to make sure that all the types are defined in the header file. The same can be done in a Zig file using @cDefine, see the Zig documentation: link1 and link2

@s-ol
Copy link
Contributor Author

s-ol commented Oct 24, 2019

@FireFox317 I have been doing that define in c.zig: https://git.s-ol.nu/zig-imgui/blob/master/src/c.zig

Are you sure that you are not using it using the typedef int ImGuiConfigFlags now?
Without the patch above it generates this for me:

...
pub const ImGuiConfigFlags = c_int;
pub const ImGuiConfigFlags_ = enum_ImGuiConfigFlags_;
...
pub const ImGuiConfigFlags_None = enum_ImGuiConfigFlags_.None;
pub const ImGuiConfigFlags_NavEnableKeyboard = enum_ImGuiConfigFlags_.NavEnableKeyboard;
...
pub const enum_ImGuiConfigFlags_ = extern enum {
    None = 0,
    NavEnableKeyboard = 1,
    NavEnableGamepad = 2,
    NavEnableSetMousePos = 4,
    NavNoCaptureKeyboard = 8,
    NoMouse = 16,
    NoMouseCursorChange = 32,
    DockingEnable = 64,
    ViewportsEnable = 1024,
    DpiEnableScaleViewports = 16384,
    DpiEnableScaleFonts = 32768,
    IsSRGB = 1048576,
    IsTouchScreen = 2097152,
};

pub const struct_ImGuiIO = extern struct {
    ConfigFlags: ImGuiConfigFlags,
    BackendFlags: ImGuiBackendFlags,
    DisplaySize: ImVec2,
    DeltaTime: f32,
...

(not sure about order in the file, I'm just jumping back and forth).
As you can see it doesn't use the enum in the struct, it just alias c_int to ImGuiConfigFlags.

The patch is to actually use the enum type in the struct, as the header by default already includes this workaround of using the int type and hiding the enum.

@FireFox317
Copy link
Contributor

Okay, yes my bad. The issue is clear to me right now. I think it is a reasonable thing that it generates a c_int type, because you want to OR multiple flags together. If a Zig enum would have been generated, it would never be possible to OR multiple enum values together, since that is not allowed in Zig. The @enumToInt is probably a good workaround for now. A related issue is #2115.

@andrewrk andrewrk added contributor friendly This issue is limited in scope and/or knowledge of Zig internals. frontend Tokenization, parsing, AstGen, Sema, and Liveness. labels Jan 2, 2020
@s-ol
Copy link
Contributor Author

s-ol commented Jan 15, 2020

(there was going to be a lengthy comment here about how there is still weirdness in @intToEnum, but it turns out I just cast to the wrong type and wasn't able to read the difference).

I think this is solved as of my current zig-git version (0.5.0+8d9d4a065)? The non-namespaced constants (c.MyEnum_A in example above) now come out with @enumToInt in the generated cimport file, while the namespaced values (c.MyEnum.MyEnum_A) keep their type. While this is not necessarily the most obvious solution from a user perspective it works fine (if documented correctly, if I hadn't found out about --verbose-cimport from @FireFox317 this would've been hard to understand)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. frontend Tokenization, parsing, AstGen, Sema, and Liveness. translate-c C to Zig source translation feature (@cImport)
Projects
None yet
Development

No branches or pull requests

5 participants