Catch2 icon indicating copy to clipboard operation
Catch2 copied to clipboard

Refactor and centralize compiler detection

Open horenmar opened this issue 3 years ago • 7 comments

Currently Catch2 detects compilers ad-hoc, at the place where the code needs to be compiler-specific. This brings a lot of trouble with more obscure compilers, because compilers like to masquerade for different compilers, such as Clang defining GCC-version macros, ICC defining both, IBM XL self-reporting as Clang, and so on. Because compiler-specific sections of code are often indeed compiler-specific, not detecting compilers properly can be a problem, e.g. Clang will complain that it found unknown pragma if we try to disable GCC-specific warning without checking that Clang is not used.

The solution is to perform more rigorous compiler detection, and centralize it, so that updates don't have to be done in multiple places.

horenmar avatar Nov 03 '20 16:11 horenmar

How do you envision this to happen? I only see it going as a giant chain of if/else. I currently pulled together a list of all the defines for all the compilers. Not sure if we should go about this trying to do macros for each compiler to ignore warnings per file or do we want global compiler configuration?

Just really wanted to get a little more detail about your thoughts on this.

LinuxDevon avatar Aug 05 '21 19:08 LinuxDevon

@LinuxDevon Yeah, the detection is going to be a big set of defined macros check, the idea is to do that somewhere centrally, so that when Catch2 needs to suppress GCC-specific warning, we can write something like

#if defined(CATCH_COMPILER_GCC)
// suppress
#endif

rather than iteratively having to stop different compilers from going into the same block, the way it is now, which can lead to this sort of code in multiple places

#if defined(__GNUC__) && !defined(__clang__) && !defined(_ICC)

horenmar avatar Aug 09 '21 22:08 horenmar

@horenmar Hi. I began collaborating with this issue as a "good first issue". Here is a list of distinct unique usages of the compiler's predefined macros. Is it desired to create a new macro as CATCH_COMPILER_GCC for each line?

#if defined(__APPLE__) && defined(__apple_build_version__) && (__clang_major__ < 10)
#if defined(__BORLANDC__)
#if defined(__CYGWIN__) || defined(__QNX__) || defined(__EMSCRIPTEN__) || defined(__DJGPP__)
#if defined(__DJGPP__)
#if defined(__EXCEPTIONS) || defined(__cpp_exceptions) || defined(_CPPUNWIND)
#if defined(__GNUC__)
#if defined(__GNUC__) && !defined(__clang__)
#if defined(__GNUC__) && !defined(__clang__) && !defined(__ICC) && !defined(__CUDACC__) && !defined(__LCC__)
#if defined(__GNUC__) && !defined(__clang__) && !defined(__ICC) && __GNUC__ < 10
#if defined(__GNUC__) && !defined(__clang__) && !defined(__ICC) && __GNUC__ < 9
#if defined(__GNUC__) && !defined(__clang__) && __GNUC__ <= 5
#if defined(__GNUC__) && (defined(__i386) || defined(__x86_64))
#if defined(__GNUC__) || defined(__clang__)
#if defined(__GNUG__) && !defined(__clang__)
#if defined(__MINGW32__)
#if defined(__ORBIS__)
#if defined(__clang__)
#if defined(__clang__) && !defined(_MSC_VER)
#if defined(__cpp_lib_is_invocable) && __cpp_lib_is_invocable >= 201703
#if defined(__cpp_lib_uncaught_exceptions) \
#if defined(__has_include)
#if defined(__i386__) || defined(__x86_64__)```

HoseynHeydari avatar Apr 08 '22 15:04 HoseynHeydari

I hadn't gotten around to this so that was my bad... that was my thought was that as you described. Setup a macro name that for each compiler.

You probably don't need the CATCH as long as you keep it in the namespace so you could do something like this maybe. Could go either way really.

namespace catch {
#define COMPILER_GCC ...
}

// Would read like this as long as you are in the namespace
#if defined(COMPILER_GCC) ...

I found this list and had planned to write them for each: https://sourceforge.net/p/predef/wiki/Compilers/

LinuxDevon avatar Apr 08 '22 20:04 LinuxDevon

As explained in this question #define may not bound by namespace.

HoseynHeydari avatar Apr 11 '22 21:04 HoseynHeydari

@HoseynHeydari I don't need a special macro per version, just per compiler. The goal is to replace these checks

#if defined(__GNUC__) && !defined(__clang__) && !defined(__ICC) && __GNUC__ < 10
#if defined(__GNUC__) && !defined(__clang__) && !defined(__ICC) && __GNUC__ < 9
#if defined(__GNUC__) && !defined(__clang__) && __GNUC__ <= 5

with ones looking like this

#if defined(CATCH_COMPILER_GCC) && __GNUC__ < 10
#if defined(CATCH_COMPILER_GCC) && __GNUC__ < 9
#if defined(CATCH_COMPILER_GCC) && __GNUC__ <= 5

horenmar avatar Apr 14 '22 08:04 horenmar

@LinuxDevon preprocessor macros are not bound by namespaces, or even C++ language structure. That's why their names are prefixed, because that essentially namespaces them per project.

horenmar avatar Apr 14 '22 08:04 horenmar