[PATCH] D35043: [ADT] Enable reverse iteration for DenseMap

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: include/llvm/Support/PointerLikeTypeTraits.h:19
 #include "llvm/Support/DataTypes.h"
+#include "llvm-c/Types.h"
 #include <type_traits>
----------------
Probably a layering violation to include this here. Also seems like a narrow solution - would require more special casing here for other types that are like these two whenever they might get used in a map eventually, etc.

Any chance of implementing is_pointer_like as "is a pointer, or (SFINAE'd) is pointer like"?

(I'd probably name the trait "IsPointerLike" to match the kind of naming of PointerLikeTypeTraits - though I realize this might be a bit confusing if it's true for things that don't have PointerLikeTypeTraits, like these two special cases you mentioned (LLVMOpaqueBasicBlock & LLVMOpaqueValue) )


================
Comment at: unittests/ADT/ReverseIterationTest.cpp:72-73
+  int IterKeys[] = { 2, 4, 1, 3 };
+  if (shouldReverseIterate<int>())
+    std::reverse(&IterKeys[0], &IterKeys[4]);
 
----------------
Seems like this code shouldn't be here (it's known/expected that "shouldReverseIterate<int>()" is always false, right? - so maybe an ASSERT_FALSE for that?)


Repository:
  rL LLVM

https://reviews.llvm.org/D35043