Support for bitflags enums

[tiawl]

Hi,

Before going into the very heart on this issue, I would like to say that you made a really good work here. Relying on your generator to write the Zig binding generator I am working on, removed a lot of unsatisfactory code I wrote. Even if my tool is not ready to be used, using your generator makes the quality of my tool far better then it was. So thank you for this.

Now that this has been said, some small parts of this ugly code remain and I am opening this issue with the hope to remove some of them.

Currently I am working on the enums section of your JSON generated file. When looking more accurately on the name field of the enumerations, we can divide them into 2 groups:

• bitflags enumerations: its name contains Flags and most of its members are power of 2 unsigned integers to use bitwise operators on them.
• other enumerations.

To make the distinction between them I have to parse the name field (this is an ugly part of the code I want to remove).

I want to make the distinction between them to treat bitflags enumerations as Zig struct. If you are interested in why I want to achieve that, I suggest you to look at this section of the Snektron/vulkan-zig README. This binding generator deals with Vulkan bitflags enumerations in a very clever way I want to implement on the tool I am working on.

I have several proposals to make bitflags enumerations management easier:

• add a is_bitflags_enum bool field to avoid users to parse the name field,
• only for involved enumeration' elements:
  • add 3 new fields: lvalue (int), rvalue (int) and bitwise_operator (string) fields to avoid users to parse the value field,
  • or add 1 new field: is_bitwise_expr bool field to allow users to know when parse the value field safely.

I do not know if other bindings relying on your generator will take advantage of this, but it allows users that want to apply special process for this enumeration kind to do it in a more robust way.

Do you think it could be a feature that you could add to your generator or it goes uncomfortably in the direction of featuritis ?

Thank you.

[ShironekoBen]

Hm... yes, that sounds like a pretty sensible idea to me. It definitely feels like having the flags/not-flags determination in Dear Bindings would make sense as that means if at some point in time the ImGui convention changes (or an exceptional case appears) it can be dealt with centrally rather than having everyone consuming the API data needing to change their code to account for it.

On the subject of parsing the values, my gut feeling is that it would probably be best to go all the way and actually evaluate the final value and expose that rather than constructing a "half-way parsed" representation and leaving the rest to the user. Otherwise this seems likely to generate a mess where items like ImGuiPopupFlags_AnyPopup or ImGuiPopupFlags_MouseButtonMask_ have to be given exceptional treatment whenever they are encountered.

Does that sounds reasonable to you, or is there some case where you think it would be preferable to have the value/operator combinations exposed separately?

[tiawl]

On the subject of parsing the values, my gut feeling is that it would probably be best to go all the way and actually evaluate the final value and expose that

Yeah, I aggre with you: evaluating the final value and expose it, sounds like a better idea.

But I think that adding an other field to say that the element is a bitflag element is also necessary for 2 reasons:

• there are a lot of bitflags enumerations with mask element that should definitely not be considered as a bitflag element but rather as a constant,
• maybe the Imgui API will expose static members for bitflag enumerations like it is already done for other enumerations (Imgui_..._COUNT elements for example) so we need to make the distinction between what should (or not) be considered as an integer where we can make bit-wise operation on it.

[ShironekoBen]

That's a tricky one... whilst logically it would be nice to treat mask elements differently, I'm not sure I can see how to meaningfully differentiate between (say) ImGuiTableFlags_SizingMask_ and ImGuiTableFlags_Borders, both of which are defined as combinations of bits.

