[PATCH] D37241: [unittests] Add reverse iteration unit tests for pointer-like keys

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: unittests/Support/ReverseIterationTest.cpp:61-67
+  static inline PtrLikeInt *getEmptyKey() {
+    return reinterpret_cast<PtrLikeInt *>(0x7fffffff);
+  }
+
+  static inline PtrLikeInt *getTombstoneKey() {
+    return reinterpret_cast<PtrLikeInt *>(-0x7fffffff - 1);
+  }
----------------
Rather than mangling pointers - have two fixed instances of PtrLikeInt that can be pointed to (a local static PtrLikeInt in getEmpty/TombstoneKey would probably suffice:

  static PtrLikeInt *getEmptyKey() {
    static const PtrLikeInt EmptyKey;
    return &EmptyKey;
  }

something like that? (drop the 'inline' keyword from (static and non-static) member functions defined inline in a class - they already have that linkage)


================
Comment at: unittests/Support/ReverseIterationTest.cpp:70
+  static unsigned getHashValue(const PtrLikeInt *PtrVal) {
+    return (unsigned)(PtrVal->value * 37U);
+  }
----------------
Maybe just make the value /be/ the hash, do you think? Not sure if that makes it any better or worse (it doesn't have to be a good hash function - simplicity might be preferable)

Maybe make PtrLikeInt's member 'unsigned' to match the hash value type.


Repository:
  rL LLVM

https://reviews.llvm.org/D37241