Enable highest level of warnings(W4) under MSVC

This will make VTK-m warning level match the one used by VTK. This commit
also resolves the first round of warnings that W4 exposes.

CMake/VTKmCompilerExtras.cmake

@@ -88,4 +88,12 @@ elseif(CMAKE_COMPILER_IS_ICCXX)
 elseif (CMAKE_COMPILER_IS_MSVCXX)
   #enable large object support so we can have 2^32 addressable sections
   list(APPEND VTKm_COMPILE_OPTIONS "/bigobj")
+
+  # Use the highest warning level for visual c++ compiler.
+  if(CMAKE_CXX_FLAGS MATCHES "/W[0-4]")
+    string(REGEX REPLACE "/W[0-4]" "/W4" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+  else()
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /W4")
+  endif()
+
 endif()

CMake/VTKmMacros.cmake

@@ -71,11 +71,31 @@ endfunction(vtkm_setup_nvcc_flags)
 #Utility to set MSVC only COMPILE_DEFINITIONS and COMPILE_FLAGS needed to
 #reduce number of warnings and compile issues with Visual Studio
 function(vtkm_setup_msvc_properties target )
+  if(NOT MSVC)
+    return()
+  endif()
+
   #disable MSVC CRT and SCL warnings as they recommend using non standard
   #c++ extensions
   target_compile_definitions(${target} PRIVATE "_SCL_SECURE_NO_WARNINGS"
                                                "_CRT_SECURE_NO_WARNINGS")
 
+  #C4702 Generates numerous false positives with template code about
+  #      unreachable code
+  #C4505 Generates numerous warnings about unused functions being
+  #      removed when doing header test builds.
+  #C4512 Generates numerous warning that implicit assignment operators can't
+  #      be constructed. This is understood and we don't care.
+  #C4510 Generates numerous warning that implicit constructors can't
+  #      be constructed. This is understood and we don't care.
+  target_compile_options(${target} PRIVATE -wd4702 -wd4505 -wd4512 -wd4510)
+
+  # In VS2013 the C4127 warning has a bug in the implementation and
+  # generates false positive warnings for lots of template code
+  if(MSVC_VERSION LESS 1900)
+    target_compile_options(${target} PRIVATE -wd4127 )
+  endif()
+
 endfunction(vtkm_setup_msvc_properties)
 
 # Builds a source file and an executable that does nothing other than
@@ -105,6 +125,9 @@ function(vtkm_add_header_build_test name dir_prefix use_cuda)
   #test. this might not happen when everything depends on thrust.
   list(LENGTH cxxfiles cxxfiles_len)
   if (use_cuda AND ${cxxfiles_len} GREATER 0)
+
+    vtkm_setup_nvcc_flags( old_nvcc_flags old_cxx_flags )
+
     # Cuda compiles do not respect target_include_directories
     # and we want system includes so we have to hijack cuda
     # to do it
@@ -119,6 +142,11 @@ function(vtkm_add_header_build_test name dir_prefix use_cuda)
                             )
 
     cuda_add_library(TestBuild_${name} STATIC ${cxxfiles} ${hfiles})
+
+
+    set(CUDA_NVCC_FLAGS ${old_nvcc_flags})
+    set(CMAKE_CXX_FLAGS ${old_cxx_flags})
+
   elseif (${cxxfiles_len} GREATER 0)
     add_library(TestBuild_${name} STATIC ${cxxfiles} ${hfiles})
     target_include_directories(TestBuild_${name} PRIVATE vtkm ${VTKm_INCLUDE_DIRS})
@@ -128,9 +156,7 @@ function(vtkm_add_header_build_test name dir_prefix use_cuda)
     PROPERTIES HEADER_FILE_ONLY TRUE
     )
 
-  if(MSVC)
-    vtkm_setup_msvc_properties(TestBuild_${name})
-  endif()
+  vtkm_setup_msvc_properties(TestBuild_${name})
 
   # Send the libraries created for test builds to their own directory so as to
   # not polute the directory with useful libraries.
@@ -301,9 +327,7 @@ function(vtkm_unit_tests)
 
     target_compile_options(${test_prog} PRIVATE ${VTKm_COMPILE_OPTIONS})
 
-    if(MSVC)
-      vtkm_setup_msvc_properties(${test_prog})
-    endif()
+    vtkm_setup_msvc_properties(${test_prog})
 
     foreach (test ${VTKm_UT_SOURCES})
       get_filename_component(tname ${test} NAME_WE)
@@ -450,9 +474,7 @@ function(vtkm_worklet_unit_tests device_adapter)
       set_tests_properties("${tname}${device_type}" PROPERTIES TIMEOUT ${timeout})
     endforeach (test)
 
-    if(MSVC)
-      vtkm_setup_msvc_properties(${test_prog})
-    endif()
+    vtkm_setup_msvc_properties(${test_prog})
 
     #set the device adapter on the executable
     target_compile_definitions(${test_prog} PRIVATE "VTKM_DEVICE_ADAPTER=${device_adapter}")
@@ -573,9 +595,7 @@ function(vtkm_benchmarks device_adapter)
       target_include_directories(${benchmark_prog} PRIVATE ${VTKm_BACKEND_INCLUDE_DIRS})
       target_link_libraries(${benchmark_prog} PRIVATE vtkm_cont ${VTKm_BACKEND_LIBRARIES})
 
-      if(MSVC)
-        vtkm_setup_msvc_properties(${benchmark_prog})
-      endif()
+      vtkm_setup_msvc_properties(${benchmark_prog})
 
       #add the specific compile options for this executable
       target_compile_options(${benchmark_prog} PRIVATE ${VTKm_COMPILE_OPTIONS})
@@ -709,9 +729,7 @@ function(vtkm_library)
     RUNTIME_OUTPUT_DIRECTORY ${VTKm_EXECUTABLE_OUTPUT_PATH}
     )
 
-  if(MSVC)
-    vtkm_setup_msvc_properties(${lib_name})
-  endif()
+  vtkm_setup_msvc_properties(${lib_name})
 
   if(VTKm_EXTRA_COMPILER_WARNINGS)
     set(cxx_args ${CMAKE_CXX_FLAGS_WARN_EXTRA})

vtkm/benchmarking/BenchmarkArrayTransfer.cxx

@@ -166,7 +166,8 @@ struct BenchmarkArrayTransfer
     VTKM_CONT
     vtkm::Float64 operator()()
     {
-      std::vector<ValueType> vec(this->NumValues, ValueTypeTraits::ZeroInitialization());
+      std::vector<ValueType> vec(static_cast<std::size_t>(this->NumValues),
+                                 ValueTypeTraits::ZeroInitialization());
       ArrayType array = vtkm::cont::make_ArrayHandle(vec);
 
       // Time the copy:
@@ -248,7 +249,8 @@ struct BenchmarkArrayTransfer
     VTKM_CONT
     vtkm::Float64 operator()()
     {
-      std::vector<ValueType> vec(this->NumValues, ValueTypeTraits::ZeroInitialization());
+      std::vector<ValueType> vec(static_cast<std::size_t>(this->NumValues),
+                                 ValueTypeTraits::ZeroInitialization());
       ArrayType array = vtkm::cont::make_ArrayHandle(vec);
 
       // Time the copy:
@@ -291,7 +293,8 @@ struct BenchmarkArrayTransfer
     VTKM_CONT
     vtkm::Float64 operator()()
     {
-      std::vector<ValueType> vec(this->NumValues, ValueTypeTraits::ZeroInitialization());
+      std::vector<ValueType> vec(static_cast<std::size_t>(this->NumValues),
+                                 ValueTypeTraits::ZeroInitialization());
       ArrayType array = vtkm::cont::make_ArrayHandle(vec);
 
       // Ensure data is in control before we start:
@@ -349,7 +352,8 @@ struct BenchmarkArrayTransfer
     VTKM_CONT
     vtkm::Float64 operator()()
     {
-      std::vector<ValueType> vec(this->NumValues, ValueTypeTraits::ZeroInitialization());
+      std::vector<ValueType> vec(static_cast<std::size_t>(this->NumValues),
+                                 ValueTypeTraits::ZeroInitialization());
       ArrayType array = vtkm::cont::make_ArrayHandle(vec);
 
       // Ensure data is in control before we start:

vtkm/cont/DataSetBuilderExplicit.h

@@ -301,6 +301,7 @@ inline VTKM_CONT vtkm::cont::DataSet DataSetBuilderExplicit::BuildDataSet(
   const std::string& coordsNm,
   const std::string& cellNm)
 {
+  (void)tag; //C4100 false positive workaround
   vtkm::cont::DataSet dataSet;
 
   dataSet.AddCoordinateSystem(vtkm::cont::CoordinateSystem(coordsNm, coords));

vtkm/cont/StorageBasic.h

@@ -90,8 +90,16 @@ struct AlignedAllocator
   pointer address(reference r) { return &r; }
   const_pointer address(const_reference r) { return &r; }
   size_type max_size() const { return (std::numeric_limits<size_type>::max)() / sizeof(T); }
-  void construct(pointer p, const T& t) { new (p) T(t); }
-  void destroy(pointer p) { p->~T(); }
+  void construct(pointer p, const T& t)
+  {
+    (void)p;
+    new (p) T(t);
+  }
+  void destroy(pointer p)
+  {
+    (void)p;
+    p->~T();
+  }
 };
 
 template <typename T, typename U, size_t AlignA, size_t AlignB>

vtkm/cont/cuda/ChooseCudaDevice.h

@@ -25,9 +25,12 @@
 #include <vtkm/cont/cuda/ErrorCuda.h>
 
 #include <algorithm>
-#include <cuda.h>
 #include <vector>
 
+VTKM_THIRDPARTY_PRE_INCLUDE
+#include <cuda.h>
+VTKM_THIRDPARTY_POST_INCLUDE
+
 namespace vtkm
 {
 namespace cont

vtkm/cont/cuda/internal/CMakeLists.txt

@@ -55,9 +55,15 @@ if (VTKm_ENABLE_CUDA)
     list(APPEND compile_options -Xcompiler=-fPIC)
   endif()
 
+  vtkm_setup_nvcc_flags( old_nvcc_flags old_cxx_flags )
+
   cuda_compile(vtkm_cont_cuda_object_files ${sources}
                OPTIONS "${compile_options}")
 
+  set(CUDA_NVCC_FLAGS ${old_nvcc_flags})
+  set(CMAKE_CXX_FLAGS ${old_cxx_flags})
+
+
   #Setup the dependency chain for the custom object build so that
   add_custom_target(vtkm_cont_cuda DEPENDS ${vtkm_cont_cuda_object_files})
 

vtkm/cont/cuda/internal/CudaAllocator.cu

@@ -21,7 +21,9 @@
 #include <vtkm/cont/cuda/ErrorCuda.h>
 #include <vtkm/cont/cuda/internal/CudaAllocator.h>
 
+VTKM_THIRDPARTY_PRE_INCLUDE
 #include <cuda_runtime.h>
+VTKM_THIRDPARTY_POST_INCLUDE
 
 // These static vars are in an anon namespace to work around MSVC linker issues.
 namespace

vtkm/cont/internal/DeviceAdapterAlgorithmGeneral.h

@@ -330,7 +330,7 @@ public:
     if (fullSize % numBlocks != 0)
       blockSize += 1;
 
-    U lastResult;
+    U lastResult = vtkm::TypeTraits<U>::ZeroInitialization();
     for (vtkm::Id block = 0; block < numBlocks; block++)
     {
       vtkm::Id numberOfInstances = blockSize;
@@ -539,7 +539,7 @@ public:
     if (fullSize % numBlocks != 0)
       blockSize += 1;
 
-    T lastResult;
+    T lastResult = vtkm::TypeTraits<T>::ZeroInitialization();
     for (vtkm::Id block = 0; block < numBlocks; block++)
     {
       vtkm::Id numberOfInstances = blockSize;

vtkm/cont/serial/internal/CMakeLists.txt

@@ -24,13 +24,15 @@ set(headers
   DeviceAdapterTagSerial.h
   VirtualObjectTransferSerial.h
   )
+vtkm_declare_headers(${headers})
 
 add_library(vtkm_cont_serial OBJECT
   ArrayManagerExecutionSerial.cxx
   DeviceAdapterAlgorithmSerial.cxx
   )
 
-vtkm_declare_headers(${headers})
+
+vtkm_setup_msvc_properties(vtkm_cont_serial)
 
 target_compile_features(vtkm_cont_serial PRIVATE cxx_auto_type)
 target_compile_definitions(vtkm_cont_serial PRIVATE vtkm_cont_EXPORTS)

vtkm/cont/serial/internal/DeviceAdapterAlgorithmSerial.h

@@ -328,7 +328,8 @@ public:
                                   const BinaryCompare& binary_compare)
   {
     internal::WrappedBinaryOperator<bool, BinaryCompare> wrappedCompare(binary_compare);
-    if (sizeof(U) > sizeof(vtkm::Id))
+    VTKM_CONSTEXPR bool larger_than_64bits = sizeof(U) > sizeof(vtkm::Int64);
+    if (larger_than_64bits)
     {
       /// More efficient sort:
       /// Move value indexes when sorting and reorder the value array at last

vtkm/cont/tbb/internal/CMakeLists.txt

@@ -47,6 +47,8 @@ add_library(vtkm_cont_tbb OBJECT
   DeviceAdapterAlgorithmTBB.cxx
   )
 
+vtkm_setup_msvc_properties(vtkm_cont_tbb)
+
 target_compile_features(vtkm_cont_tbb PRIVATE cxx_auto_type)
 target_compile_definitions(vtkm_cont_tbb PRIVATE vtkm_cont_EXPORTS)
 if(BUILD_SHARED_LIBS)

vtkm/cont/tbb/internal/DeviceAdapterAlgorithmTBB.h

@@ -210,7 +210,8 @@ public:
                                   Compare comp)
   {
     using KeyType = vtkm::cont::ArrayHandle<T, StorageT>;
-    if (sizeof(U) > sizeof(vtkm::Id))
+    VTKM_CONSTEXPR bool larger_than_64bits = sizeof(U) > sizeof(vtkm::Int64);
+    if (larger_than_64bits)
     {
       /// More efficient sort:
       /// Move value indexes when sorting and reorder the value array at last

vtkm/cont/testing/TestingFancyArrayHandles.h

@@ -85,9 +85,10 @@ struct ValueScale
   VTKM_EXEC_CONT ValueType operator()(const ValueType& v) const
   {
     using Traits = vtkm::VecTraits<ValueType>;
+    using TTraits = vtkm::TypeTraits<ValueType>;
     using ComponentType = typename Traits::ComponentType;
 
-    ValueType result;
+    ValueType result = TTraits::ZeroInitialization();
     for (vtkm::IdComponent i = 0; i < Traits::GetNumberOfComponents(v); ++i)
     {
       vtkm::Float64 vi = static_cast<vtkm::Float64>(Traits::GetComponent(v, i));

vtkm/cont/testing/UnitTestCellSetExplicit.cxx

@@ -29,16 +29,18 @@ namespace
 
 const vtkm::Id numberOfPoints = 11;
 
-vtkm::UInt8 shapes[] = { static_cast<vtkm::UInt8>(vtkm::CELL_SHAPE_HEXAHEDRON),
-                         static_cast<vtkm::UInt8>(vtkm::CELL_SHAPE_PYRAMID),
-                         static_cast<vtkm::UInt8>(vtkm::CELL_SHAPE_TETRA),
-                         static_cast<vtkm::UInt8>(vtkm::CELL_SHAPE_WEDGE) };
+vtkm::UInt8 g_shapes[] = { static_cast<vtkm::UInt8>(vtkm::CELL_SHAPE_HEXAHEDRON),
+                           static_cast<vtkm::UInt8>(vtkm::CELL_SHAPE_PYRAMID),
+                           static_cast<vtkm::UInt8>(vtkm::CELL_SHAPE_TETRA),
+                           static_cast<vtkm::UInt8>(vtkm::CELL_SHAPE_WEDGE) };
 
-vtkm::IdComponent numIndices[] = { 8, 5, 4, 6 };
+vtkm::IdComponent g_numIndices[] = { 8, 5, 4, 6 };
 
-vtkm::Id indexOffset[] = { 0, 8, 13, 17 };
+vtkm::Id g_indexOffset[] = { 0, 8, 13, 17 };
 
-vtkm::Id connectivity[] = { 0, 1, 5, 4, 3, 2, 6, 7, 1, 5, 6, 2, 8, 5, 8, 10, 6, 4, 7, 9, 5, 6, 10 };
+vtkm::Id g_connectivity[] = {
+  0, 1, 5, 4, 3, 2, 6, 7, 1, 5, 6, 2, 8, 5, 8, 10, 6, 4, 7, 9, 5, 6, 10
+};
 
 template <typename T, std::size_t Length>
 vtkm::Id ArrayLength(const T (&)[Length])
@@ -51,10 +53,10 @@ vtkm::cont::CellSetExplicit<> MakeTestCellSet1()
 {
   vtkm::cont::CellSetExplicit<> cs;
   cs.Fill(numberOfPoints,
-          vtkm::cont::make_ArrayHandle(shapes, 4),
-          vtkm::cont::make_ArrayHandle(numIndices, 4),
-          vtkm::cont::make_ArrayHandle(connectivity, ArrayLength(connectivity)),
-          vtkm::cont::make_ArrayHandle(indexOffset, 4));
+          vtkm::cont::make_ArrayHandle(g_shapes, 4),
+          vtkm::cont::make_ArrayHandle(g_numIndices, 4),
+          vtkm::cont::make_ArrayHandle(g_connectivity, ArrayLength(g_connectivity)),
+          vtkm::cont::make_ArrayHandle(g_indexOffset, 4));
   return cs;
 }
 
@@ -62,11 +64,11 @@ vtkm::cont::CellSetExplicit<> MakeTestCellSet1()
 vtkm::cont::CellSetExplicit<> MakeTestCellSet2()
 {
   vtkm::cont::CellSetExplicit<> cs;
-  cs.Fill(
-    numberOfPoints,
-    vtkm::cont::make_ArrayHandle(shapes + 1, 2),
-    vtkm::cont::make_ArrayHandle(numIndices + 1, 2),
-    vtkm::cont::make_ArrayHandle(connectivity + indexOffset[1], indexOffset[3] - indexOffset[1]));
+  cs.Fill(numberOfPoints,
+          vtkm::cont::make_ArrayHandle(g_shapes + 1, 2),
+          vtkm::cont::make_ArrayHandle(g_numIndices + 1, 2),
+          vtkm::cont::make_ArrayHandle(g_connectivity + g_indexOffset[1],
+                                       g_indexOffset[3] - g_indexOffset[1]));
   return cs;
 }
 
@@ -112,7 +114,7 @@ void TestCellSetExplicit()
                    "result length not equal to number of cells");
   for (vtkm::Id i = 0; i < result.GetNumberOfValues(); ++i)
   {
-    VTKM_TEST_ASSERT(result.GetPortalConstControl().Get(i) == numIndices[i], "incorrect result");
+    VTKM_TEST_ASSERT(result.GetPortalConstControl().Get(i) == g_numIndices[i], "incorrect result");
   }
 
   std::cout << "\tTesting CellToPoint\n";
@@ -136,8 +138,8 @@ void TestCellSetExplicit()
 
   VTKM_TEST_ASSERT(result.GetNumberOfValues() == cellset.GetNumberOfCells(),
                    "result length not equal to number of cells");
-  VTKM_TEST_ASSERT(result.GetPortalConstControl().Get(0) == numIndices[1] &&
-                     result.GetPortalConstControl().Get(1) == numIndices[2],
+  VTKM_TEST_ASSERT(result.GetPortalConstControl().Get(0) == g_numIndices[1] &&
+                     result.GetPortalConstControl().Get(1) == g_numIndices[2],
                    "incorrect result");
 
   std::cout << "\tTesting CellToPoint\n";

vtkm/exec/CellFace.h

@@ -191,6 +191,7 @@ template <typename CellShapeTag>
 static inline VTKM_EXEC vtkm::IdComponent CellFaceNumberOfFaces(CellShapeTag shape,
                                                                 const vtkm::exec::FunctorBase&)
 {
+  (void)shape; //C4100 false positive workaround
   return detail::NumFaces[shape.Id];
 }
 

vtkm/exec/ParametricCoordinates.h

@@ -876,7 +876,7 @@ WorldCoordinatesToParametricCoordinates(const WorldCoordVector& pointWCoords,
   // polygon. The point is in the triangle if it is on the correct side of both
   // planes.
   vtkm::IdComponent firstPointIndex;
-  vtkm::IdComponent secondPointIndex;
+  vtkm::IdComponent secondPointIndex = 0;
   bool foundTriangle = false;
   for (firstPointIndex = 0; firstPointIndex < numPoints - 1; firstPointIndex++)
   {

vtkm/internal/Configure.h.in

@@ -175,8 +175,16 @@
 #elif (defined(VTKM_MSVC))
 #define VTKM_THIRDPARTY_PRE_INCLUDE \
   __pragma(warning(push)) \
-  __pragma(warning(disable:4267)) \
+  __pragma(warning(disable:4100)) \
+  __pragma(warning(disable:4127)) \
+  __pragma(warning(disable:4201)) \
   __pragma(warning(disable:4244)) \
+  __pragma(warning(disable:4267)) \
+  __pragma(warning(disable:4324)) \
+  __pragma(warning(disable:4510)) \
+  __pragma(warning(disable:4512)) \
+  __pragma(warning(disable:4515)) \
+  __pragma(warning(disable:4610)) \
   __pragma(warning(disable:4800))
 #define VTKM_THIRDPARTY_POST_INCLUDE \
   __pragma(warning(pop))

vtkm/io/reader/VTKDataSetReaderBase.h

@@ -186,7 +186,8 @@ vtkm::cont::DynamicArrayHandle CreateDynamicArrayHandle(const std::vector<T>& ve
     case 1:
     {
       using CommonType = typename ClosestCommonType<T>::Type;
-      if (!std::is_same<T, CommonType>::value)
+      VTKM_CONSTEXPR bool not_same = !std::is_same<T, CommonType>::value;
+      if (not_same)
       {
         std::cerr << "Type " << vtkm::io::internal::DataTypeName<T>::Name()
                   << " is currently unsupported. Converting to "
@@ -208,7 +209,8 @@ vtkm::cont::DynamicArrayHandle CreateDynamicArrayHandle(const std::vector<T>& ve
       using InComponentType = typename vtkm::VecTraits<T>::ComponentType;
       using OutComponentType = typename ClosestFloat<InComponentType>::Type;
       using CommonType = vtkm::Vec<OutComponentType, 3>;
-      if (!std::is_same<T, CommonType>::value)
+      VTKM_CONSTEXPR bool not_same = !std::is_same<T, CommonType>::value;
+      if (not_same)
       {
         std::cerr << "Type " << vtkm::io::internal::DataTypeName<InComponentType>::Name() << "["
                   << vtkm::VecTraits<T>::NUM_COMPONENTS << "] "

vtkm/rendering/Camera.cxx

@@ -164,7 +164,14 @@ void Camera::GetRealViewport(vtkm::Id screenWidth,
     vtkm::Float32 daspect =
       (this->Camera2D.Right - this->Camera2D.Left) / (this->Camera2D.Top - this->Camera2D.Bottom);
     daspect *= this->Camera2D.XScale;
-    //cerr << "waspect="<<waspect << "   \tdaspect="<<daspect<<endl;
+//cerr << "waspect="<<waspect << "   \tdaspect="<<daspect<<endl;
+
+//needed as center is a constant value
+#if defined(VTKM_MSVC)
+#pragma warning(push)
+#pragma warning(disable : 4127) // conditional expression is constant
+#endif
+
     const bool center = true; // if false, anchor to bottom-left
     if (waspect > daspect)
     {
@@ -198,6 +205,9 @@ void Camera::GetRealViewport(vtkm::Id screenWidth,
       left = this->ViewportLeft;
       right = this->ViewportRight;
     }
+#if defined(VTKM_MSVC)
+#pragma warning(pop)
+#endif
   }
 }