KDE compiler settings fail to consider AppleClang
ClosedPublic

Authored by rjvbb on Mar 17 2017, 5:02 PM.

Details

Reviewers
kfunk
Group Reviewers
Frameworks
Build System
Summary

KDECompilerSettings.cmake and KDEFrameworksCompilerSettings.cmake fail to consider the fact that cmake considers Apple's clang compiler to be different (differences that exist but are largely irrelevant for KDE and the ECM in particular).

I just ran into a failure building the KJs 5.32.0 framework as a result, using the system/Xcode clang compiler for OS X 10.9.5 . The build system had failed to put the compiler into C++11 mode.

This patch address the issue by checking the CMAKE_*_COMPILER_ID against both "Clang" and "AppleClang". A more elegant approach may be possible with regexp pattern matching but might cause problems if other clang flavours exist and match that shouldn't .

Test Plan

With this patch KJs builds without further errors. I will update this ticket should I run into unforeseen side-effects.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
rjvbb created this revision.Mar 17 2017, 5:02 PM
apol added a subscriber: apol.Mar 17 2017, 5:48 PM

Maybe it would make sense to use MATCHES? are there any other *Clang*?

rjvbb added a comment.Mar 17 2017, 6:16 PM
In D5089#95732, @apol wrote:

Maybe it would make sense to use MATCHES? are there any other *Clang*?

That was my first idea, use MATCHES "*Clang", but then I realised I have no idea if there are other SomethingClang. Searching the CMake sources for [^"]Clang" shows only AppleClang but I also see Android-Clang.cmake and Windows-Clang.cmake files. If we don't worry about the sudden future introduction of new AndroidClang, WindowsClang or FooClang IDs then yes, using MATCHES will be cleaner.

Come to think of it, it might actually be better because all possible Clang flavours will be covered. After all there's only 1 Clang check currently that's Apple specific, and I left that one alone because it shouldn't trigger on Mac.

I'll see what others think.

kfunk added a subscriber: kfunk.Mar 17 2017, 7:02 PM

+1 to apol's recommendation. ... MATCHES "Clang" is the expression everyone uses.

Even in cmake.git:

Modules/CMakeDetermineCCompiler.cmake
138:  if(CMAKE_C_COMPILER_ID MATCHES "GNU" OR CMAKE_C_COMPILER_ID MATCHES "Clang")

Also see: http://stackoverflow.com/questions/10046114/in-cmake-how-can-i-test-if-the-compiler-is-clang

kfunk requested changes to this revision.Mar 17 2017, 7:02 PM
This revision now requires changes to proceed.Mar 17 2017, 7:02 PM
rjvbb updated this revision to Diff 12583.Mar 18 2017, 11:36 AM
rjvbb edited edge metadata.

Uses MATCHES "Clang" .

Tested and appears to work with AppleClang Apple LLVM version 6.0 (clang-600.0.57) (based on LLVM 3.5svn)

kfunk accepted this revision.Mar 18 2017, 11:50 AM

LGTM

This revision is now accepted and ready to land.Mar 18 2017, 11:50 AM