rjvbb added a comment.
> Thus these places need to be turned into:
>
> ...
> if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND
CMAKE_CXX_COMPILER_VERSION VERSION_LESS "3.8")
> elseif(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" AND
CMAKE_CXX_COMPILER_VERSION VERSION_LESS "8.1.0")
As you know this doesn't work reliably: whether it works or not depends on
cmake version and on CMP0025 - and thus requires all projects to set that
policy before calling `project`. I don't think that's an option, nor is
expecting Mac users to patch all toplevel CMake files of projects they want to
work with (or patch CMake so it sets the policy which is basically what I do on
my end). I also vaguely recall that cmake used the dotless AppleClang version
in earlier versions (= 810 instead of 8.1.0).
Besides, all those repeated separate ifs and elses do not make the code
easier to read or parse. Isn't it the whole purpose of having macros to reduce
code duplication and to hide complexity in a single location so that the
intended behaviour becomes easier to follow?
This makes Aleix's suggestion just to use `compiler<lang>_check_flag` much
more appealing, despite the cost and the fact it's so easy to break.
> Statements like `CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT
CMAKE_CXX_COMPILER_VERSION VERSION_LESS 3.5` do not make sense.
That's a bit too strong. They make sense everywhere except on APPLE (and even
there they can be reliable).
> If you're unsure in which version of AppleClang a certain feature was
introduced then:
This is the gist of what I'm doing, except that I do not want to rely on
Apple clang being reported as AppleClang (= CMP0025 set to NEW).
> This "let's abuse ${ARGN} as code to be evaluated later" is pretty ugly.
It comes from a cmake ML post by a cmake dev (Brad Kind IIRC) so I guess it's
sanctioned ugliness (as so much in cmake syntax). The boolean operators are
only available in the IF functions so there aren't much options if you don't
want to use CmakeParseArguments. I'm already looking at doing just that for a
version that will look like `kde_add_supported_compiler_flags(FLAGS <flags>
SUPPORTED_IF <conditions>)`. The names (macro and keywords) are open for
discussion of course, and so is the addition of additional options.
I'll look at kdevelop's versions too. I have no real objection to favour my
own code over them if they work reliably. But now that you mention kdevelop:
I've long been forced to use stock compilers to build KDevelop (on Mac). I
always put that off to my AppleClang being too old but with this new knowledge
I suspect it's simply because something goes wrong in the compiler detection.
> I don't really understand why this branch is needed. The macro name name
suggests it does the compile check on all compilers. Again very misleading.
Erm, no?! The macro name suggests it runs a check, which is true. It doesn't
say which check...
(FWIW, `cmake_<lang>_compiler_check_flag` is equally misleading: it doesn't
check `${flag}` but `"${CMAKE_CXX_FLAGS} ${flag}"`)
REPOSITORY
R240 Extra CMake Modules
REVISION DETAIL
https://phabricator.kde.org/D16894
To: rjvbb, #build_system, kfunk
Cc: kfunk, apol, kde-frameworks-devel, kde-buildsystem, #build_system,
michaelh, ngraham, bruns
