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

[Richard Smith - zygoloid via Phabricator via llvm-commits]

rsmith added inline comments.


================
Comment at: include/llvm/Support/ReverseIteration.h:26
+  return ReverseIterate<bool>::value &&
+         std::is_pointer<T>::value;
 }
----------------
mgrang wrote:
> dblaikie wrote:
> > Should this use an LLVM trait, to catch more interesting things that are pointer-like but not is_pointer? (I guess unique/shared ptr, and maybe PointerIntPair, PointerUnion, etc? Not sure if there's already an LLVM trait that covers all of those)
> Yes, ideally this should use an LLVM trait. I tried adding a "bool IsPointerLikeType" field to each template specialization in PointerLikeTypeTraits.h. As a result I had to add this field to each definition/specialization of this template throughout llvm/clang/polly as its definitions are scattered throughout.
> 
> More pressingly, the compilation failed since it will now call PointerLikeTypeTrait<T>::IsPointerLikeType for a T which does not have a complete trait definition (like LLVMBasicBlockRef).
> 
> I also tried adding a similar field to each specialization in DenseMapInfo. But that would make it too specific as I would then need to do something like "DenseMapInfo<T>::IsPointerLikeType". This would make it difficult to generalize this for other containers.
> 
> Could you please suggest some ways I can work around these?
Is there a reason that we define the primary `PointerLikeTypeTraits` template? SFINAE on whether `PointerLikeTypeTraits<T>` is a complete type seems like it would be a convenient way to detect if it's been specialized (if it worked).


Repository:
  rL LLVM

https://reviews.llvm.org/D35043