From 4807b3c4722dedaff612e91d156435918482422b Mon Sep 17 00:00:00 2001 From: "David C. Lonie" Date: Thu, 13 Apr 2017 14:06:33 -0400 Subject: [PATCH 1/4] Silence warnings about unavoidable weak vtables. - Exception classes cannot be exported due to MSVC's design decisions. See http://stackoverflow.com/questions/24511376. We must leave these classes as header only and silence the warnings. - TransferResource in BufferState.h must remain a header-only class since there is no vtkm_interop library to compile the class into. - The VTKDataSetReader hierarchy must similarly remain header-only since there is no vtkm_io library. - The OptionParser Action classes are part of a header-only utility and cannot be easily compiled into a library. - --- vtkm/cont/Error.h | 4 ++++ vtkm/cont/ErrorBadAllocation.h | 4 ++++ vtkm/cont/ErrorBadType.h | 4 ++++ vtkm/cont/ErrorBadValue.h | 4 ++++ vtkm/cont/ErrorExecution.h | 4 ++++ vtkm/cont/ErrorInternal.h | 4 ++++ vtkm/cont/cuda/ErrorCuda.h | 4 ++++ vtkm/internal/ExportMacros.h | 15 +++++++++++++++ vtkm/interop/BufferState.h | 8 ++++++++ vtkm/io/ErrorIO.h | 4 ++++ vtkm/io/reader/VTKDataSetReader.h | 4 ++++ vtkm/io/reader/VTKDataSetReaderBase.h | 4 ++++ vtkm/io/reader/VTKPolyDataReader.h | 4 ++++ vtkm/io/reader/VTKRectilinearGridReader.h | 4 ++++ vtkm/io/reader/VTKStructuredGridReader.h | 4 ++++ vtkm/io/reader/VTKStructuredPointsReader.h | 4 ++++ vtkm/io/reader/VTKUnstructuredGridReader.h | 4 ++++ vtkm/testing/OptionParser.h | 9 +++++++++ 18 files changed, 92 insertions(+) diff --git a/vtkm/cont/Error.h b/vtkm/cont/Error.h index 58b5b3a50..d518a37e1 100644 --- a/vtkm/cont/Error.h +++ b/vtkm/cont/Error.h @@ -32,6 +32,8 @@ namespace vtkm { namespace cont { +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + /// The superclass of all exceptions thrown by any VTKm function or method. /// class VTKM_ALWAYS_EXPORT Error : public std::exception @@ -69,6 +71,8 @@ private: std::string Message; }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } // namespace vtkm::cont diff --git a/vtkm/cont/ErrorBadAllocation.h b/vtkm/cont/ErrorBadAllocation.h index c0fbee7eb..f42e90328 100644 --- a/vtkm/cont/ErrorBadAllocation.h +++ b/vtkm/cont/ErrorBadAllocation.h @@ -25,6 +25,8 @@ namespace vtkm { namespace cont { +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + /// This class is thrown when VTK-m attempts to manipulate memory that it should /// not. /// @@ -35,6 +37,8 @@ public: : Error(message) { } }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } // namespace vtkm::cont diff --git a/vtkm/cont/ErrorBadType.h b/vtkm/cont/ErrorBadType.h index 03e3cb510..2c70f80dd 100644 --- a/vtkm/cont/ErrorBadType.h +++ b/vtkm/cont/ErrorBadType.h @@ -25,6 +25,8 @@ namespace vtkm { namespace cont { +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + /// This class is thrown when VTK-m encounters data of a type that is /// incompatible with the current operation. /// @@ -35,6 +37,8 @@ public: : Error(message) { } }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } // namespace vtkm::cont diff --git a/vtkm/cont/ErrorBadValue.h b/vtkm/cont/ErrorBadValue.h index d9d7b90bd..b6e1d428f 100644 --- a/vtkm/cont/ErrorBadValue.h +++ b/vtkm/cont/ErrorBadValue.h @@ -25,6 +25,8 @@ namespace vtkm { namespace cont { +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + /// This class is thrown when a VTKm function or method encounters an invalid /// value that inhibits progress. /// @@ -35,6 +37,8 @@ public: : Error(message) { } }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } // namespace vtkm::cont diff --git a/vtkm/cont/ErrorExecution.h b/vtkm/cont/ErrorExecution.h index 53b3fc4d8..bc59c3cb8 100644 --- a/vtkm/cont/ErrorExecution.h +++ b/vtkm/cont/ErrorExecution.h @@ -25,6 +25,8 @@ namespace vtkm { namespace cont { +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + /// This class is thrown in the control environment whenever an error occurs in /// the execution environment. /// @@ -35,6 +37,8 @@ public: : Error(message) { } }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } // namespace vtkm::cont diff --git a/vtkm/cont/ErrorInternal.h b/vtkm/cont/ErrorInternal.h index 85518a77e..2eaf1be96 100644 --- a/vtkm/cont/ErrorInternal.h +++ b/vtkm/cont/ErrorInternal.h @@ -25,6 +25,8 @@ namespace vtkm { namespace cont { +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + /// This class is thrown when VTKm detects an internal state that should never /// be reached. This error usually indicates a bug in vtkm or, at best, VTKm /// failed to detect an invalid input it should have. @@ -36,6 +38,8 @@ public: : Error(message) { } }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } // namespace vtkm::cont diff --git a/vtkm/cont/cuda/ErrorCuda.h b/vtkm/cont/cuda/ErrorCuda.h index 62e2e36b3..92d186055 100644 --- a/vtkm/cont/cuda/ErrorCuda.h +++ b/vtkm/cont/cuda/ErrorCuda.h @@ -67,6 +67,8 @@ namespace vtkm { namespace cont { namespace cuda { +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + /// This error is thrown whenever an unidentified CUDA runtime error is /// encountered. /// @@ -92,6 +94,8 @@ public: } }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } } // namespace vtkm::cont:cuda diff --git a/vtkm/internal/ExportMacros.h b/vtkm/internal/ExportMacros.h index 92d29ad5b..651d96f68 100644 --- a/vtkm/internal/ExportMacros.h +++ b/vtkm/internal/ExportMacros.h @@ -91,6 +91,21 @@ #define VTKM_OVERRIDE override +// Clang will warn about weak vtables (-Wweak-vtables) on exception classes, +// but there's no good way to eliminate them in this case because MSVC (See +// http://stackoverflow.com/questions/24511376). These macros will silence the +// warning for classes defined within them. +#ifdef VTKM_CLANG +#define VTKM_SILENCE_WEAK_VTABLE_WARNING_START \ + _Pragma("clang diagnostic push") \ + _Pragma("clang diagnostic ignored \"-Wweak-vtables\"") +#define VTKM_SILENCE_WEAK_VTABLE_WARNING_END \ + _Pragma("clang diagnostic pop") +#else // VTKM_CLANG +#define VTKM_SILENCE_WEAK_VTABLE_WARNING_START +#define VTKM_SILENCE_WEAK_VTABLE_WARNING_END +#endif // VTKM_CLANG + /// Simple macro to identify a parameter as unused. This allows you to name a /// parameter that is not used. There are several instances where you might /// want to do this. For example, when using a parameter to overload or diff --git a/vtkm/interop/BufferState.h b/vtkm/interop/BufferState.h index 9879cac03..12e2abce0 100644 --- a/vtkm/interop/BufferState.h +++ b/vtkm/interop/BufferState.h @@ -24,6 +24,8 @@ #include #include +#include + #include namespace vtkm{ @@ -32,6 +34,9 @@ namespace interop{ namespace internal { + +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + /// \brief Device backend and opengl interop resources management /// /// \c TransferResource manages a context for a given device backend and a @@ -43,6 +48,9 @@ namespace internal public: virtual ~TransferResource() {} }; + +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } /// \brief Manages the state for transferring an ArrayHandle to opengl. diff --git a/vtkm/io/ErrorIO.h b/vtkm/io/ErrorIO.h index ecbe84678..4345703cd 100644 --- a/vtkm/io/ErrorIO.h +++ b/vtkm/io/ErrorIO.h @@ -25,6 +25,8 @@ namespace vtkm { namespace io { +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + class VTKM_ALWAYS_EXPORT ErrorIO : public vtkm::cont::Error { public: @@ -32,6 +34,8 @@ public: ErrorIO(const std::string message) : Error(message) { } }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } // namespace vtkm::io diff --git a/vtkm/io/reader/VTKDataSetReader.h b/vtkm/io/reader/VTKDataSetReader.h index b8bb4d7ca..550111217 100644 --- a/vtkm/io/reader/VTKDataSetReader.h +++ b/vtkm/io/reader/VTKDataSetReader.h @@ -33,6 +33,8 @@ namespace vtkm { namespace io { namespace reader { +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + class VTKDataSetReader : public VTKDataSetReaderBase { public: @@ -96,6 +98,8 @@ private: std::unique_ptr Reader; }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } } // vtkm::io::reader diff --git a/vtkm/io/reader/VTKDataSetReaderBase.h b/vtkm/io/reader/VTKDataSetReaderBase.h index 9197c2210..0187af4d4 100644 --- a/vtkm/io/reader/VTKDataSetReaderBase.h +++ b/vtkm/io/reader/VTKDataSetReaderBase.h @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -199,6 +200,7 @@ inline vtkm::cont::DynamicCellSet CreateCellSetStructured(const vtkm::Id3 &dim) } // namespace internal +VTKM_SILENCE_WEAK_VTABLE_WARNING_START class VTKDataSetReaderBase { @@ -820,6 +822,8 @@ private: friend class VTKDataSetReader; }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } } // vtkm::io::reader diff --git a/vtkm/io/reader/VTKPolyDataReader.h b/vtkm/io/reader/VTKPolyDataReader.h index 1cf1709aa..30b97d4a7 100644 --- a/vtkm/io/reader/VTKPolyDataReader.h +++ b/vtkm/io/reader/VTKPolyDataReader.h @@ -66,6 +66,8 @@ inline vtkm::cont::ArrayHandle ConcatinateArrayHandles( } // namespace internal +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + class VTKPolyDataReader : public VTKDataSetReaderBase { public: @@ -175,6 +177,8 @@ private: } }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } } // namespace vtkm::io:reader diff --git a/vtkm/io/reader/VTKRectilinearGridReader.h b/vtkm/io/reader/VTKRectilinearGridReader.h index 3d851d36d..64389901e 100644 --- a/vtkm/io/reader/VTKRectilinearGridReader.h +++ b/vtkm/io/reader/VTKRectilinearGridReader.h @@ -26,6 +26,8 @@ namespace vtkm { namespace io { namespace reader { +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + class VTKRectilinearGridReader : public VTKDataSetReaderBase { public: @@ -100,6 +102,8 @@ private: } }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } } // namespace vtkm::io:reader diff --git a/vtkm/io/reader/VTKStructuredGridReader.h b/vtkm/io/reader/VTKStructuredGridReader.h index 59084360f..32f7d56a2 100644 --- a/vtkm/io/reader/VTKStructuredGridReader.h +++ b/vtkm/io/reader/VTKStructuredGridReader.h @@ -26,6 +26,8 @@ namespace vtkm { namespace io { namespace reader { +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + class VTKStructuredGridReader : public VTKDataSetReaderBase { public: @@ -71,6 +73,8 @@ private: } }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } } // namespace vtkm::io:reader diff --git a/vtkm/io/reader/VTKStructuredPointsReader.h b/vtkm/io/reader/VTKStructuredPointsReader.h index 49862effe..01cb7362b 100644 --- a/vtkm/io/reader/VTKStructuredPointsReader.h +++ b/vtkm/io/reader/VTKStructuredPointsReader.h @@ -26,6 +26,8 @@ namespace vtkm { namespace io { namespace reader { +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + class VTKStructuredPointsReader : public VTKDataSetReaderBase { public: @@ -87,6 +89,8 @@ private: } }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } } // namespace vtkm::io:reader diff --git a/vtkm/io/reader/VTKUnstructuredGridReader.h b/vtkm/io/reader/VTKUnstructuredGridReader.h index 4f05b4f36..cc7c0890f 100644 --- a/vtkm/io/reader/VTKUnstructuredGridReader.h +++ b/vtkm/io/reader/VTKUnstructuredGridReader.h @@ -26,6 +26,8 @@ namespace vtkm { namespace io { namespace reader { +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + class VTKUnstructuredGridReader : public VTKDataSetReaderBase { public: @@ -95,6 +97,8 @@ private: } }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + } } } // namespace vtkm::io:reader diff --git a/vtkm/testing/OptionParser.h b/vtkm/testing/OptionParser.h index 53e224afa..06682080f 100644 --- a/vtkm/testing/OptionParser.h +++ b/vtkm/testing/OptionParser.h @@ -37,6 +37,9 @@ * */ +// For warning suppression macros: +#include + /** * @file * @@ -1352,6 +1355,10 @@ private: } }; +// The Action classes emit weak vtables. These classes are small, so we aren't +// concerned about the overhead here. +VTKM_SILENCE_WEAK_VTABLE_WARNING_START + /** * @internal * @brief Interface for actions Parser::workhorse() should perform for each Option it @@ -1479,6 +1486,8 @@ public: } }; +VTKM_SILENCE_WEAK_VTABLE_WARNING_END + inline void Parser::parse(bool gnu, const Descriptor usage[], int argc, const char** argv, Option options[], Option buffer[], int min_abbr_len, bool single_minus_longopt, int bufmax) { From 96bf0dac1b9c031529a789a40221887d0061ea59 Mon Sep 17 00:00:00 2001 From: "David C. Lonie" Date: Thu, 13 Apr 2017 14:07:41 -0400 Subject: [PATCH 2/4] Give AxisAnnotation a virtual destructor. This is necessary since the class has virtual methods, and also instructs the compiler that the class's vtable is to be stored in the vtk_rendering library. --- vtkm/rendering/AxisAnnotation.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vtkm/rendering/AxisAnnotation.h b/vtkm/rendering/AxisAnnotation.h index 634d482e3..2399b2018 100644 --- a/vtkm/rendering/AxisAnnotation.h +++ b/vtkm/rendering/AxisAnnotation.h @@ -42,7 +42,7 @@ protected: public: AxisAnnotation(); - ~AxisAnnotation(); + virtual ~AxisAnnotation(); virtual void Render(const vtkm::rendering::Camera &camera, const vtkm::rendering::WorldAnnotator &worldAnnotator, From fd0e3e0e3e69fa378cc0cb71035d674e2f8e3888 Mon Sep 17 00:00:00 2001 From: "David C. Lonie" Date: Thu, 13 Apr 2017 14:08:27 -0400 Subject: [PATCH 3/4] Remove vtable from WaveletFilter. This header-only class had a virtual destructor, but no other virtual methods or subclasses. --- vtkm/worklet/wavelets/WaveletFilter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vtkm/worklet/wavelets/WaveletFilter.h b/vtkm/worklet/wavelets/WaveletFilter.h index b754279f1..9627ad559 100644 --- a/vtkm/worklet/wavelets/WaveletFilter.h +++ b/vtkm/worklet/wavelets/WaveletFilter.h @@ -96,7 +96,7 @@ public: } // destructor - virtual ~WaveletFilter() + ~WaveletFilter() { if( LowDecomposeFilter ) { From 77baa8a2957b2da2547aadfb524fde58202be34c Mon Sep 17 00:00:00 2001 From: "David C. Lonie" Date: Thu, 13 Apr 2017 15:16:35 -0400 Subject: [PATCH 4/4] Add test for exceptions thrown across library boundaries. --- vtkm/testing/CMakeLists.txt | 1 + vtkm/testing/UnitTestExceptions.cxx | 45 +++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 vtkm/testing/UnitTestExceptions.cxx diff --git a/vtkm/testing/CMakeLists.txt b/vtkm/testing/CMakeLists.txt index b51ef605d..d55764fe2 100644 --- a/vtkm/testing/CMakeLists.txt +++ b/vtkm/testing/CMakeLists.txt @@ -33,6 +33,7 @@ set(unit_tests UnitTestBinaryOperators.cxx UnitTestBounds.cxx UnitTestCellShape.cxx + UnitTestExceptions.cxx UnitTestImplicitFunctions.cxx UnitTestListTag.cxx UnitTestMath.cxx diff --git a/vtkm/testing/UnitTestExceptions.cxx b/vtkm/testing/UnitTestExceptions.cxx new file mode 100644 index 000000000..8d3aea96a --- /dev/null +++ b/vtkm/testing/UnitTestExceptions.cxx @@ -0,0 +1,45 @@ +//============================================================================ +// Copyright (c) Kitware, Inc. +// All rights reserved. +// See LICENSE.txt for details. +// This software is distributed WITHOUT ANY WARRANTY; without even +// the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR +// PURPOSE. See the above copyright notice for more information. +// +// Copyright 2017 Sandia Corporation. +// Copyright 2017 UT-Battelle, LLC. +// Copyright 2017 Los Alamos National Security. +// +// Under the terms of Contract DE-AC04-94AL85000 with Sandia Corporation, +// the U.S. Government retains certain rights in this software. +// +// Under the terms of Contract DE-AC52-06NA25396 with Los Alamos National +// Laboratory (LANL), the U.S. Government retains certain rights in +// this software. +//============================================================================ + +#include +#include + +#include + +//------------------------------------------------------------------------------ +// This test ensures that exceptions thrown internally by the vtkm_cont library +// can be correctly caught across library boundaries. +int UnitTestExceptions(int, char *[]) +{ + vtkm::cont::RuntimeDeviceTracker tracker; + + try { + // This throws a ErrorBadValue from RuntimeDeviceTracker::CheckDevice, + // which is compiled into the vtkm_cont library: + tracker.ResetDevice(vtkm::cont::DeviceAdapterTagError()); + } + catch (vtkm::cont::ErrorBadValue&) + { + return EXIT_SUCCESS; + } + + std::cerr << "Did not catch expected ErrorBadValue exception.\n"; + return EXIT_FAILURE; +}