Merge pull request NVIDIA#1359 from allisonvacanti/enh/pedantic_flags/gh.cub228

Enable more warning flags.

Author: alliepiper

CMakeLists.txt

@@ -1,6 +1,6 @@
 # 3.15 is the minimum for including the project with add_subdirectory.
 # 3.17 for building the project's standalone tests/examples/etc.
-# 3.18 for C++17 + CUDA.
+# 3.18.3 for C++17 + CUDA
 cmake_minimum_required(VERSION 3.15)
 
 # Remove this when we use the new CUDA_ARCHITECTURES properties with both

cmake/ThrustBuildCompilerTargets.cmake

@@ -6,40 +6,59 @@
 # - Interface target providing compiler-specific options needed to build
 #   Thrust's tests, examples, etc.
 #
+# thrust.compiler_interface_cpp11
+# thrust.compiler_interface_cpp14
+# thrust.compiler_interface_cpp17
+# - Interface targets providing compiler-specific options that should only be
+#   applied to certain dialects of C++.
+#
 # thrust.promote_cudafe_warnings
 # - Interface target that adds warning promotion for NVCC cudafe invocations.
 # - Only exists to work around github issue #1174 on tbb.cuda configurations.
 # - May be combined with thrust.compiler_interface when #1174 is fully resolved.
+#
+# thrust.silence_unreachable_code_warnings
+# - Interface target that silences unreachable code warnings.
+# - Used to selectively disable such warnings in unit tests caused by
+#   unconditionally thrown exceptions.
 
 function(thrust_build_compiler_targets)
   set(cxx_compile_definitions)
   set(cxx_compile_options)
 
   thrust_update_system_found_flags()
 
-  if (THRUST_TBB_FOUND)
-    # There's a ton of these in the TBB backend, even though the code is correct.
-    # TODO: silence these warnings in code instead
-    append_option_if_available("-Wno-unused-parameter" cxx_compile_options)
-  endif()
-
   if ("MSVC" STREQUAL "${CMAKE_CXX_COMPILER_ID}")
-    # TODO Enable /Wall instead of W3
-    append_option_if_available("/W3" cxx_compile_options)
+    append_option_if_available("/W4" cxx_compile_options)
+
+    # Treat all warnings as errors. This is only supported on Release builds,
+    # as `nv_exec_check_disable` doesn't seem to work with MSVC debug iterators
+    # and spurious warnings are emitted.
+    # See NVIDIA/thrust#1273, NVBug 3129879.
+    if (CMAKE_BUILD_TYPE STREQUAL "Release")
+      append_option_if_available("/WX" cxx_compile_options)
+    endif()
 
-    # Treat all warnings as errors:
-    append_option_if_available("/WX" cxx_compile_options)
+    # Suppress overly-pedantic/unavoidable warnings brought in with /W4:
+    # C4324: structure was padded due to alignment specifier
+    append_option_if_available("/wd4324" cxx_compile_options)
+    # C4505: unreferenced local function has been removed
+    # The CUDA `host_runtime.h` header emits this for
+    # `__cudaUnregisterBinaryUtil`.
+    append_option_if_available("/wd4505" cxx_compile_options)
+    # C4706: assignment within conditional expression
+    # MSVC doesn't provide an opt-out for this warning when the assignment is
+    # intentional. Clang will warn for these, but suppresses the warning when
+    # double-parentheses are used around the assignment. We'll let Clang catch
+    # unintentional assignments and suppress all such warnings on MSVC.
+    append_option_if_available("/wd4706" cxx_compile_options)
 
     # Disabled loss-of-data conversion warnings.
     # TODO Re-enable.
     append_option_if_available("/wd4244" cxx_compile_options)
-    append_option_if_available("/wd4267" cxx_compile_options)
-
-    # Suppress numeric conversion-to-bool warnings.
-    # TODO Re-enable.
-    append_option_if_available("/wd4800" cxx_compile_options)
 
     # Disable warning about applying unary operator- to unsigned type.
+    # TODO Re-enable.
     append_option_if_available("/wd4146" cxx_compile_options)
 
     # MSVC STL assumes that `allocator_traits`'s allocator will use raw pointers,
@@ -64,48 +83,29 @@ function(thrust_build_compiler_targets)
     append_option_if_available("-Winit-self" cxx_compile_options)
     append_option_if_available("-Woverloaded-virtual" cxx_compile_options)
     append_option_if_available("-Wcast-qual" cxx_compile_options)
