Set warnings on tests as well as src (halide#8022)

* Don't use variable-length arrays There was a rogue use of VLAs (an extension we don't want to use) in one of the runtime tests. Fixed the test. I'll follow up with a separate PR to ensure this warning is enabled everywhere to flush out other usages. * Set warnings on tests as well as src
ardier · Mar 3, 2024 · 97c323b · 97c323b
1 parent 528b381
commit 97c323b
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 67 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -96,6 +96,80 @@ if (TARGET_VULKAN)
     set(TARGET_SPIRV ON) # required
 endif()
 
+# Helper function to set C++ compiler warnings in a sane way
+function(set_halide_compiler_warnings NAME)
+    target_compile_options(
+        ${NAME}
+        PRIVATE
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wall>
+
+        # variable length arrays in C++ are a Clang extension, we don't want to use them
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wvla-extension>
+
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wcast-qual>
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wignored-qualifiers>
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Woverloaded-virtual>
+
+        $<$<CXX_COMPILER_ID:GNU>:-Wsuggest-override>
+
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Winconsistent-missing-destructor-override>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Winconsistent-missing-override>
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wdeprecated-declarations>
+
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-double-promotion>
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-float-conversion>
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-float-equal>
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-missing-field-initializers>
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-old-style-cast>
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-shadow>
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-sign-conversion>
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-switch-enum>
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-undef>
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-unused-function>
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-unused-macros>
+        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-unused-parameter>
+
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-c++98-compat-pedantic>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-c++98-compat>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-cast-align>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-comma>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-covered-switch-default>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-documentation-unknown-command>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-documentation>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-exit-time-destructors>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-global-constructors>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-implicit-float-conversion>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-implicit-int-conversion>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-implicit-int-float-conversion>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-missing-prototypes>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-nonportable-system-include-path>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-reserved-id-macro>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-shadow-field-in-constructor>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-shadow-field>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-shorten-64-to-32>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-undefined-func-template>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-unused-member-function>
+        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-unused-template>
+
+        # This warning was removed in Clang 13
+        $<$<AND:$<CXX_COMPILER_ID:Clang,AppleClang>,$<VERSION_LESS:$<CXX_COMPILER_VERSION>,13.0>>:-Wno-return-std-move-in-c++11>
+
+        $<$<CXX_COMPILER_ID:MSVC>:/W3>
+        $<$<CXX_COMPILER_ID:MSVC>:/wd4018>  # 4018: disable "signed/unsigned mismatch"
+        $<$<CXX_COMPILER_ID:MSVC>:/wd4141>  # 4141: 'inline' used more than once
+        $<$<CXX_COMPILER_ID:MSVC>:/wd4146>  # 4146: unary minus applied to unsigned type
+        $<$<CXX_COMPILER_ID:MSVC>:/wd4244>  # 4244: conversion, possible loss of data
+        $<$<CXX_COMPILER_ID:MSVC>:/wd4267>  # 4267: conversion from 'size_t' to 'int', possible loss of data
+        $<$<CXX_COMPILER_ID:MSVC>:/wd4291>  # 4291: No matching operator delete found
+        $<$<CXX_COMPILER_ID:MSVC>:/wd4503>  # 4503: disable "decorated name length exceeded, name was truncated"
+        $<$<CXX_COMPILER_ID:MSVC>:/wd4800>  # 4800: forcing value to bool 'true' or 'false' (performance warning)
+
+        # No: enable deprecation warnings
+        # $<$<CXX_COMPILER_ID:MSVC>:/wd4996>  # 4996: compiler encountered deprecated declaration
+    )
+endfunction()
+
+
 ##
 # Import dependencies
 ##

diff --git a/cmake/HalideTestHelpers.cmake b/cmake/HalideTestHelpers.cmake
@@ -54,6 +54,7 @@ function(add_halide_test TARGET)
     add_test(NAME ${TARGET}
              COMMAND ${args_COMMAND} ${args_ARGS}
              WORKING_DIRECTORY "${args_WORKING_DIRECTORY}")
