[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:88-93
+  int *Keys[] = { &P.a, &P.b, &P.c, &P.d };
+
+  // Insert keys into the SmallPtrSet.
+  SmallPtrSet<int *, 4> Set;
+  for (auto *Key: Keys)
+    Set.insert(Key);
----------------
I'm not sure this is going to be more stable than the previous version, right? The pointers may be strictly ordered now, but the hashing container might order them in other ways that could mean different orders depending on higher bits in the pointer values... 

What I had in mind/was previously suggesting was to use integer keys that are pointer like & so you can control all the bits. This would work for the DenseMap test, presumably, but maybe not the SmallPtrSet test (which might only accept real pointer keys, not pointer-like keys).


================
Comment at: unittests/Support/ReverseIterationTest.cpp:100
+  if (shouldReverseIterate<int *>())
+    std::reverse(&IterKeys[0], &IterKeys[4]);
+
----------------
  llvm::reverse(IterKeys);


Repository:
  rL LLVM

https://reviews.llvm.org/D37241