And on an API level it seems that people would likely want to use ImGuiTableFlags_Borders and ImGuiTableFlags_BordersInnerH interchangeably without needing to care that one of those is a composite of other values (and we can't say for sure that ImGuiTableFlags_BordersInnerH won't become a composite in some future version of ImGui either - it's very possible that one day it will be defined as ImGuiTableFlags_BordersInnerLeft | ImGuiTableFlags_BordersInnerRight or whatever).

Decomposing the bitfields into a set of independent booleans is a nice idea, but I'm not sure how it can be made compatible with the use-cases above without either exposing every defined value as a separate flag and then using runtime checks to complain if an "impossible" permutation is selected, or manually going through and adding hand-crafted exceptions to expose cases like ImGuiTableFlags_Borders in a sensible manner.

We could expose a is_hidden flag based on the trailing _, I guess, which would at least be a hint to API generators that they might not want to expose that (or put some sort of attribute of their own on it)?

[tiawl]

I'm not sure I can see how to meaningfully differentiate between (say) ImGuiTableFlags_SizingMask_ and ImGuiTableFlags_Borders, both of which are defined as combinations of bits.

I do not think you should make the distinction between ImGuiTableFlags_SizingMask_ and ImGuiTableFlags_Borders. Both of them are combinations of bits and we would like to make bit-wise operations but they are not power of 2. I am considering both of them as static elements because these elements are referring to other elements. From my side, if ImGuiTableFlags is translated as a struct of independant booleans (all set to false by default), ImGuiTableFlags_SizingMask_ and ImGuiTableFlags_Borders are particular case of ImGuiTableFlags struct where some bitfields are already set to true value.

So for me the is_hidden should not be based on the trailing _ but on the value field. If the value field is a power of two AND is unique (so the value field is not referring to other bitflag elements), it is set to true, otherwise it is false.

For example:

• ImGuiTableFlags_SizingStretchProp is not a power of 2 so its is_hidden is false,
• ImGuiTableFlags_SizingFixedSame is a power of 2 so its is_hidden is true,
• ImGuiTableFlags_SizingMask_ and ImGuiTableFlags_Borders are not power of 2 are referring to others bitflag elements so their is_hidden fields are false,
• ImGuiButtonFlags_MouseButtonDefault_ is a power of 2 but it is referring to ImGuiButtonFlags_MouseButtonLeft so its is_hidden is false.

Maybe is_hidden is not the best name for this ?

[ShironekoBen]

I've added evaluation of enum values, so the value field in the JSON is now an integer that contains the actual value of that element. The old value field (the "raw" expression from the C header) is now called value_expression to differentiate it.

I've also done a little bit of tweaking with regards to enum element flags - is_internal is now set to true on elements that end with a _ (previously it was set if the value had an explicit "Internal" comment, but that didn't catch all cases). I've also added is_count, which is set on _COUNT elements.

The more I thought about trying to generate some sort of unified is_hidden flag, the more I started to think that it would be very difficult to make those decisions with Dear Bindings, as different languages have different ideas about what they want there. For example, _COUNT values could be usefully hidden in many cases in C# (as reflection can be used to achieve the same effect), but they are required in C-like languages that lack introspection features.

Reading what you've written above, it sounds like in that example is_hidden is more is_single_unique_bit or similar? In other words, it indicates that the enum value uniquely represents a single bit in the bitfield?

That kinda makes sense, but I still worry that there's no rule that says that a single-bit value today won't become a multi-bit value tomorrow because of some internal implementation change (say the addition of a "ImGuiTableFlags_LegacySizingBehaviour" flag or similar that is then ORed into all the existing enum values), so basing API exposure on that seems a little dangerous.

And more broadly, it feels like the precise definition of what is wanted here is likely to be language/implementation dependent, so I'm a little concerned that trying to wrap all that complexity into a generic flag is going to result in confusion when other implementations try to use it but find it ends up being "almost, but not quite" what they want.

So my gut feeling right now is that the best route forward may be to expose any data that is needed as simple unambiguous flags (in other words flags based on traits of the data, rather than assumed use-cases) which the implementation can then combine as they want to generate the precise definition they need. How does that sound as a route forwards? (and if it makes sense, what other flags would you need exposed for your implementation...? is_single_unique_bit is doable but given that I'm not sure how universally useful it is would it make more sense to use the new value field and just check that implementation-side?)

[tiawl]

And more broadly, it feels like the precise definition of what is wanted here is likely to be language/implementation dependent

Yes you are right, re-reading my last message about the is_hidden/is_single_unique_bit field and I feel like if I were in your place I would think the same thing: it should not be universally useful. Sorry for that: I was maybe too enthusiastic.

However what about:

It definitely feels like having the flags/not-flags determination in Dear Bindings would make sense

I have the feeling that it is going in the direction you are describing in your last words: it will allow me (and maybe other users) to make a precise distinction between this two enumeration kinds from our side, with a simple and unambiguous flag definition from your side.

[ShironekoBen]

Oh, whoops - sorry, yeah, I completely forgot about marking flags enums when I was implementing the other stuff. I've added is_flags_enum to the JSON for enums now... hopefully that makes life a little easier!

[tiawl]

Thank you !