Define the SbHash noexcept hash contract

src/base/SbString.cpp

@@ -93,7 +93,7 @@ static unsigned int hash(const unsigned char * str, int n)
   return hash;
 }
 
-unsigned int SbHashFunc(const SbString & key) {
+unsigned int SbHashFunc(const SbString & key) noexcept {
   const unsigned char * cKey = reinterpret_cast<const unsigned char *>(key.getString());
   return hash(cKey,key.getLength());
 }

src/caches/SoPrimitiveVertexCache.cpp

@@ -135,7 +135,7 @@ class SoPrimitiveVertexCacheP {
     int texcoordidx;
 
     // needed for SbHash
-    operator unsigned long(void) const;
+    operator unsigned long(void) const noexcept;
 
     // needed, since if we don't add this the unsigned long operator
     // will be used when comparing two vertices.
@@ -896,7 +896,7 @@ SoPrimitiveVertexCache::depthSortTriangles(SoState * state)
   }
 }
 
-SoPrimitiveVertexCacheP::Vertex::operator unsigned long(void) const
+SoPrimitiveVertexCacheP::Vertex::operator unsigned long(void) const noexcept
 {
   unsigned long key = 0;
   // create an xor key based on coordinates, normal and texcoords

src/fields/SoField.cpp

@@ -137,9 +137,9 @@
 #include "misc/SoConfigSettings.h"
 #include "threads/threadsutilp.h"
 #include "tidbitsp.h"
-inline unsigned int SbHashFunc(const void * key);
+inline unsigned int SbHashFunc(const void * key) noexcept;
 #include "misc/SbHash.h"
-inline unsigned int SbHashFunc(const void * key)
+inline unsigned int SbHashFunc(const void * key) noexcept
 {
   return SbHashFunc(reinterpret_cast<size_t>(key));
 }

src/io/SoOutput.cpp

@@ -163,7 +163,7 @@ class SoOutputROUTEList : public SbList<SoOutputROUTE> {
 
 // *************************************************************************
 
-unsigned int SbHashFunc(const SoOutput * key) {
+unsigned int SbHashFunc(const SoOutput * key) noexcept {
   return SbHashFunc(reinterpret_cast<size_t>(key));
 }
 

src/lists/SoCallbackList.cpp

@@ -52,7 +52,7 @@
 #include <vector>
 
 inline unsigned int
-SbHashFunc(const SbPList * key)
+SbHashFunc(const SbPList * key) noexcept
 {
   return SbHashFunc(reinterpret_cast<size_t>(key));
 }

src/misc/SbHash.h

@@ -73,7 +73,7 @@
 
 //Create an uint of an arbitrary length datatype
 template <class T>
-inline unsigned int toUint(T in) {
+inline unsigned int toUint(T in) noexcept {
   if (sizeof(T)>sizeof(unsigned int)) {
     T retVal=in;
     for (size_t i = sizeof(T)/sizeof(unsigned int)-1; i>0; i--) {
@@ -89,7 +89,7 @@ inline unsigned int toUint(T in) {
 #if !defined(_MSC_VER) || (_MSC_VER >= 1300) // 'long long' not in vc6
 #ifndef COIN_INTERNAL //Not available for internal use, as this is not
                     //available on all platforms.
-inline unsigned int SbHashFunc(unsigned long long key) { return toUint<unsigned long long>(key); }
+inline unsigned int SbHashFunc(unsigned long long key) noexcept { return toUint<unsigned long long>(key); }
 #endif //COIN_INTERNAL
 #endif
 
@@ -98,20 +98,37 @@ inline unsigned int SbHashFunc(unsigned long long key) { return toUint<unsigned
  * where int is 32-bit and long and pointer are 64-bit. */
 /* FIXME: the following solution is a kludge. 20081001 tamer. */
 #if defined(_WIN64)
-inline unsigned int SbHashFunc(unsigned long long key) { return toUint<unsigned long long>(key); }
+inline unsigned int SbHashFunc(unsigned long long key) noexcept { return toUint<unsigned long long>(key); }
 #else
 //The identity hash function
-inline unsigned int SbHashFunc(unsigned int key) { return key; }
+inline unsigned int SbHashFunc(unsigned int key) noexcept { return key; }
 
 //Some implementation of other basetypes
-inline unsigned int SbHashFunc(int key) { return static_cast<unsigned int>(key); }
+inline unsigned int SbHashFunc(int key) noexcept { return static_cast<unsigned int>(key); }
 
-inline unsigned int SbHashFunc(unsigned long key) { return toUint<unsigned long>(key); }
+inline unsigned int SbHashFunc(unsigned long key) noexcept { return toUint<unsigned long>(key); }
 #endif
 
+// Preserve the historical content hash for interned C strings without
+// allocating an SbString temporary. Mutable character buffers are used as
+// pointer-identity keys by SoField and keep that distinct behavior.
+inline unsigned int SbHashFunc(const char * key) noexcept {
+  if (key == NULL) return 0;
+  const unsigned char * str = reinterpret_cast<const unsigned char *>(key);
+  unsigned long hash = 0;
+  while (*str) {
+    hash = (*str++) + (hash << 6) + (hash << 16) - hash;
+  }
+  return static_cast<unsigned int>(hash);
+}
+
+inline unsigned int SbHashFunc(char * key) noexcept {
+  return SbHashFunc(reinterpret_cast<size_t>(key));
+}
+
 //String has its own implementation
 class SbString;
-unsigned int SbHashFunc(const SbString & key);
+unsigned int SbHashFunc(const SbString & key) noexcept;
 
 /*
   Some implementations of pointers, all functions are per writing only reinterpret_casts to size_t
@@ -120,9 +137,9 @@ unsigned int SbHashFunc(const SbString & key);
 class SoBase;
 class SoOutput;
 class SoSensor;
-unsigned int SbHashFunc(const SoBase * key);
-unsigned int SbHashFunc(const SoOutput * key);
-unsigned int SbHashFunc(const SoSensor * key);
+unsigned int SbHashFunc(const SoBase * key) noexcept;
+unsigned int SbHashFunc(const SoOutput * key) noexcept;
+unsigned int SbHashFunc(const SoSensor * key) noexcept;
 
 template <class Key, class Type>
 class SbHash {
@@ -415,9 +432,8 @@ class SbHash {
 
 
 protected:
-  unsigned int getIndex(const Key & key) const {
-    unsigned int idx = SbHashFunc(key);
-    return (idx % this->size);
+  unsigned int getIndex(const Key & key) const noexcept {
+    return this->getIndex(key, this->size);
   }
 
   void resize(unsigned int newsize) {
@@ -491,6 +507,13 @@ class SbHash {
   }
 
  private:
+  static unsigned int getIndex(const Key & key, unsigned int bucketsize) noexcept
+  {
+    static_assert(noexcept(SbHashFunc(key)),
+                  "SbHashFunc(key) and implicit key conversions must be noexcept");
+    return SbHashFunc(key) % bucketsize;
+  }
+
   SbBool getP(const Key & key, Type *& obj) const
   {
     SbHashEntry * entry;

src/misc/SoBase.cpp

@@ -180,7 +180,7 @@ SoType SoBase::classTypeId STATIC_SOTYPE_INIT;
 // <mortene@sim.no>
 #define ALIVE_PATTERN 0xd
 
-unsigned int SbHashFunc(const SoBase * key) {
+unsigned int SbHashFunc(const SoBase * key) noexcept {
   return SbHashFunc(reinterpret_cast<size_t>(key));
 }
 

src/misc/SoContextHandler.cpp

@@ -82,7 +82,7 @@ class socontexthandler_cbitem {
       this->closure == theother.closure;
   }
 
-  operator unsigned long(void) const {
+  operator unsigned long(void) const noexcept {
     unsigned long key = 0;
     // create an xor key
     const unsigned char * ptr = (const unsigned char *) this;

src/misc/SoDebug.cpp

@@ -47,9 +47,9 @@
 
 #include "tidbitsp.h"
 
-inline unsigned int SbHashFunc(const void * key);
+inline unsigned int SbHashFunc(const void * key) noexcept;
 #include "misc/SbHash.h"
-inline unsigned int SbHashFunc(const void * key)
+inline unsigned int SbHashFunc(const void * key) noexcept
 {
   return SbHashFunc(reinterpret_cast<size_t>(key));
 }

src/rendering/SoGLDriverDatabase.cpp

@@ -89,7 +89,7 @@ class SoGLDriverDatabaseP {
     SbName feature;
 
     // needed for SbHash
-    operator unsigned long(void) const {
+    operator unsigned long(void) const noexcept {
       unsigned long bitmask = (unsigned long) ((uintptr_t) this->feature.getString());
       bitmask ^= contextid;
       return bitmask;

src/sensors/SoSensor.cpp

@@ -111,7 +111,7 @@
   registering with SoSensor.
 */
 
-unsigned int SbHashFunc(const SoSensor * key) {
+unsigned int SbHashFunc(const SoSensor * key) noexcept {
   return SbHashFunc(reinterpret_cast<size_t>(key));
 }
 

src/vrml97/Extrusion.cpp

@@ -391,7 +391,7 @@ class SoVRMLExtrusionVertex {
   SbVec2f texcoord;
 
   // needed for SbHash
-  operator unsigned long(void) const {
+  operator unsigned long(void) const noexcept {
     unsigned long key = 0;
     // create an xor key based on coordinates, normal and texcoords
     const unsigned char * ptr = (const unsigned char *) this;

testsuite/CMakeLists.txt

@@ -116,15 +116,17 @@ add_executable(CoinTests
 )
 
 # Private Coin symbols are not exported from the Windows DLL. Compile the
-# dictionary implementation and the existing prime-number support directly
+# dictionary implementation, prime-number and SbHashFunc support directly
 # into CoinTests. Static builds resolve these symbols from the Coin archive.
 if(WIN32 AND COIN_BUILD_SHARED_LIBS)
 	target_sources(CoinTests PRIVATE
 		${PROJECT_SOURCE_DIR}/src/base/dict.cpp
-		CoinGeqPrimeNumberWin32TestSupport.cpp)
+		CoinGeqPrimeNumberWin32TestSupport.cpp
+		SbHashFuncSbStringWin32TestSupport.cpp)
 	set_source_files_properties(
 		${PROJECT_SOURCE_DIR}/src/base/dict.cpp
 		CoinGeqPrimeNumberWin32TestSupport.cpp
+		SbHashFuncSbStringWin32TestSupport.cpp
 		PROPERTIES
 		COMPILE_DEFINITIONS COIN_INTERNAL)
 endif()

testsuite/InternalMacroTest.cpp

@@ -42,6 +42,8 @@
 #error InternalMacroTest is not part of the Coin library.
 #endif
 
+#include <Inventor/SbString.h>
+
 #include "CoinTest.h"
 #include "misc/SbHash.h"
 #include <type_traits>
@@ -62,6 +64,16 @@ class SbHashStatsProbe : public SbHash<unsigned int, int> {
   }
 };
 
+class SbHashIndexProbe : public SbHash<unsigned int, int> {
+public:
+  SbHashIndexProbe(unsigned int sizearg) : SbHash<unsigned int, int>(sizearg) { }
+
+  unsigned int bucketIndex(unsigned int key) const
+  {
+    return this->getIndex(key);
+  }
+};
+
 } // namespace
 
 BOOST_AUTO_TEST_CASE(SbHash_self_assignment_preserves_entries)
@@ -284,3 +296,32 @@ BOOST_AUTO_TEST_CASE(SbHash_statistics_track_collision_chain_erasure)
   BOOST_CHECK_EQUAL(average, 0.0f);
   BOOST_CHECK_EQUAL(maximum, 0);
 }
+
+BOOST_AUTO_TEST_CASE(SbHash_preserves_legacy_bucket_mapping)
+{
+  // A literal 0U is also a null-pointer-constant candidate for SbHashFunc's
+  // pointer overloads (const char *, const SoBase *, ...), which MSVC and
+  // GCC/Clang rank differently and MSVC reports as ambiguous. MSVC also
+  // treats a *const* integral variable initialized to 0 as a null-pointer
+  // constant (the pre-C++11 rule), so this must be a non-const variable to
+  // reliably fail the "constant expression" test under every compiler.
+  unsigned int zero = 0U;
+  static_assert(noexcept(SbHashFunc(zero)),
+                "built-in SbHash functions must be non-throwing");
+  SbHashIndexProbe hash(257);
+  BOOST_CHECK_EQUAL(hash.bucketIndex(0U), 0U);
+  BOOST_CHECK_EQUAL(hash.bucketIndex(1U), 1U);
+  BOOST_CHECK_EQUAL(hash.bucketIndex(255U), 255U);
+  BOOST_CHECK_EQUAL(hash.bucketIndex(258U), 1U);
+}
+
+BOOST_AUTO_TEST_CASE(SbHash_hashes_c_strings_without_an_SbString_temporary)
+{
+  const char * text = "SbHash";
+  const SbString string(text);
+
+  static_assert(noexcept(SbHashFunc(text)),
+                "C-string hashing must satisfy the SbHash noexcept contract");
+  BOOST_CHECK_EQUAL(SbHashFunc(static_cast<const char *>(NULL)), 0U);
+  BOOST_CHECK_EQUAL(SbHashFunc(text), SbHashFunc(string));
+}

testsuite/SbHashFuncSbStringWin32TestSupport.cpp

@@ -0,0 +1,33 @@
+// The SbHash_hashes_c_strings_without_an_SbString_temporary test (in
+// InternalMacroTest.cpp) calls SbHashFunc(const SbString &), a private
+// Coin symbol defined out-of-line in src/base/SbString.cpp. On Windows
+// shared-library builds this symbol is not exported from the Coin DLL,
+// so linking CoinTests.exe against it fails. Compile a copy of the
+// (small, self-contained) implementation directly into the test
+// executable instead of exposing it as public API, matching the
+// existing dict.cpp precedent used elsewhere in this file for the same
+// kind of problem.
+//
+// Included through misc/SbHash.h (with COIN_INTERNAL defined, see
+// CMakeLists.txt) so this definition matches the exact declaration
+// InternalMacroTest.cpp calls against. SbHash.h only forward-declares
+// SbString, so the full class definition is pulled in separately.
+#include "misc/SbHash.h"
+#include <Inventor/SbString.h>
+
+namespace {
+  unsigned int sbstring_hash(const unsigned char * str, int n) {
+    unsigned long hash = 0;
+    while (n--) {
+      hash = (*str++) + (hash << 6) + (hash << 16) - hash;
+    }
+    return hash;
+  }
+}
+
+unsigned int
+SbHashFunc(const SbString & key) noexcept
+{
+  const unsigned char * cKey = reinterpret_cast<const unsigned char *>(key.getString());
+  return sbstring_hash(cKey, key.getLength());
+}