<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/111661/">http://git.reviewboard.kde.org/r/111661/</a>
     </td>
    </tr>
   </table>
   <br />










<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On July 23rd, 2013, 7:02 p.m. UTC, <b>Alexander Neundorf</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  



<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="http://git.reviewboard.kde.org/r/111661/diff/1/?file=173101#file173101line239" style="color: black; font-weight: bold; text-decoration: underline;">kde-modules/KDECompilerSettings.cmake</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

    </span>
   </th>
  </tr>
 </thead>



 
 

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">238</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">if</span><span class="p">(</span><span class="s2">"${CMAKE_CXX_COMPILER_ID}"</span> <span class="s">STREQUAL</span> <span class="s2">"GNU"</span> <span class="s">OR</span> <span class="s2">"${CMAKE_CXX_COMPILER_ID}"</span> <span class="s">STREQUAL</span> <span class="s2">"Clang"</span><span class="p">)</span></pre></td>
  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Is this patch complete ?
This part looks wrong.

This is for the lines where the version is checked, right ?
I guess there should be a check in line 219 to accept also Clang, and in line 221 to accept only GNU (for the version check).

I also don't see why you need the endif() and opening if() in line 235.</pre>
 </blockquote>





</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Well, both achieve the same result - i.e. keep the version check GNU only. I just splitted the conditionals, into a GNU specific part and a shared GNU/Clang part. So which one do you prefer, I really don't care.</pre>
<br />




<p>- Milian</p>


<br />
<p>On July 23rd, 2013, 6:53 p.m. UTC, Milian Wolff wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for Build System, Andreas Pakulat and Alexander Neundorf.</div>
<div>By Milian Wolff.</div>


<p style="color: grey;"><i>Updated July 23, 2013, 6:53 p.m.</i></p>






<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Add support for Clang in KDECompilerSettings.cmake.

This more or less copies what was done for KDE4 in https://git.reviewboard.kde.org/r/111612/ .
</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">With the changes applied I successfully installed a hello world application. The linker and compiler command lines seem to be correct and include all extended features.
I also checked and debugfull also works as expected (-g3 is added).

milian@minime:~/projects/kde4/test-ecm/build$ cmake ..
-- The C compiler identification is Clang 3.3.0
-- The CXX compiler identification is Clang 3.3.0
-- Check for working C compiler: /usr/bin/ccache
-- Check for working C compiler: /usr/bin/ccache -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/ccache
-- Check for working CXX compiler: /usr/bin/ccache -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Performing Test __KDE_HAVE_W_OVERLOADED_VIRTUAL
-- Performing Test __KDE_HAVE_W_OVERLOADED_VIRTUAL - Success
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY - Success
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY - Success
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR - Success
-- Configuring done
-- Generating done
-- Build files have been written to: /home/milian/projects/kde4/test-ecm/build

