json.hpp forcibly defines GCC_VERSION #417

Closed
polesapart opened this Issue Jan 4, 2017 · 3 comments

Projects

None yet

3 participants

@polesapart
polesapart commented Jan 4, 2017 edited

While json.hpp defines CLANG_VERSION & GCC_VERSION where appropriate:

// exclude unsupported compilers
#if defined(__clang__)
    #define CLANG_VERSION (__clang_major__ * 10000 + __clang_minor__ * 100 +     __clang_patchlevel__)
    #if CLANG_VERSION < 30400
        #error "unsupported Clang version - see https://github.com/nlohmann/json#supported-compilers"
    #endif
#elif defined(__GNUC__)
    #define GCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 +     __GNUC_PATCHLEVEL__)
    #if GCC_VERSION < 40900
        #error "unsupported GCC version - see https://github.com/nlohmann/json#supported-compilers"
    #endif
#endif

It fails to take a few facts into account:

  1. At least GCC_VERSION is used elsewhere (several open source projects such as python, mysql, etc). Leaking it leads to errors such as this: components/third_party/json.hpp:66:0: error: GCC_VERSION" redefined [-Werror]

  2. The macro is only used on the spot, so it seems like a better use would be something like that:

     #elif defined(__GNUC__)
         #define JSGCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 +     __GNUC_PATCHLEVEL__)
        #if JSGCC_VERSION < 40900
        ...
        #undef JSGCC_VERSION
    #endif /* defined __GNUC__ */
    

Or something.

@nlohmann
Owner
nlohmann commented Jan 4, 2017

Good point!

@gregmarr
gregmarr commented Jan 4, 2017

This macro isn't even really needed:

#elif defined(__GNUC__)
    #if (__GNUC__ < 4) || ((__GNUC__ == 4) && (__GNUC_MINOR__ < 9))
        #error "unsupported GCC version - see https://github.com/nlohmann/json#supported-compilers"
    #endif
#endif /* defined __GNUC__ */
@nlohmann nlohmann added a commit that referenced this issue Jan 4, 2017
@nlohmann 🐛 fix for #417 9f6c86f
@nlohmann
Owner
nlohmann commented Jan 4, 2017

I changed the code so that no symbol is defined.

@nlohmann nlohmann self-assigned this Jan 4, 2017
@nlohmann nlohmann added this to the Release 2.0.11 milestone Jan 4, 2017
@nlohmann nlohmann closed this Jan 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment