catchorg / Catch2

A modern, C++-native, test framework for unit-tests, TDD and BDD - using C++14, C++17 and later (C++11 support is in v2.x branch, and C++03 on the Catch1.x branch)
https://discord.gg/4CWS9zD
Boost Software License 1.0
18.25k stars 3.01k forks source link

Refactor and centralize compiler detection #2094

Open horenmar opened 3 years ago

horenmar commented 3 years ago

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.

LinuxDevon commented 2 years ago

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.

horenmar commented 2 years ago

@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)
HoseynHeydari commented 2 years ago

@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__)```
LinuxDevon commented 2 years ago

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/

HoseynHeydari commented 2 years ago

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

horenmar commented 2 years ago

@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 commented 2 years ago

@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.