dearimgui / dear_bindings

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

ImDrawIdx may be used before being defined #12

Closed rokups closed 2 years ago

rokups commented 2 years ago

ImDrawIdx is used in ImVector_ImDrawIdx_t before it is defined later. This happens only if user does not define ImDrawIdx in imconfig.h.

ShironekoBen commented 2 years ago

Unless this is a different issue, I think this is the thing where ImDrawIdx needs to be defined before ImVector, because when the instantiation of ImVector<ImDrawIdx> gets generated it (for obvious reasons) needs the declaration. In the previous thread when we discussed this I think we came to the conclusion that the best/easiest fix here was just to move the ImDrawIdx declaration up above ImVector in the main imgui.h file itself - does that still seem like the best solution @ocornut ?

rokups commented 2 years ago

Can confirm moving ImDrawIdx to the start of imgui.h fixes the issue so that would be a lazy way out. I also noticed that ImDrawIdx was already defined before ImVector<ImDrawIdx> is first used in imgui.h so bindings generator does not follow order of how things appear in file 100% it seems.

ShironekoBen commented 2 years ago

Cool, glad that's working for you now! I did actually look at generating template instantiations at the point-of-use rather than point-of-declaration, but that had it's own issues - my memory on what exactly went wrong is hazy, but in the original thread I wrote:

I had to do one very small hack to imgui.h - moving the definition of ImDrawIdx up above ImVector<>, as otherwise when things try to instantiate ImVector the generated code references it before it is declared (I spent a while trying to work around that by moving the generated template instantiations down, but that ends up being a bit of a catch-22 situation as then they can land after code that references them, and you can't forward-declare it because it's used as a value member in a struct).

...so fixing it that way would likely require a bit more of complicated dependency resolution pass or something. Thus if we can avoid it by moving the ImDrawIdx typedef up a bit in master that would definitely be my preferred solution!

ocornut commented 2 years ago

Interestingly, the problem is that unlike other ImVector_ instances, ImDrawIdx is not a struct so it doesn't get forward-declared.

typedef struct ImVector_ImDrawVert_t
{
    int Size;
    int Capacity;
    ImDrawVert* Data; // this benefit from forward declaration of ImDrawVert
} ImVector_ImDrawVert;
typedef struct ImVector_ImDrawIdx_t
{
    int Size;
    int Capacity;
    ImDrawIdx* Data; // this doesn't
} ImVector_ImDrawIdx;

I don't mind moving it higher in the file, but out of curiosity, I assume C doesn't allow to forward declare a typedef?

ocornut commented 2 years ago

Made the change here https://github.com/ocornut/imgui/commit/e6ffc0429107b5356f215fb450561b19ca2569a1

ShironekoBen commented 2 years ago

Cool, thanks!

FWIW, I'm reasonably sure forward declaration of typedefs isn't a thing, simply because I can't really imagine how it would work... normal forward declarations work because for a lot of purposes the compiler only cares about the broad idea that "I know XYZ is a struct/class and therefore in this situation I can treat it as a pointer-to-some-opaque-thing-I-don't-really-care-about-the-insides-of", but with a typedef just knowing that something "is" a typedef isn't really helpful because although syntactically you would then know it's a type you wouldn't have any clue if it's an elemental type, a struct, a pointer or whatever. And at the point where you've provided enough information to resolve that, you've basically given the entire definition of the typedef anyway so it's pretty moot!