-    append_option_if_available("-Wno-cast-align" cxx_compile_options)
-    append_option_if_available("-Wno-long-long" cxx_compile_options)
-    append_option_if_available("-Wno-variadic-macros" cxx_compile_options)
+    append_option_if_available("-Wpointer-arith" cxx_compile_options)
+    append_option_if_available("-Wunused-local-typedef" cxx_compile_options)
+    append_option_if_available("-Wvla" cxx_compile_options)
+
+    # Disable GNU extensions (flag is clang only)
+    append_option_if_available("-Wgnu" cxx_compile_options)
+    # Calling a variadic macro with zero args is a GNU extension until C++20,
+    # but the THRUST_PP_ARITY macro is used with zero args. Need to see if this
+    # is a real problem worth fixing.
+    append_option_if_available("-Wno-gnu-zero-variadic-macro-arguments" cxx_compile_options)
+
+    # This complains about functions in CUDA system headers when used with nvcc.
     append_option_if_available("-Wno-unused-function" cxx_compile_options)
-    append_option_if_available("-Wno-unused-variable" cxx_compile_options)
   endif()
 
   if ("GNU" STREQUAL "${CMAKE_CXX_COMPILER_ID}")
-    if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.5)
-      # In GCC 4.4, the CUDA backend's kernel launch templates cause
-      # impossible-to-decipher "'<anonymous>' is used uninitialized in this
-      # function" warnings, so we disable uninitialized variable warnings.
-      append_option_if_available("-Wno-uninitialized" cxx_compile_options)
-    endif()
-
-    if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 4.5)
-      # This isn't available until GCC 4.3, and misfires on TMP code until
-      # GCC 4.5.
-      append_option_if_available("-Wlogical-op" cxx_compile_options)
-    endif()
-
     if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 7.3)
       # GCC 7.3 complains about name mangling changes due to `noexcept`
       # becoming part of the type system; we don't care.
       append_option_if_available("-Wno-noexcept-type" cxx_compile_options)
     endif()
   endif()
 
-  if (("Clang" STREQUAL "${CMAKE_CXX_COMPILER_ID}") OR
-      ("XL" STREQUAL "${CMAKE_CXX_COMPILER_ID}"))
-    # xlC and Clang warn about unused parameters in uninstantiated templates.
-    # This causes xlC to choke on the OMP backend, which is mostly #ifdef'd out
-    # (and thus has unused parameters) when you aren't using it.
-    append_option_if_available("-Wno-unused-parameters" cxx_compile_options)
-  endif()
-
-  if ("Clang" STREQUAL "${CMAKE_CXX_COMPILER_ID}")
-    # -Wunneeded-internal-declaration misfires in the unit test framework
-    # on older versions of Clang.
-    append_option_if_available("-Wno-unneeded-internal-declaration" cxx_compile_options)
-  endif()
-
   if ("Intel" STREQUAL "${CMAKE_CXX_COMPILER_ID}")
     # Disable warning that inlining is inhibited by compiler thresholds.
     append_option_if_available("-diag-disable=11074" cxx_compile_options)
@@ -159,4 +159,36 @@ function(thrust_build_compiler_targets)
   target_compile_options(thrust.promote_cudafe_warnings INTERFACE
     $<$<AND:$<COMPILE_LANGUAGE:CUDA>,$<CUDA_COMPILER_ID:NVIDIA>>:-Xcudafe=--promote_warnings>
   )
+
+  # Some of our unit tests unconditionally throw exceptions, and compilers will
+  # detect that the following instructions are unreachable. This is intentional
+  # and unavoidable in these cases. This target can be used to silence
+  # unreachable code warnings.
+  add_library(thrust.silence_unreachable_code_warnings INTERFACE)
+  if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
+    target_compile_options(thrust.silence_unreachable_code_warnings INTERFACE
+      $<$<COMPILE_LANGUAGE:CXX>:/wd4702>
+      $<$<AND:$<COMPILE_LANGUAGE:CUDA>,$<CUDA_COMPILER_ID:NVIDIA>>:-Xcompiler=/wd4702>
+    )
+  endif()
+
+  # These targets are used for dialect-specific options:
+  add_library(thrust.compiler_interface_cpp11 INTERFACE)
+  add_library(thrust.compiler_interface_cpp14 INTERFACE)
+  add_library(thrust.compiler_interface_cpp17 INTERFACE)
+
+  if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
+    # C4127: conditional expression is constant
+    # Disable this MSVC warning for C++11/C++14. In C++17, we can use
+    # THRUST_IF_CONSTEXPR to address these warnings.
+    target_compile_options(thrust.compiler_interface_cpp11 INTERFACE
+      $<$<COMPILE_LANGUAGE:CXX>:/wd4127>
+      $<$<AND:$<COMPILE_LANGUAGE:CUDA>,$<CUDA_COMPILER_ID:NVIDIA>>:-Xcompiler=/wd4127>
+    )
+    target_compile_options(thrust.compiler_interface_cpp14 INTERFACE
+      $<$<COMPILE_LANGUAGE:CXX>:/wd4127>
+      $<$<AND:$<COMPILE_LANGUAGE:CUDA>,$<CUDA_COMPILER_ID:NVIDIA>>:-Xcompiler=/wd4127>
+    )
+  endif()
+
 endfunction()

