<table><tr><td style="">rjvbb requested changes to this revision.<br />rjvbb added a comment.<br />This revision now requires changes to proceed.
</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/D18547">View Revision</a></tr></table><br /><div><div><p>See also <a href="https://phabricator.kde.org/D16894" class="remarkup-link" target="_blank" rel="noreferrer">https://phabricator.kde.org/D16894</a> which (initially) aimed to tackle this in a more general fashion.</p>

<p>IMHO the way forward (if my proposal is not acceptable) is to replace the existing clang version check with an actual test if the compiler supports the flac (CXXCompilerCheckFlag IIRC):</p>

<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);">if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") 
  if (CXXCompilerCheckFlag("-foo")
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} foo")
  endif()
endif()</pre></div>

<p>That avoids the overhead when GCC is used but also won't give false positives or negatives when clang is used because the compiler/version detection was just off.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18547#inline-102643">View Inline</a><span style="color: #4b4d51; font-weight: bold;">KDEFrameworkCompilerSettings.cmake:75</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="bright"></span><span style="color: #304a96"><span class="bright">if</span></span><span class="bright"> </span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span style="color: #766510"><span class="bright">CMAKE_CXX_COMPILER_ID</span></span><span class="bright"> </span><span style="color: #766510"><span class="bright">MATCHES</span></span><span class="bright"> </span><span style="color: #766510"><span class="bright">"C</span>lang<span class="bright">"</span></span><span class="bright"></span><span class="p"><span class="bright">)</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="bright"></span><span style="color: #74777d"><span class="bright"># don't enable with apple c</span>lang</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">if</span> <span class="p">(</span><span style="color: #766510">CMAKE_CXX_COMPILER_ID</span> <span style="color: #766510">STREQUAL</span> <span style="color: #766510">"Clang"</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">   <span style="color: #304a96">if</span> <span class="p">(</span><span style="color: #766510">NOT</span> <span style="color: #766510">CMAKE_CXX_COMPILER_VERSION</span> <span style="color: #766510">VERSION_LESS</span> <span style="color: #766510">"5.0.0"</span><span class="p">)</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I'm amazed that this works because CMake *may* detect Apple's clang as AppleClang depending on version and how policy 25 (IIRC) is set. Annoyingly that policy can only be set before the toplevel project statement, i.e. not in the ECM.</p>

<p style="padding: 0; margin: 8px;">In addition, Apple's versioning is very hard if not impossible to match with upstream versioning.</p></div></div></div></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/D18547">https://phabricator.kde.org/D18547</a></div></div><br /><div><strong>To: </strong>vonreth, aacid, apol, dfaure, rjvbb, bcooksley<br /><strong>Cc: </strong>aacid, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns<br /></div>