dearimgui / dear_bindings

C header (and language binding metadata) generator for Dear ImGui
MIT License
221 stars 12 forks source link

Defines not possible to evaluate #44

Open Ldash4 opened 9 months ago

Ldash4 commented 9 months ago

Due to how the "conditionals" field is constructed, there are ambiguous situations that arise when evaluating them. Given this series of defines, we can see that we expect both to pass. However only the first will be, if we update our list of defines after the condition evaluates to true.

#ifndef MAYBE_DEFINED_EXTERNALLY
#define MAYBE_DEFINED_EXTERNALLY A_DEFAULT // (will pass) conditionals: [{ "condition": "ifndef", "expression": "MAYBE_DEFINED_EXTERNALLY" }]
#define RELATED_THING A_DEFAULT_AS_WELL // (will not pass) conditionals: [{ "condition": "ifndef", "expression": "MAYBE_DEFINED_EXTERNALLY" }]
#endif // MAYBE_DEFINED_EXTERNALLY

You can see how that series of defines would result in the same data as these, even though they have different behavior.

#ifndef MAYBE_DEFINED_EXTERNALLY
#define MAYBE_DEFINED_EXTERNALLY A_DEFAULT
#endif // MAYBE_DEFINED_EXTERNALLY
#ifndef MAYBE_DEFINED_EXTERNALLY
#define RELATED_THING A_DEFAULT_AS_WELL
#endif // MAYBE_DEFINED_EXTERNALLY

This happens in practice for the IM_COL32_(R|G|B|A)_SHIFT defines:

#ifndef IM_COL32_R_SHIFT
#ifdef IMGUI_USE_BGRA_PACKED_COLOR
#define IM_COL32_R_SHIFT    16
#define IM_COL32_G_SHIFT    8
#define IM_COL32_B_SHIFT    0
#define IM_COL32_A_SHIFT    24
#define IM_COL32_A_MASK     0xFF000000
#else
#define IM_COL32_R_SHIFT    0
#define IM_COL32_G_SHIFT    8
#define IM_COL32_B_SHIFT    16
#define IM_COL32_A_SHIFT    24
#define IM_COL32_A_MASK     0xFF000000
#endif // #ifdef IMGUI_USE_BGRA_PACKED_COLOR
#endif // #ifndef IM_COL32_R_SHIFT
Ldash4 commented 9 months ago

Thinking this through, it's a bit tricky to represent. Maybe something like this could work?

# defines
[
    {
        "conditional": { "condition": "ifndef", "expression": "IM_COL32_R_SHIFT" },
        "true_case": {
            {
                "conditional": { "condition": "ifdef", "expression": "IMGUI_USE_BGRA_PACKED_COLOR" },
                "true_case": {
                    "defines": [
                        { "name": "IM_COL32_R_SHIFT", "content": "16" },
                        { "name": "IM_COL32_G_SHIFT", "content": "8" },
                        { "name": "IM_COL32_B_SHIFT", "content": "0" },
                        { "name": "IM_COL32_A_SHIFT", "content": "24" },
                        { "name": "IM_COL32_A_MASK", "content": "0xFF000000" },
                    ],
                },
                "false_case": {
                    "defines": [
                        { "name": "IM_COL32_R_SHIFT", "content": "0" },
                        { "name": "IM_COL32_G_SHIFT", "content": "8" },
                        { "name": "IM_COL32_B_SHIFT", "content": "16" },
                        { "name": "IM_COL32_A_SHIFT", "content": "24" },
                        { "name": "IM_COL32_A_MASK", "content": "0xFF000000" },
                    ],
                },
            }
        },
    },
]

This would be a special case only for defines, since this is the only section where it gets weird.

ShironekoBen commented 9 months ago

I have to admit I'd not really considered the case of evaluating conditional prerequisites on defines themselves, only on other declarations (my assumption being that other languages likely didn't support defines, or wouldn't want to import the C ones as they are mostly fairly C-specific...

Internally the DOM representation of ifdefs is closer to the representation you propose here, so it's definitely not impossible to implement, but I'm slightly nervous about trying to expose that in the JSON because it feels like it will make everything much more complex - consistency would suggest that other declarations should be handled in a similar way, but then we end up needing to wrap everything in conditional blocks... (and also start caring about declaration ordering in the JSON, too)

From what I can see the only case where this is actually an issue right now is the IM_COL32... macros - given their nature I'm wondering if it would make more sense to just add a special-case for those for now that optionally removes the guard define (making the assumption that it's actually pretty rare for people to override them, at least in a scenario where bindings are involved). Does that sound like it would be a sensible solution in the immediate term?