Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor and centralize compiler detection #2094

Open
horenmar opened this issue Nov 3, 2020 · 7 comments · May be fixed by #2410
Open

Refactor and centralize compiler detection #2094

horenmar opened this issue Nov 3, 2020 · 7 comments · May be fixed by #2410
Labels
Development Issues related to further development of Catch2 Good First Issue Issues that can be undertaken by someone new to the project Tweak request
Milestone

Comments

@horenmar
Copy link
Member

horenmar commented Nov 3, 2020

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 horenmar added Tweak request Development Issues related to further development of Catch2 labels Nov 3, 2020
@horenmar horenmar added this to the 3.0 milestone Nov 3, 2020
@horenmar horenmar added the Good First Issue Issues that can be undertaken by someone new to the project label Nov 3, 2020
@LinuxDevon
Copy link
Contributor

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
Copy link
Member Author

horenmar commented Aug 9, 2021

@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
Copy link

@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
Copy link
Contributor

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
Copy link

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

@horenmar
Copy link
Member Author

@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
Copy link
Member Author

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

HoseynHeydari added a commit to HoseynHeydari/Catch2 that referenced this issue Apr 14, 2022
HoseynHeydari added a commit to HoseynHeydari/Catch2 that referenced this issue Apr 14, 2022
HoseynHeydari added a commit to HoseynHeydari/Catch2 that referenced this issue Apr 14, 2022
HoseynHeydari added a commit to HoseynHeydari/Catch2 that referenced this issue Apr 14, 2022
HoseynHeydari added a commit to HoseynHeydari/Catch2 that referenced this issue Apr 14, 2022
HoseynHeydari added a commit to HoseynHeydari/Catch2 that referenced this issue Apr 15, 2022
HoseynHeydari added a commit to HoseynHeydari/Catch2 that referenced this issue Apr 15, 2022
HoseynHeydari added a commit to HoseynHeydari/Catch2 that referenced this issue Apr 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development Issues related to further development of Catch2 Good First Issue Issues that can be undertaken by someone new to the project Tweak request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants