[PATCH] D19895: cmake: Introduce TEST_SUITE_C_FLAGS, TEST_SUITE_CXX_FLAGS, etc.

[Matthias Braun via llvm-commits]

Yeah CMAKE_C_FLAGS_RELEASE would work but feels like a hack as well... We would need to override the flags for DEBUG, RELEASE, MINSIZEREL and RELWITHDEBINFO to be on the safe side. And I kinda liked the fact that we currently can have 1 cache file specifying optimization flags like -flto, -fomit-frame-pointer etc. and combine that with another cache file that just sets stuff up for a different target. Admittedly there are ways to get this working...

> On May 3, 2016, at 6:24 PM, Justin Bogner via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Matthias Braun via llvm-commits <llvm-commits at lists.llvm.org> writes:
>> MatzeB created this revision.
>> MatzeB added reviewers: cmatthews, kristof.beyls, jmolloy.
>> MatzeB added a subscriber: llvm-commits.
>> MatzeB set the repository for this revision to rL LLVM.
>> Herald added a subscriber: mcrosier.
>> 
>> Intuitively one would use CMAKE_C_FLAGS, CMAKE_CXX_FLAGS and
>> CMAKE_EXE_LINKER_FLAGS to inject custom flags into the build. However
>> this clashes with the use of cmake cache files which want to add flags as well.
>> A user specifying CMAKE_C_FLAGS would (accidentally) override the
>> flags set by the cache files.
> 
> AFAIK The usual trick is to set CMAKE_C_FLAGS_RELEASE or
> CMAKE_C_FLAGS_MINSIZEREL or whatever in the cache file, and leave the
> CMAKE_C_FLAGS for the command line user to do what they want with.
> 
>> The solution here is to introduce a new set of flags called
>> TEST_SUITE_C_FLAGS, TEST_SUITE_CXX_FLAGS and TEST_SUITE_EXE_LINKER_FLAGS
>> as a means for users to specify additional flags. This way the flags
>> will not override the settings from the cache file.
>> 
>> In order to educate users to use the new flags a warning message is
>> displayed if CMAKE_C_FLAGS is set.
>> 
>> Repository:
>>  rL LLVM
>> 
>> http://reviews.llvm.org/D19895
>> 
>> Files:
>>  CMakeLists.txt
>>  cmake/caches/util/arch_flags.cmake
>> 
>> Index: cmake/caches/util/arch_flags.cmake
>> ===================================================================
>> --- cmake/caches/util/arch_flags.cmake
>> +++ cmake/caches/util/arch_flags.cmake
>> @@ -2,3 +2,5 @@
>> set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${ARCH_FLAGS}" CACHE STRING "")
>> set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} ${ARCH_FLAGS}" CACHE STRING "")
>> set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${ARCH_FLAGS}" CACHE STRING "")
>> +# Silence the warning in the toplevel CMakeLists.txt.
>> +set(EXPECTED_CMAKE_C_FLAGS "${CMAKE_C_FLAGS}" CACHE STRING "")
>> Index: CMakeLists.txt
>> ===================================================================
>> --- CMakeLists.txt
>> +++ CMakeLists.txt
>> @@ -12,13 +12,33 @@
>> mark_as_advanced(CMAKE_INSTALL_PREFIX)
>> # On the other hand we often want to switch compiler or cflags
>> mark_as_advanced(CLEAR CMAKE_C_COMPILER CMAKE_CXX_COMPILER CMAKE_LINKER
>> -  CMAKE_C_FLAGS CMAKE_C_FLAGS_DEBUG CMAKE_C_FLAGS_MINSIZEREL
>> +  CMAKE_C_FLAGS_DEBUG CMAKE_C_FLAGS_MINSIZEREL
>>   CMAKE_C_FLAGS_RELEASE CMAKE_C_FLAGS_RELWITHDEBINFO
>> -  CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_MINSIZEREL
>> +  CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_MINSIZEREL
>>   CMAKE_CXX_FLAGS_RELEASE CMAKE_CXX_FLAGS_RELWITHDEBINFO
>> -  CMAKE_EXE_LINKER_FLAGS CMAKE_EXE_LINKER_FLAGS_DEBUG
>> -  CMAKE_EXE_LINKER_FLAGS_MINSIZEREL CMAKE_EXE_LINKER_FLAGS_RELEASE
>> -  CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO)
>> +  CMAKE_EXE_LINKER_FLAGS_DEBUG CMAKE_EXE_LINKER_FLAGS_MINSIZEREL
>> +  CMAKE_EXE_LINKER_FLAGS_RELEASE CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO)
>> +
>> +# Encourage people to use TEST_SUITE_C_FLAGS/TEST_SUITE_CXX_FLAGS so
>> +# CMAKE_C_FLAGS can be used exclusively by -C cachefiles for specifying
>> +# target flags without the user accidentally overriding them.
>> +set(EXPECTED_CMAKE_C_FLAGS "" CACHE STRING
>> +    "Allows cache files to disable the CMAKE_C_FLAGS warning message")
>> +mark_as_advanced(EXPECTED_CMAKE_C_FLAGS)
>> +if(NOT "${CMAKE_C_FLAGS}" STREQUAL "${EXPECTED_CMAKE_C_FLAGS}")
>> +  message(WARNING "
>> +Specifying CMAKE_C_FLAGS, CMAKE_CXX_FLAGS, \
>> +CMAKE_EXE_LINKER_FLAGS is discouraged. Use TEST_SUITE_C_FLAGS, \
>> +TEST_SUITE_CXX_FLAGS and TEST_SUITE_EXE_LINKER_FLAGS instead.\
>> +")
>> +endif()
>> +mark_as_advanced(CMAKE_C_FLAGS CMAKE_CXX_FLAGS CMAKE_EXE_LINKER_FLAGS)
>> +set(TEST_SUITE_C_FLAGS "" CACHE STRING "Flags used by the C compiler.")
>> +set(TEST_SUITE_CXX_FLAGS "${TEST_SUITE_C_FLAGS}" CACHE STRING "Flags used by the C++ compiler.")
>> +set(TEST_SUITE_EXE_LINKER_FLAGS "" CACHE STRING "Flags used by the linker.")
>> +set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${TEST_SUITE_C_FLAGS}")
>> +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${TEST_SUITE_CXX_FLAGS}")
>> +set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${TEST_SUITE_EXE_LINKER_FLAGS}")
>> 
>> project(test-suite C CXX)
>> 
>> 
>> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits