Merge pull request #1620 from Idclip/siof_problems

Various Fixes around SIOF

Author: Idclip

cmake/config/OpenVDBCXX.cmake

@@ -178,6 +178,14 @@ if(OPENVDB_CXX_STRICT)
   add_compile_options("$<$<COMPILE_LANG_AND_ID:CXX,Clang,AppleClang>:-Wall>")
   add_compile_options("$<$<COMPILE_LANG_AND_ID:CXX,Clang,AppleClang>:-Wextra>")
   add_compile_options("$<$<COMPILE_LANG_AND_ID:CXX,Clang,AppleClang>:-Wconversion>")
+  add_compile_options("$<$<COMPILE_LANG_AND_ID:CXX,Clang,AppleClang>:-Wnon-virtual-dtor>")
+  add_compile_options("$<$<COMPILE_LANG_AND_ID:CXX,Clang,AppleClang>:-Wover-aligned>")
+  add_compile_options("$<$<COMPILE_LANG_AND_ID:CXX,Clang,AppleClang>:-Wimplicit-fallthrough>")
+  add_compile_options("$<$<AND:$<CXX_COMPILER_ID:GNU>,$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,9.3.0>>:-Wimplicit-fallthrough>")
+  # Only check global constructors for libraries (we should really check for
+  # executables too but gtest relies on these types of constructors for its
+  # framework).
+  add_compile_options("$<$<AND:$<NOT:$<STREQUAL:$<TARGET_PROPERTY:TYPE>,EXECUTABLE>>,$<COMPILE_LANG_AND_ID:CXX,Clang,AppleClang>>:-Wglobal-constructors>")
   add_compile_options("$<$<COMPILE_LANG_AND_ID:CXX,Clang,AppleClang>:-Wno-sign-conversion>")
   # GNU
   add_compile_options("$<$<COMPILE_LANG_AND_ID:CXX,GNU>:-Werror>")
@@ -189,6 +197,7 @@ if(OPENVDB_CXX_STRICT)
   add_compile_options("$<$<COMPILE_LANG_AND_ID:CXX,GNU>:-Wconversion>")
   add_compile_options("$<$<COMPILE_LANG_AND_ID:CXX,GNU>:-Wdisabled-optimization>")
   add_compile_options("$<$<COMPILE_LANG_AND_ID:CXX,GNU>:-Woverloaded-virtual>")
+  add_compile_options("$<$<COMPILE_LANG_AND_ID:CXX,GNU>:-Wnon-virtual-dtor>")
 else()
   # NO OPENVDB_CXX_STRICT, suppress some warnings
   if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")

openvdb/openvdb/CMakeLists.txt

@@ -387,7 +387,6 @@ set(OPENVDB_LIBRARY_SOURCE_FILES
   points/StreamCompression.cc
   points/points.cc
   util/Formats.cc
-  util/Util.cc
 )
 
 set(OPENVDB_LIBRARY_INCLUDE_FILES

openvdb/openvdb/io/Archive.cc

@@ -99,8 +99,13 @@ struct StreamState
     int halfFloat;
     int mappedFile;
     int metadata;
+};
+
+inline StreamState& GetSteamState()
+{
+    static StreamState sStreamState;
+    return sStreamState;
 }
-sStreamState;
 
 const long StreamState::MAGIC_NUMBER =
     long((uint64_t(OPENVDB_MAGIC) << 32) | (uint64_t(OPENVDB_MAGIC)));
@@ -654,14 +659,14 @@ uint32_t
 getFormatVersion(std::ios_base& is)
 {
     /// @todo get from StreamMetadata
-    return static_cast<uint32_t>(is.iword(sStreamState.fileVersion));
+    return static_cast<uint32_t>(is.iword(GetSteamState().fileVersion));
 }
 
 
 void
 Archive::setFormatVersion(std::istream& is)
 {
-    is.iword(sStreamState.fileVersion) = mFileVersion; ///< @todo remove
+    is.iword(GetSteamState().fileVersion) = mFileVersion; ///< @todo remove
     if (StreamMetadata::Ptr meta = getStreamMetadataPtr(is)) {
         meta->setFileVersion(mFileVersion);
     }
@@ -673,17 +678,17 @@ getLibraryVersion(std::ios_base& is)
 {
     /// @todo get from StreamMetadata
     VersionId version;
-    version.first = static_cast<uint32_t>(is.iword(sStreamState.libraryMajorVersion));
-    version.second = static_cast<uint32_t>(is.iword(sStreamState.libraryMinorVersion));
+    version.first = static_cast<uint32_t>(is.iword(GetSteamState().libraryMajorVersion));
+    version.second = static_cast<uint32_t>(is.iword(GetSteamState().libraryMinorVersion));
     return version;
 }
 
 
 void
 Archive::setLibraryVersion(std::istream& is)
 {
-    is.iword(sStreamState.libraryMajorVersion) = mLibraryVersion.first; ///< @todo remove
-    is.iword(sStreamState.libraryMinorVersion) = mLibraryVersion.second; ///< @todo remove
+    is.iword(GetSteamState().libraryMajorVersion) = mLibraryVersion.first; ///< @todo remove
+    is.iword(GetSteamState().libraryMinorVersion) = mLibraryVersion.second; ///< @todo remove
     if (StreamMetadata::Ptr meta = getStreamMetadataPtr(is)) {
         meta->setLibraryVersion(mLibraryVersion);
     }
@@ -703,9 +708,9 @@ getVersion(std::ios_base& is)
 void
 setCurrentVersion(std::istream& is)
 {
-    is.iword(sStreamState.fileVersion) = OPENVDB_FILE_VERSION; ///< @todo remove
-    is.iword(sStreamState.libraryMajorVersion) = OPENVDB_LIBRARY_MAJOR_VERSION; ///< @todo remove
-    is.iword(sStreamState.libraryMinorVersion) = OPENVDB_LIBRARY_MINOR_VERSION; ///< @todo remove
+    is.iword(GetSteamState().fileVersion) = OPENVDB_FILE_VERSION; ///< @todo remove
+    is.iword(GetSteamState().libraryMajorVersion) = OPENVDB_LIBRARY_MAJOR_VERSION; ///< @todo remove
+    is.iword(GetSteamState().libraryMinorVersion) = OPENVDB_LIBRARY_MINOR_VERSION; ///< @todo remove
     if (StreamMetadata::Ptr meta = getStreamMetadataPtr(is)) {
         meta->setFileVersion(OPENVDB_FILE_VERSION);
         meta->setLibraryVersion(VersionId(
@@ -717,9 +722,9 @@ setCurrentVersion(std::istream& is)
 void
 setVersion(std::ios_base& strm, const VersionId& libraryVersion, uint32_t fileVersion)
 {
-    strm.iword(sStreamState.fileVersion) = fileVersion; ///< @todo remove
-    strm.iword(sStreamState.libraryMajorVersion) = libraryVersion.first; ///< @todo remove
-    strm.iword(sStreamState.libraryMinorVersion) = libraryVersion.second; ///< @todo remove
+    strm.iword(GetSteamState().fileVersion) = fileVersion; ///< @todo remove
+    strm.iword(GetSteamState().libraryMajorVersion) = libraryVersion.first; ///< @todo remove
+    strm.iword(GetSteamState().libraryMinorVersion) = libraryVersion.second; ///< @todo remove
     if (StreamMetadata::Ptr meta = getStreamMetadataPtr(strm)) {
         meta->setFileVersion(fileVersion);
         meta->setLibraryVersion(libraryVersion);
@@ -743,14 +748,14 @@ uint32_t
 getDataCompression(std::ios_base& strm)
 {
     /// @todo get from StreamMetadata
-    return uint32_t(strm.iword(sStreamState.dataCompression));
+    return uint32_t(strm.iword(GetSteamState().dataCompression));
 }
 
 
 void
 setDataCompression(std::ios_base& strm, uint32_t c)
 {
-    strm.iword(sStreamState.dataCompression) = c; ///< @todo remove
+    strm.iword(GetSteamState().dataCompression) = c; ///< @todo remove
     if (StreamMetadata::Ptr meta = getStreamMetadataPtr(strm)) {
         meta->setCompression(c);
     }
@@ -832,14 +837,14 @@ bool
 getWriteGridStatsMetadata(std::ios_base& strm)
 {
     /// @todo get from StreamMetadata
-    return strm.iword(sStreamState.writeGridStatsMetadata) != 0;
+    return strm.iword(GetSteamState().writeGridStatsMetadata) != 0;
 }
 
 
 void
 setWriteGridStatsMetadata(std::ios_base& strm, bool writeGridStats)
 {
-    strm.iword(sStreamState.writeGridStatsMetadata) = writeGridStats; ///< @todo remove
+    strm.iword(GetSteamState().writeGridStatsMetadata) = writeGridStats; ///< @todo remove
     if (StreamMetadata::Ptr meta = getStreamMetadataPtr(strm)) {
         meta->setWriteGridStats(writeGridStats);
     }
@@ -853,7 +858,7 @@ uint32_t
 getGridClass(std::ios_base& strm)
 {
     /// @todo get from StreamMetadata
-    const uint32_t val = static_cast<uint32_t>(strm.iword(sStreamState.gridClass));
+    const uint32_t val = static_cast<uint32_t>(strm.iword(GetSteamState().gridClass));
     if (val >= NUM_GRID_CLASSES) return GRID_UNKNOWN;
     return val;
 }
@@ -862,7 +867,7 @@ getGridClass(std::ios_base& strm)
 void
 setGridClass(std::ios_base& strm, uint32_t cls)
 {
-    strm.iword(sStreamState.gridClass) = long(cls); ///< @todo remove
+    strm.iword(GetSteamState().gridClass) = long(cls); ///< @todo remove
     if (StreamMetadata::Ptr meta = getStreamMetadataPtr(strm)) {
         meta->setGridClass(cls);
     }
@@ -873,14 +878,14 @@ bool
 getHalfFloat(std::ios_base& strm)
 {
     /// @todo get from StreamMetadata
-    return strm.iword(sStreamState.halfFloat) != 0;
+    return strm.iword(GetSteamState().halfFloat) != 0;
 }
 
 
 void
 setHalfFloat(std::ios_base& strm, bool halfFloat)
 {
-    strm.iword(sStreamState.halfFloat) = halfFloat; ///< @todo remove
+    strm.iword(GetSteamState().halfFloat) = halfFloat; ///< @todo remove
     if (StreamMetadata::Ptr meta = getStreamMetadataPtr(strm)) {
         meta->setHalfFloat(halfFloat);
     }
@@ -891,14 +896,14 @@ const void*
 getGridBackgroundValuePtr(std::ios_base& strm)
 {
     /// @todo get from StreamMetadata
-    return strm.pword(sStreamState.gridBackground);
+    return strm.pword(GetSteamState().gridBackground);
 }
 
 
 void
 setGridBackgroundValuePtr(std::ios_base& strm, const void* background)
 {
-    strm.pword(sStreamState.gridBackground) = const_cast<void*>(background); ///< @todo remove
+    strm.pword(GetSteamState().gridBackground) = const_cast<void*>(background); ///< @todo remove
     if (StreamMetadata::Ptr meta = getStreamMetadataPtr(strm)) {
         meta->setBackgroundPtr(background);
     }
@@ -909,7 +914,7 @@ setGridBackgroundValuePtr(std::ios_base& strm, const void* background)
 MappedFile::Ptr
 getMappedFilePtr(std::ios_base& strm)
 {
-    if (const void* ptr = strm.pword(sStreamState.mappedFile)) {
+    if (const void* ptr = strm.pword(GetSteamState().mappedFile)) {
         return *static_cast<const MappedFile::Ptr*>(ptr);
     }
     return MappedFile::Ptr();
@@ -919,15 +924,15 @@ getMappedFilePtr(std::ios_base& strm)
 void
 setMappedFilePtr(std::ios_base& strm, io::MappedFile::Ptr& mappedFile)
 {
-    strm.pword(sStreamState.mappedFile) = &mappedFile;
+    strm.pword(GetSteamState().mappedFile) = &mappedFile;
 }
 #endif // OPENVDB_USE_DELAYED_LOADING
 
 
 StreamMetadata::Ptr
 getStreamMetadataPtr(std::ios_base& strm)
 {
-    if (const void* ptr = strm.pword(sStreamState.metadata)) {
+    if (const void* ptr = strm.pword(GetSteamState().metadata)) {
         return *static_cast<const StreamMetadata::Ptr*>(ptr);
     }
     return StreamMetadata::Ptr();
@@ -937,7 +942,7 @@ getStreamMetadataPtr(std::ios_base& strm)
 void
 setStreamMetadataPtr(std::ios_base& strm, StreamMetadata::Ptr& meta, bool transfer)
 {
-    strm.pword(sStreamState.metadata) = &meta;
+    strm.pword(GetSteamState().metadata) = &meta;
     if (transfer && meta) meta->transferTo(strm);
 }
 
@@ -946,7 +951,7 @@ StreamMetadata::Ptr
 clearStreamMetadataPtr(std::ios_base& strm)
 {
     StreamMetadata::Ptr result = getStreamMetadataPtr(strm);
-    strm.pword(sStreamState.metadata) = nullptr;
+    strm.pword(GetSteamState().metadata) = nullptr;
     return result;
 }
 
@@ -1201,8 +1206,8 @@ doReadGrid(GridBase::Ptr grid, const GridDescriptor& gd, std::istream& is, const
 
     // Restore the file-level stream metadata on exit.
     struct OnExit {
-        OnExit(std::ios_base& strm_): strm(&strm_), ptr(strm_.pword(sStreamState.metadata)) {}
-        ~OnExit() { strm->pword(sStreamState.metadata) = ptr; }
+        OnExit(std::ios_base& strm_): strm(&strm_), ptr(strm_.pword(GetSteamState().metadata)) {}
+        ~OnExit() { strm->pword(GetSteamState().metadata) = ptr; }
         std::ios_base* strm;
         void* ptr;
     };
@@ -1426,8 +1431,8 @@ Archive::writeGrid(GridDescriptor& gd, GridBase::ConstPtr grid,
 {
     // Restore file-level stream metadata on exit.
     struct OnExit {
-        OnExit(std::ios_base& strm_): strm(&strm_), ptr(strm_.pword(sStreamState.metadata)) {}
-        ~OnExit() { strm->pword(sStreamState.metadata) = ptr; }
+        OnExit(std::ios_base& strm_): strm(&strm_), ptr(strm_.pword(GetSteamState().metadata)) {}
+        ~OnExit() { strm->pword(GetSteamState().metadata) = ptr; }
         std::ios_base* strm;
         void* ptr;
     };

openvdb/openvdb/math/ConjGradient.h

@@ -245,7 +245,7 @@ class SparseStencilMatrix
     class ConstRow;
     class RowEditor;
 
-    static const ValueType sZeroValue;
+    static inline constexpr ValueType sZeroValue = zeroVal<ValueType>();
 
     /// Construct an @a n x @a n matrix with at most @a STENCIL_SIZE nonzero elements per row.
     SparseStencilMatrix(SizeType n);
@@ -814,10 +814,6 @@ Vector<T>::str() const
 ////////////////////////////////////////
 
 
-template<typename ValueType, SizeType STENCIL_SIZE>
-const ValueType SparseStencilMatrix<ValueType, STENCIL_SIZE>::sZeroValue = zeroVal<ValueType>();
-
-
 template<typename ValueType, SizeType STENCIL_SIZE>
 inline
 SparseStencilMatrix<ValueType, STENCIL_SIZE>::SparseStencilMatrix(SizeType numRows)

openvdb/openvdb/math/Coord.h

@@ -33,8 +33,8 @@ class Coord
     using Limits = std::numeric_limits<ValueType>;
 
     Coord(): mVec{{0, 0, 0}} {}
-    explicit Coord(Int32 xyz): mVec{{xyz, xyz, xyz}} {}
-    Coord(Int32 x, Int32 y, Int32 z): mVec{{x, y, z}} {}
+    constexpr explicit Coord(Int32 xyz): mVec{{xyz, xyz, xyz}} {}
+    constexpr Coord(Int32 x, Int32 y, Int32 z): mVec{{x, y, z}} {}
     explicit Coord(const Vec3i& v): mVec{{v[0], v[1], v[2]}} {}
     explicit Coord(const Vec3I& v): mVec{{Int32(v[0]), Int32(v[1]), Int32(v[2])}} {}
     explicit Coord(const Int32* v): mVec{{v[0], v[1], v[2]}} {}

openvdb/openvdb/math/Maps.cc

@@ -10,42 +10,33 @@ namespace OPENVDB_VERSION_NAME {
 namespace math {
 
 namespace {
-
-// Declare this at file scope to ensure thread-safe initialization.
-// NOTE: Do *NOT* move this into Maps.h or else we will need to pull in
-//       Windows.h with things like 'rad2' defined!
-std::mutex sInitMapRegistryMutex;
-
+inline std::mutex& GetMapRegistryMutex()
+{
+    static std::mutex sInitMapRegistryMutex;
+    return sInitMapRegistryMutex;
+}
 } // unnamed namespace
 
 
 ////////////////////////////////////////
 
 
-// Caller is responsible for calling this function serially.
 MapRegistry*
-MapRegistry::staticInstance()
+MapRegistry::instance()
 {
     static MapRegistry registry;
     return &registry;
 }
 
 
-MapRegistry*
-MapRegistry::instance()
-{
-    std::lock_guard<std::mutex> lock(sInitMapRegistryMutex);
-    return staticInstance();
-}
-
-
 MapBase::Ptr
 MapRegistry::createMap(const Name& name)
 {
-    std::lock_guard<std::mutex> lock(sInitMapRegistryMutex);
-    MapDictionary::const_iterator iter = staticInstance()->mMap.find(name);
+    std::lock_guard<std::mutex> lock(GetMapRegistryMutex());
+    auto* reg = MapRegistry::instance();
+    MapDictionary::const_iterator iter = reg->mMap.find(name);
 
-    if (iter == staticInstance()->mMap.end()) {
+    if (iter == reg->mMap.end()) {
         OPENVDB_THROW(LookupError, "Cannot create map of unregistered type " << name);
     }
 
@@ -56,37 +47,39 @@ MapRegistry::createMap(const Name& name)
 bool
 MapRegistry::isRegistered(const Name& name)
 {
-    std::lock_guard<std::mutex> lock(sInitMapRegistryMutex);
-    return (staticInstance()->mMap.find(name) != staticInstance()->mMap.end());
+    std::lock_guard<std::mutex> lock(GetMapRegistryMutex());
+    const auto* reg = MapRegistry::instance();
+    return (reg->mMap.find(name) != reg->mMap.end());
 }
 
 
 void
 MapRegistry::registerMap(const Name& name, MapBase::MapFactory factory)
 {
-    std::lock_guard<std::mutex> lock(sInitMapRegistryMutex);
+    std::lock_guard<std::mutex> lock(GetMapRegistryMutex());
+    auto* reg = MapRegistry::instance();
 
-    if (staticInstance()->mMap.find(name) != staticInstance()->mMap.end()) {
+    if (reg->mMap.find(name) != reg->mMap.end()) {
         OPENVDB_THROW(KeyError, "Map type " << name << " is already registered");
     }
 
-    staticInstance()->mMap[name] = factory;
+    reg->mMap[name] = factory;
 }
 
 
 void
 MapRegistry::unregisterMap(const Name& name)
 {
-    std::lock_guard<std::mutex> lock(sInitMapRegistryMutex);
-    staticInstance()->mMap.erase(name);
+    std::lock_guard<std::mutex> lock(GetMapRegistryMutex());
+    MapRegistry::instance()->mMap.erase(name);
 }
 
 
 void
 MapRegistry::clear()
 {
-    std::lock_guard<std::mutex> lock(sInitMapRegistryMutex);
-    staticInstance()->mMap.clear();
+    std::lock_guard<std::mutex> lock(GetMapRegistryMutex());
+    MapRegistry::instance()->mMap.clear();
 }