<table><tr><td style="">rjvbb added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D16894">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">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")</pre></div></blockquote>

<p>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 <tt style="background: #ebebeb; font-size: 13px;">project</tt>. 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).</p>

<p>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?</p>

<p>This makes Aleix's suggestion just to use <tt style="background: #ebebeb; font-size: 13px;">compiler<lang>_check_flag</tt> much more appealing, despite the cost and the fact it's so easy to break.</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>Statements like <tt style="background: #ebebeb; font-size: 13px;">CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT CMAKE_CXX_COMPILER_VERSION VERSION_LESS 3.5</tt> do not make sense.</p></blockquote>

<p>That's a bit too strong. They make sense everywhere except on APPLE (and even there they can be reliable).</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>If you're unsure in which version of AppleClang a certain feature was introduced then:</p></blockquote>

<p>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).</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>This "let's abuse ${ARGN} as code to be evaluated later" is pretty ugly.</p></blockquote>

<p>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 <tt style="background: #ebebeb; font-size: 13px;">kde_add_supported_compiler_flags(FLAGS <flags> SUPPORTED_IF <conditions>)</tt>. The names (macro and keywords) are open for discussion of course, and so is the addition of additional options.</p>

<p>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.</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>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.</p></blockquote>

<p>Erm, no?! The macro name suggests it runs a check, which is true. It doesn't say which check...<br />
(FWIW, <tt style="background: #ebebeb; font-size: 13px;">cmake_<lang>_compiler_check_flag</tt> is equally misleading: it doesn't check <tt style="background: #ebebeb; font-size: 13px;">${flag}</tt> but <tt style="background: #ebebeb; font-size: 13px;">"${CMAKE_CXX_FLAGS} ${flag}"</tt>)</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R240 Extra CMake Modules</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D16894">https://phabricator.kde.org/D16894</a></div></div><br /><div><strong>To: </strong>rjvbb, Build System, kfunk<br /><strong>Cc: </strong>kfunk, apol, kde-frameworks-devel, kde-buildsystem, Build System, michaelh, ngraham, bruns<br /></div>