milian@minime:~/projects/kde4/test-ecm/build$ make VERBOSE=1
makeobj[0]: Entering directory `/home/milian/projects/kde4/test-ecm/build'
/usr/bin/cmake -H/home/milian/projects/kde4/test-ecm -B/home/milian/projects/kde4/test-ecm/build --check-build-system CMakeFiles/Makefile.cmake 0
/usr/bin/cmake -E cmake_progress_start /home/milian/projects/kde4/test-ecm/build/CMakeFiles /home/milian/projects/kde4/test-ecm/build/CMakeFiles/progress.marks
/usr/bin/make -f CMakeFiles/Makefile2 all
make[1]: Entering directory `/home/milian/projects/kde4/test-ecm/build'
/usr/bin/make -f CMakeFiles/hello.dir/build.make CMakeFiles/hello.dir/depend
make[2]: Entering directory `/home/milian/projects/kde4/test-ecm/build'
cd /home/milian/projects/kde4/test-ecm/build && /usr/bin/cmake -E cmake_depends "Unix Makefiles" /home/milian/projects/kde4/test-ecm /home/milian/projects/kde4/test-ecm /home/milian/projects/kde4/test-ecm/build /home/milian/projects/kde4/test-ecm/build /home/milian/projects/kde4/test-ecm/build/CMakeFiles/hello.dir/DependInfo.cmake --color=
Dependee "/home/milian/projects/kde4/test-ecm/build/CMakeFiles/hello.dir/DependInfo.cmake" is newer than depender "/home/milian/projects/kde4/test-ecm/build/CMakeFiles/hello.dir/depend.internal".
Dependee "/home/milian/projects/kde4/test-ecm/build/CMakeFiles/CMakeDirectoryInformation.cmake" is newer than depender "/home/milian/projects/kde4/test-ecm/build/CMakeFiles/hello.dir/depend.internal".
Scanning dependencies of target hello
make[2]: Leaving directory `/home/milian/projects/kde4/test-ecm/build'
/usr/bin/make -f CMakeFiles/hello.dir/build.make CMakeFiles/hello.dir/build
make[2]: Entering directory `/home/milian/projects/kde4/test-ecm/build'
/usr/bin/cmake -E cmake_progress_report /home/milian/projects/kde4/test-ecm/build/CMakeFiles 1
[100%] Building CXX object CMakeFiles/hello.dir/main.cpp.o
/usr/bin/ccache  /usr/bin/clang++ -Qunused-arguments  -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_FROM_BYTEARRAY -DQT_NO_CAST_TO_ASCII -DQT_NO_SIGNALS_SLOTS_KEYWORDS -DQT_NO_URL_CAST_FROM_STRING -DQT_STRICT_ITERATORS -DQT_USE_FAST_CONCATENATION -DQT_USE_FAST_OPERATOR_PLUS -D_BSD_SOURCE -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -D_XOPEN_SOURCE=500 -Wnon-virtual-dtor -Wno-long-long -Wundef -Wcast-align -Wchar-subscripts -Wall -W -Wpointer-arith -Wformat-security -fno-exceptions -DQT_NO_EXCEPTIONS -fno-check-new -fno-common -Woverloaded-virtual -Werror=return-type -fvisibility=hidden -fvisibility-inlines-hidden -O2 -g -DNDEBUG -DQT_NO_DEBUG   -o CMakeFiles/hello.dir/main.cpp.o -c /home/milian/projects/kde4/test-ecm/main.cpp
Linking CXX executable hello
/usr/bin/cmake -E cmake_link_script CMakeFiles/hello.dir/link.txt --verbose=1
/usr/bin/ccache  /usr/bin/clang++ -Qunused-arguments   -Wnon-virtual-dtor -Wno-long-long -Wundef -Wcast-align -Wchar-subscripts -Wall -W -Wpointer-arith -Wformat-security -fno-exceptions -DQT_NO_EXCEPTIONS -fno-check-new -fno-common -Woverloaded-virtual -Werror=return-type -fvisibility=hidden -fvisibility-inlines-hidden -O2 -g -DNDEBUG -DQT_NO_DEBUG  -Wl,--enable-new-dtags   CMakeFiles/hello.dir/main.cpp.o  -o hello -rdynamic 
make[2]: Leaving directory `/home/milian/projects/kde4/test-ecm/build'
/usr/bin/cmake -E cmake_progress_report /home/milian/projects/kde4/test-ecm/build/CMakeFiles  1
[100%] Built target hello
make[1]: Leaving directory `/home/milian/projects/kde4/test-ecm/build'
/usr/bin/cmake -E cmake_progress_start /home/milian/projects/kde4/test-ecm/build/CMakeFiles 0
makeobj[0]: Leaving directory `/home/milian/projects/kde4/test-ecm/build'</pre>
  </td>
 </tr>
</table>




<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>kde-modules/KDECompilerSettings.cmake <span style="color: grey">(d72bd9e)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/111661/diff/" style="margin-left: 3em;">View Diff</a></p>







  </td>
 </tr>
</table>








  </div>
 </body>
</html>