+    set_halide_compiler_warnings(${TARGET})
 
     # We can't add Halide::TerminateHandler here, because it requires Halide::Error
     # and friends to be present in the final linkage, but some callers of add_halide_test()

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -575,73 +575,7 @@ endif ()
 ##
 # Set compiler options for libHalide
 ##
-
-target_compile_options(
-        Halide
-        PRIVATE
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wall>
-
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wcast-qual>
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wignored-qualifiers>
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Woverloaded-virtual>
-
-        $<$<CXX_COMPILER_ID:GNU>:-Wsuggest-override>
-
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Winconsistent-missing-destructor-override>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Winconsistent-missing-override>
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wdeprecated-declarations>
-
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-double-promotion>
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-float-conversion>
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-float-equal>
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-missing-field-initializers>
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-old-style-cast>
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-shadow>
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-sign-conversion>
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-switch-enum>
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-undef>
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-unused-function>
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-unused-macros>
-        $<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-unused-parameter>
-
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-c++98-compat-pedantic>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-c++98-compat>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-cast-align>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-comma>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-covered-switch-default>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-documentation-unknown-command>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-documentation>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-exit-time-destructors>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-global-constructors>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-implicit-float-conversion>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-implicit-int-conversion>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-implicit-int-float-conversion>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-missing-prototypes>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-nonportable-system-include-path>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-reserved-id-macro>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-shadow-field-in-constructor>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-shadow-field>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-shorten-64-to-32>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-undefined-func-template>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-unused-member-function>
-        $<$<CXX_COMPILER_ID:Clang,AppleClang>:-Wno-unused-template>
-
-        # This warning was removed in Clang 13
-        $<$<AND:$<CXX_COMPILER_ID:Clang,AppleClang>,$<VERSION_LESS:$<CXX_COMPILER_VERSION>,13.0>>:-Wno-return-std-move-in-c++11>
-
-        $<$<CXX_COMPILER_ID:MSVC>:/W3>
-        $<$<CXX_COMPILER_ID:MSVC>:/wd4018>  # 4018: disable "signed/unsigned mismatch"
-        $<$<CXX_COMPILER_ID:MSVC>:/wd4141>  # 4141: 'inline' used more than once
-        $<$<CXX_COMPILER_ID:MSVC>:/wd4146>  # 4146: unary minus applied to unsigned type
-        $<$<CXX_COMPILER_ID:MSVC>:/wd4244>  # 4244: conversion, possible loss of data
-        $<$<CXX_COMPILER_ID:MSVC>:/wd4267>  # 4267: conversion from 'size_t' to 'int', possible loss of data
-        $<$<CXX_COMPILER_ID:MSVC>:/wd4291>  # 4291: No matching operator delete found
-        $<$<CXX_COMPILER_ID:MSVC>:/wd4503>  # 4503: disable "decorated name length exceeded, name was truncated"
-        $<$<CXX_COMPILER_ID:MSVC>:/wd4800>  # 4800: forcing value to bool 'true' or 'false' (performance warning)
-
-        # No: enable deprecation warnings
-        # $<$<CXX_COMPILER_ID:MSVC>:/wd4996>  # 4996: compiler encountered deprecated declaration
-)
+set_halide_compiler_warnings(Halide)
 
 if (CMAKE_GENERATOR MATCHES "Visual Studio")
     # We could expose the /MP flag to all targets, but that might end up saturating the build

diff --git a/test/runtime/CMakeLists.txt b/test/runtime/CMakeLists.txt
@@ -15,6 +15,7 @@ function(_set_target_options NAME)
         COMPILING_HALIDE_RUNTIME
         COMPILING_HALIDE_RUNTIME_TESTS
     )
+    set_halide_compiler_warnings(${NAME})
 endfunction()
 
 function(halide_define_runtime_internal_test NAME)