cmake/ThrustBuildTargetList.cmake

@@ -61,7 +61,16 @@ function(thrust_set_target_properties target_name host device dialect prefix)
   )
 
   get_target_property(type ${target_name} TYPE)
-  if (NOT ${type} STREQUAL "INTERFACE_LIBRARY")
+  if (${type} STREQUAL "INTERFACE_LIBRARY")
+    target_compile_features(${target_name} INTERFACE
+      cxx_std_${dialect}
+      cuda_std_${dialect}
+    )
+  else()
+    target_compile_features(${target_name} PUBLIC
+      cxx_std_${dialect}
+      cuda_std_${dialect}
+    )
     set_target_properties(${target_name}
       PROPERTIES
         CXX_STANDARD ${dialect}
@@ -147,6 +156,7 @@ function(_thrust_add_target_to_target_list target_name host device dialect prefi
 
   target_link_libraries(${target_name} INTERFACE
     thrust.compiler_interface
+    thrust.compiler_interface_cpp${dialect}
   )
 
   # Workaround Github issue #1174. cudafe promote TBB header warnings to

cmake/ThrustMultiConfig.cmake

@@ -42,10 +42,10 @@ function(thrust_configure_multiconfig)
     option(THRUST_MULTICONFIG_ENABLE_SYSTEM_OMP "Generate build configurations that use OpenMP." OFF)
     option(THRUST_MULTICONFIG_ENABLE_SYSTEM_TBB "Generate build configurations that use TBB." OFF)
 
-    # CMake added C++17 support for CUDA targets in 3.18:
+    # CMake fixed C++17 support for NVCC + MSVC targets in 3.18.3:
     if (THRUST_MULTICONFIG_ENABLE_DIALECT_CPP17 AND
         THRUST_MULTICONFIG_ENABLE_SYSTEM_CUDA)
-      cmake_minimum_required(VERSION 3.18)
+      cmake_minimum_required(VERSION 3.18.3)
     endif()
 
     # Workload:
@@ -120,10 +120,10 @@ function(thrust_configure_multiconfig)
       ${THRUST_CPP_DIALECT_OPTIONS}
     )
 
-    # CMake added C++17 support for CUDA targets in 3.18:
+    # CMake fixed C++17 support for NVCC + MSVC targets in 3.18.3:
     if (THRUST_CPP_DIALECT EQUAL 17 AND
         THRUST_DEVICE_SYSTEM STREQUAL "CUDA")
-      cmake_minimum_required(VERSION 3.18)
+      cmake_minimum_required(VERSION 3.18.3)
     endif()
   endif()
 endfunction()

dependencies/cub

@@ -2057,12 +2057,8 @@ INCLUDE_FILE_PATTERNS  =
 # recursively expanded use the := operator instead of the = operator.
 # This tag requires that the tag ENABLE_PREPROCESSING is set to YES.
 
-PREDEFINED             = THRUST_NOEXCEPT=noexcept \
-                         "THRUST_DEFAULT={}" \
-                         "THRUST_NODISCARD=[[nodiscard]]" \
+PREDEFINED             = "THRUST_NODISCARD=[[nodiscard]]" \
                          "THRUST_MR_DEFAULT_ALIGNMENT=alignof(max_align_t)" \
-                         "THRUST_FINAL=final" \
-                         "THRUST_OVERRIDE=" \
                          "cuda_cub=system::cuda"
 
 # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then this

doc/thrust.dox

@@ -226,7 +226,7 @@ int main()
 
   // print values from original device_vector<float> Z 
   // to ensure that range view was mapped to this vector
-  for (int i = 0, n = Z.size(); i < n; ++i)
+  for (std::size_t i = 0, n = Z.size(); i < n; ++i)
   {
     cout << "z[" << i << "]= " << Z[i] << endl;
   }

examples/cuda/range_view.cu

@@ -84,11 +84,9 @@ int main(void)
   typedef Vector::iterator           Iterator;
   typedef thrust::device_system_tag  System;
 
-  size_t N = 5;
-
   // allocate device memory
-  Vector A(N);
-  Vector B(N);
+  Vector A(5);
+  Vector B(5);
 
   // initialize A and B
   thrust::sequence(A.begin(), A.end());
@@ -100,7 +98,7 @@ int main(void)
 
   // note: we must specify the System to ensure correct execution
   thrust::for_each(thrust::counting_iterator<int,System>(0),
-                   thrust::counting_iterator<int,System>(N),
+                   thrust::counting_iterator<int,System>(5),
                    copy_iterators<Iterator,Iterator>(A.begin(), B.begin()));
 
   std::cout << "After A->B Copy" << std::endl;

examples/raw_reference_cast.cu

@@ -41,7 +41,7 @@ void initialize(thrust::device_vector<int>& v1, thrust::device_vector<int>& v2)
   for(size_t i = 0; i < v1.size(); i++)
   {
     v1[i] = dist(rng);
-    v2[i] = i;
+    v2[i] = static_cast<int>(i);
   }
 }
 

