Header guard is generated incorrectly

dearimgui / dear_bindings

C header (and language binding metadata) generator for Dear ImGui

MIT License

221 stars 12 forks source link

Header guard is generated incorrectly #15

Closed PathogenDavid closed 2 years ago

PathogenDavid commented 2 years ago

The header guard for cimgui.h is currently generated as:

#ifndef IMGUI_H

// dear imgui, v1.84 WIP
// ... rest of the header here ...

#endif // #ifndef IMGUI_H

There's two issues here:

IMGUI_H is never defined so it's not actually guarding from multiple inclusion.
The name of the constant is misleading, it should be CIMGUI_H

As an aside, this include guard could/should probably just be #pragma once, same as Dear ImGui's headers.

ocornut commented 2 years ago

I think for that one you could safely make a pr/patch.

PathogenDavid commented 2 years ago

The thought crossed my mind but it wasn't actually immediately obvious to me what is even responsible for writing the header guard out in the first place.

PathogenDavid commented 2 years ago

It's handled here: mod_remove_pragma_once.py.

My preference would be to just remove this entirely and use #pragma once because it's 2021, but I wouldn't want to do so without hearing why @ShironekoBen added it in the first place. (As far as I know there aren't any relevant C compilers that don't support it these days.)

ShironekoBen commented 2 years ago

That's an interesting point - the replacement is simply because I assumed there were probably C compilers still out there somewhere that didn't support #pragma once. But I'm not really in the habit of using actual-genuine-C-compilers much (as opposed to C++ compilers compiling C code), so that was purely a guess on my part.

However it's definitely a bug that the #define gets dropped - that's probably the result of some changes I made to how the define element in the DOM works. I'll add in a fix for that (and the name, which IIRC ends up like that because it's based off the source filename not the destination one), but if the feeling is that actually #pragma once is fine for any reasonable use then we can also disable it (maybe leaving the code around in case someone does find a use-case for it).

ocornut commented 2 years ago

In my experience in C++ land #pragma once has been fine for a whole. In the case of C users I suspect they are even more likely to be using modern toolchain as C really improved in the last decade. Just using #pragma once is also the best way to eventually find out if it may be a problem for some.

ShironekoBen commented 2 years ago

OK, I've fixed the #define code generation, and also disabled removal of #pragma once by default now. 7e3cd50a39805022e09e7d379b80ac966ff667f9

ocornut commented 2 years ago

Seems good to me, thanks!