[libcxx] r203637 - build: fix add_definition abuse in CMake

[Alexey Samsonov]

OK, for now I've just put -Werror to CMAKE_C_FLAGS/CMAKE_CXX_FLAGS instead
of calling add_definitions() for it. Perhaps, we may safely do this for all
compile flags w/o any additional work, as LLVM_DEFINITIONS doesn't seem to
be used or exported anywhere. Weird.


On Thu, Mar 13, 2014 at 5:33 AM, Saleem Abdulrasool
<compnerd at compnerd.org>wrote:

> On Wed, Mar 12, 2014 at 4:30 AM, Alexey Samsonov <samsonov at google.com>wrote:
>
>> Hi Saleem,
>>
>> This resurrects the error I tried to fix in r191814. See the error on our
>> buildbot:
>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/7812/steps/build%2064-bit%20llvm%20using%20clang/logs/stdio
>> -Werror flag, which is presumably added by add_llvm_definitions() in
>> HandleLLVMOptions.cmake appears *after* -Wno-error added by libcxx
>> CMakeLists.txt. Should we just get rid of add_llvm_definitions()
>> in LLVM's CMake files and add a flag to CMAKE_C_FLAGS and CMAKE_CXX_FLAGS
>> variables instead?
>>
>
> Ah, sorry about that.
>
> I think that llvm_add_definitions doesn't really add much and removal of
> it would be a nice cleanup.  It seems that the raison d'être for
> llvm_add_definitions is to track compile flags for llvm-config.  A general
> sweep of llvm_add_definition usage for compiler flags could be replaced
> with adding to the CMAKE_CXX_FLAGS (and CMAKE_C_FLAGS) and using
> add_definitions otherwise.  Once that is done, simply using the string
> replace and add_definitions to add a CPP macro for the compile flags would
> allow us to retain the desired functionality.
>
> On Wed, Mar 12, 2014 at 8:11 AM, Saleem Abdulrasool <compnerd at compnerd.org
>> > wrote:
>>
>>> Author: compnerd
>>> Date: Tue Mar 11 23:11:28 2014
>>> New Revision: 203637
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=203637&view=rev
>>> Log:
>>> build: fix add_definition abuse in CMake
>>>
>>> add_definitions is meant for adding C preprocessor definitions.  Modern
>>> cmake
>>> suggests use of the CMAKE_CXX_FLAGS for the purposes of pushing flags to
>>> the
>>> compilation commands.  Simply switch to the modern form given that we are
>>> already requiring a new enough cmake.
>>>
>>> Modified:
>>>     libcxx/trunk/CMakeLists.txt
>>>
>>> Modified: libcxx/trunk/CMakeLists.txt
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/libcxx/trunk/CMakeLists.txt?rev=203637&r1=203636&r2=203637&view=diff
>>>
>>> ==============================================================================
>>> --- libcxx/trunk/CMakeLists.txt (original)
>>> +++ libcxx/trunk/CMakeLists.txt Tue Mar 11 23:11:28 2014
>>> @@ -259,11 +259,14 @@ if (MSVC)
>>>    add_definitions(-D_CRT_SECURE_NO_WARNINGS)
>>>  endif()
>>>
>>> -add_definitions(
>>> - ${LIBCXX_CXX_REQUIRED_FLAGS}
>>> - ${LIBCXX_CXX_WARNING_FLAGS}
>>> - ${LIBCXX_CXX_FEATURE_FLAGS}
>>> - )
>>> +string(REPLACE ";" " " LIBCXX_CXX_REQUIRED_FLAGS
>>> "${LIBCXX_CXX_REQUIRED_FLAGS}")
>>> +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${LIBCXX_CXX_REQUIRED_FLAGS}")
>>> +
>>> +string(REPLACE ";" " " LIBCXX_CXX_WARNING_FLAGS
>>> "${LIBCXX_CXX_WARNING_FLAGS}")
>>> +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${LIBCXX_CXX_WARNING_FLAGS}")
>>> +
>>> +string(REPLACE ";" " " LIBCXX_CXX_FEATURE_FLAGS
>>> "${LIBCXX_CXX_FEATURE_FLAGS}")
>>> +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${LIBCXX_CXX_FEATURE_FLAGS}")
>>>
>>>
>>>  #===============================================================================
>>>  # Setup Source Code
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>
>>
>>
>> --
>> Alexey Samsonov, MSK
>>
>
>
>
> --
> Saleem Abdulrasool
> compnerd (at) compnerd (dot) org
>



-- 
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140313/97bd5805/attachment.html>