examples/sort.cu

@@ -17,43 +17,29 @@ endif()
 # Generate testing framework libraries:
 add_subdirectory(unittest)
 
-# List of tests that aren't implemented for all backends, but are implemented for CUDA.
-set(partially_implemented_CUDA
-  async_copy
-  async_for_each
-  async_reduce
-  async_reduce_into
-  async_sort
-  async_transform
-  event
-  future
-
-  # This test is incompatible with TBB and OMP, since it requires special per-device
-  # handling to process exceptions in a device function, which is only implemented
-  # for CUDA.
-  unittest_static_assert
-)
-
-# List of tests that aren't implemented for all backends, but are implemented for CPP.
-set(partially_implemented_CPP
-)
-
-# List of tests that aren't implemented for all backends, but are implemented for TBB.
-set(partially_implemented_TBB
-)
-
-# List of tests that aren't implemented for all backends, but are implemented for OMP.
-set(partially_implemented_OMP
-)
-
-# List of all partially implemented tests.
-set(partially_implemented
-  ${partially_implemented_CUDA}
-  ${partially_implemented_CPP}
-  ${partially_implemented_TBB}
-  ${partially_implemented_OMP}
-)
-list(REMOVE_DUPLICATES partially_implemented)
+# Some tests only support certain host.device configurations. Use this macro to
+# declare allowed configurations. If not specified, all host.device config are
+# used.
+set(restricted_tests)
+macro(thrust_declare_test_restrictions test_name)
+  list(APPEND restricted_tests ${test_name})
+  list(APPEND ${test_name}_host.device_allowed ${ARGN})
+endmacro()
+
+# Async/future/event tests only support the CUDA backend:
+thrust_declare_test_restrictions(async_copy        CPP.CUDA OMP.CUDA TBB.CUDA)
+thrust_declare_test_restrictions(async_for_each    CPP.CUDA OMP.CUDA TBB.CUDA)
+thrust_declare_test_restrictions(async_reduce      CPP.CUDA OMP.CUDA TBB.CUDA)
+thrust_declare_test_restrictions(async_reduce_into CPP.CUDA OMP.CUDA TBB.CUDA)
+thrust_declare_test_restrictions(async_sort        CPP.CUDA OMP.CUDA TBB.CUDA)
+thrust_declare_test_restrictions(async_transform   CPP.CUDA OMP.CUDA TBB.CUDA)
+thrust_declare_test_restrictions(event             CPP.CUDA OMP.CUDA TBB.CUDA)
+thrust_declare_test_restrictions(future            CPP.CUDA OMP.CUDA TBB.CUDA)
+
+# This test is incompatible with TBB and OMP, since it requires special per-device
+# handling to process exceptions in a device function, which is only implemented
+# for CUDA.
+thrust_declare_test_restrictions(unittest_static_assert CPP.CPP CPP.CUDA)
 
 ## thrust_add_test
 #
@@ -89,7 +75,7 @@ function(thrust_add_test target_name_var test_name test_src thrust_target)
   set(test_meta_target thrust.all.test.${test_name})
 
   add_executable(${test_target} "${real_test_src}")
-  target_link_libraries(${test_target} ${config_framework_target})
+  target_link_libraries(${test_target} PRIVATE ${config_framework_target})
   target_include_directories(${test_target} PRIVATE "${Thrust_SOURCE_DIR}/testing")
   thrust_clone_target_properties(${test_target} ${thrust_target})
 
@@ -140,15 +126,18 @@ file(GLOB test_srcs
 
 # Add common tests to all configs:
 foreach(thrust_target IN LISTS THRUST_TARGETS)
+  thrust_get_target_property(config_host ${thrust_target} HOST)
   thrust_get_target_property(config_device ${thrust_target} DEVICE)
   thrust_get_target_property(config_prefix ${thrust_target} PREFIX)
 
   foreach(test_src IN LISTS test_srcs)
     get_filename_component(test_name "${test_src}" NAME_WLE)
-    if ("${test_name}" IN_LIST partially_implemented)
-      # This test is partially implemented on _some_ backends...
-      if (NOT "${test_name}" IN_LIST partially_implemented_${config_device})
-        # ...but not on the current one.
+
+    # Is this test restricted to only certain host/device combinations?
+    if(${test_name} IN_LIST restricted_tests)
+      # Is the current host/device combination supported?
+      if (NOT "${config_host}.${config_device}" IN_LIST
+            ${test_name}_host.device_allowed)
         continue()
       endif()
     endif()