D16894: [ECM] use a macro to add compiler flags conditionally

Sun Jan 27 22:13:05 GMT 2019

rjvbb added a comment.

  In D16894#400949 <https://phabricator.kde.org/D16894#400949>, @dfaure wrote:

  > This makes sense to me. Just the name "SUPPORTED_IF" is strange, when reading that, one thinks "well, if we know the compiler flag is supported, why are we testing that it is?".

  I agree it looks strange, but apparently the combination with the macro name isn't as unambiguous as I hoped.

  The idea is: add the given option(s) but only If they are Supported by the compiler ... and they are Supported If the given expression is true.

  > I think this should be something like TRY_IF.

  That reads nicer, but isn't exact either because a priori there will be no more trying if the expression is true...

  I'll change the macro name to `ECM_ADD_<LANG>_FLAGS_CONDITIONALLY` and the parameter name to `CONDITION`, let's see how that works out.

  > Then it's clearer that no harm will occur if we set a too low compiler version after TRY_IF, it's just an optimization to avoid e.g. testing all gcc flags on MSVC and vice-versa.

  Uhm? That was not the intention. Currently if a TRY_IF condition is present then it is the only criterium used, UNLESS the compiler is Clang/AppleClang on APPLE. In that latter case the compiler is queried; this is also the case when no conditional expression is specified.

  I can of course inverse that logic, and make the condition a filter for when to query the compiler. At first sight I'd say that the conditional expressions won't need adapting but I have a feeling it might not be as simple.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D16894

To: rjvbb, #build_system, kfunk
Cc: dfaure, kfunk, apol, kde-frameworks-devel, kde-buildsystem, #build_system, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20190127/441b5b13/attachment-0001.html>