memononen / nanosvg

Simple stupid SVG parser
zlib License
1.69k stars 357 forks source link

VS2012 compatibility #147

Open SergeySlice opened 5 years ago

SergeySlice commented 5 years ago

This definition

typedef struct NSVGpaint {
    char type;
    union {
        unsigned int color;
        NSVGgradient* gradient;
    };
} NSVGpaint;

causes warning C4201 nameless union can be corrected by

typedef struct NSVGpaint {
    char type;
    union {
        unsigned int color;
        NSVGgradient* gradient;
    } paint;
} NSVGpaint;
Albrecht-S commented 2 years ago

@SergeySlice Anonymous ("nameless") unions have a particular reason so you don't need to add the name of the union to access a particular member, for instance:

#include "nanosvg.h"
typedef struct NSVGpaint_new {          // your new struct
    char type;
    union {
        unsigned int color;
        NSVGgradient* gradient;
    } paint;
} NSVGpaint_new;

int test1(NSVGpaint *p) {               // using the original struct
  return p->color;                      // <-- simple access
}

int test2(NSVGpaint_new *p) {           // using your new struct
  return p->paint.color;                // <-- see added 'paint.'
}

[Note: code above tested in a simple C source file under Linux only.]

cppreference.com says "Members of an anonymous union are injected in the enclosing scope".

Note: this is C++, I'm not aware of differences to the C standard here (maybe it's not allowed in C?).

I'm not one of the authors of NanoSVG but I believe that this change would neither be useful nor acceptable. Even if the original author(s) would be willing to change the NanoSVG sources (which would likely be a lot of work) it would have an impact on all users using the defined structs in their own code (source compatibility would be affected).

That said, I don't see this warning in my (C++) compilation with VS 2019, even if I switch to warning level 4 (/W4) and disable MS extensions (/Za). Note that VS 2019 doesn't let me select C++ standard older than C++14. Is there anything else you'd need to do (set other compiler properties) to see this warning?

I propose to close this issue ("won't fix").

RokeJulianLockhart commented 2 years ago

@SergeySlice, does this problem continue with Visual Studio 2022? If it does, please replace the date within the title to demonstrate this.