[test-suite] r255023 - [cmake] Add an option to suppress warnings in the test-suite

[James Molloy via llvm-commits]

Hi Matthias,

> Maybe we shouldn't add -DNDEBUG unconditionally

Without -DNDEBUG a load of the tests fail (to run, and some to compile), so I think -DNDEBUG should always be on. All I did was move it around in the file slightly.

> I think add_definitions is only meant for actual -D flags

Yeah, I tend to cheat and use it as a shorthand. I’ll replace it with what you suggest, and change to TEST_SUITE_ENABLE_WARNINGS.

James
> On 8 Dec 2015, at 23:21, Matthias Braun <mbraun at apple.com> wrote:
>
>
>> On Dec 8, 2015, at 7:36 AM, James Molloy via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>
>> Author: jamesm
>> Date: Tue Dec  8 09:36:47 2015
>> New Revision: 255023
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=255023&view=rev
>> Log:
>> [cmake] Add an option to suppress warnings in the test-suite
>>
>> Warnings are only useful when they are actionable, and we can't change the tests, so suppress them to avoid clutter during test runs.
>>
>> Modified:
>>   test-suite/trunk/CMakeLists.txt
>>
>> Modified: test-suite/trunk/CMakeLists.txt
>> URL: http://llvm.org/viewvc/llvm-project/test-suite/trunk/CMakeLists.txt?rev=255023&r1=255022&r2=255023&view=diff
>> ==============================================================================
>> --- test-suite/trunk/CMakeLists.txt (original)
>> +++ test-suite/trunk/CMakeLists.txt Tue Dec  8 09:36:47 2015
>> @@ -7,10 +7,14 @@ if (NOT CMAKE_BUILD_TYPE AND NOT CMAKE_C
>>  set(CMAKE_BUILD_TYPE "Release")
>> endif()
>>
>> -add_definitions(-DNDEBUG)
>> -
>> project(test-suite C CXX)
>>
>> +add_definitions(-DNDEBUG)
> Maybe we shouldn't add -DNDEBUG unconditionally. I also think having CMAKE_BUILD_TYPE set to "Release" already appends that for me anyway so maybe we don't need to do anything here?
>
>> +option(TEST_SUITE_SUPPRESS_WARNINGS "Suppress all warnings" ON)
>> +if(${TEST_SUITE_SUPPRESS_WARNINGS})
>> +  add_definitions(-w)
>> +endif()
> I think add_definitions is only meant for actual -D flags, this case should be:
>
> list(APPEND CFLAGS -w)
> list(APPEND CXXFLAGS -w)
>
> I think. Maybe the option should also be changed to TEST_SUITE_ENABLE_WARNINGS to be in line with LLVM_ENABLE_WARNINGS.
>
> - Matthias
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.