travis test for clang v 3.9 on osx; fix AppleClang detection #86
Conversation
| @@ -726,9 +726,9 @@ | |||
| // Extern template is supported. | |||
| #elif defined(EA_COMPILER_CPP11_ENABLED) && defined(__EDG_VERSION__) && (__EDG_VERSION__ >= 401) // EDG 4.1+. | |||
| // supported. | |||
| #elif defined(EA_COMPILER_CPP11_ENABLED) && defined(__clang__) && defined(__APPLE__) && (EA_COMPILER_VERSION >= 401) | |||
| #elif defined(EA_COMPILER_CPP11_ENABLED) && defined(__clang__) && defined(__apple_build_version__) && (EA_COMPILER_VERSION >= 401) | |||
There was a problem hiding this comment.
Why is the change from 'APPLE' to 'apple_build_version' required?
There was a problem hiding this comment.
__APPLE__ identifies the platform (Max OS X)
__apple_build_version__ identifies the compiler (AppleClang)
https://sourceforge.net/p/predef/wiki/Compilers/
https://sourceforge.net/p/predef/wiki/Home/
Based on documentation in that file, I believe the original intent was to recognize AppleClang and not the platform.
There was a problem hiding this comment.
Yup. Got that. But we are already testing for both "APPLE && clang" which would be equivalent. Unless I'm missing something? The clang would already prevent using GCC on MacOSX, for example.
There was a problem hiding this comment.
Ah you know what I just did a s/APPLE/apple_build_version/ on this file because I saw a couple bad examples. Let me revisit... In hindsight, I think the combination of APPLE clang is clearer than simply apple_build_version.
There was a problem hiding this comment.
Ahh I remember the problem now... if I have clang3.9 running on mac OS X, this is true:
defined(__clang__) && defined(__APPLE__)
because it is the case that the compiler is clang and the platform is apple.
There was a problem hiding this comment.
Why is that a problem? Sorry, I'm not following the issue that was causing as we change the "EA_COMPILER_VERSION" checks to use the AppleClang version numbers.
There was a problem hiding this comment.
The problem is clang3.9 is not AppleClang. It's just plain old "Clang" and so 3.9 looks like a really old version of AppleClang because 3.9 < 4.0.1 but really it should be hit the non-apple clang comparison (where any version of "Clang" is ok).
See this snippet:
#elif defined(EA_COMPILER_CPP11_ENABLED) && defined(__clang__) && defined(__apple_build_version__) && (EA_COMPILER_VERSION >= 401)
// Extern template is supported.
#elif defined(EA_COMPILER_CPP11_ENABLED) && defined(__clang__) && !defined(__apple_build_version__) // Clang other than Apple's Clang
// Extern template is supported.
Consider this simplification for the sake of explanation:
#if defined(__clang__) && defined(__apple_build_version__) && (EA_COMPILER_VERSION >= 401)
// condition 1
#if defined(__clang__) && !defined(__apple_build_version__)
// condition 2
#else
// condition 3 (this is bad, this turns off features)
In the simplified version using __apple_build_version__ on Mac OS X with a standard Clang3.9, it hits condition 2.
Consider this simplification for the sake of explanation:
#if defined(__clang__) && defined(__APPLE__) && (EA_COMPILER_VERSION >= 401)
// condition 1
#if defined(__clang__) && !defined(__APPLE__)
// condition 2
#else
// condition 3 (this is bad, this turns off features)
In the simplified version using __APPLE__ on Mac OS X with a standard Clang3.9, it cannot hit condition 1 because the compiler version is too small, it doesn't hit condition 2, and then it falls into condition 3.
|
Awesome! Thanks for the explanation. :) |
Fix for issue #68