floooh / sokol

minimal cross-platform standalone C headers
https://floooh.github.io/sokol-html5
zlib License
6.54k stars 469 forks source link

Possible bug in #define _SAPP_CLEAR_ARC_STRUCT(type, item) #976

Closed garettbass closed 5 months ago

garettbass commented 5 months ago

The conditional block that defines _SAPP_CLEAR_ARC_STRUCT checks defined(_SAPP_MACOS) || defined(_SAPP_IOS) before those macros are defined:

#if defined(_SAPP_MACOS) || defined(_SAPP_IOS)
    // this is ARC compatible
    #if defined(__cplusplus)
        #define _SAPP_CLEAR_ARC_STRUCT(type, item) { item = type(); }
    #else
        #define _SAPP_CLEAR_ARC_STRUCT(type, item) { item = (type) { 0 }; }
    #endif
#else
    #define _SAPP_CLEAR_ARC_STRUCT(type, item) { _sapp_clear(&item, sizeof(item)); }
#endif

/// ...

        #define _SAPP_MACOS (1)

/// ...

        #define _SAPP_IOS (1)

As a result, it looks like _SAPP_CLEAR_ARC_STRUCT is always defined as:

#define _SAPP_CLEAR_ARC_STRUCT(type, item) { _sapp_clear(&item, sizeof(item)); }

If this is behaving correctly, perhaps we can remove the other conditional definitions? Happy to make this change, just uncertain what the correct result should be.

floooh commented 5 months ago

Whoopsie, that must have been a copy paste error (in sokol_gfx.h it's correct. It's a bit strange that clang doesn't warn when a struct with ObjC references is erased via memset).

Even when wrong the current behaviour doesn't do harm, because the struct is only cleared on entry and exit, but we should fix it properly by moving the _SAPP_CLEAR_ARC_STRUCT macro below the platform defines.

I can do a quick fix in the evening. Thanks for making me aware of that!