<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Jul 24, 2017 at 12:29 PM Richard Smith - zygoloid via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">rsmith added inline comments.<br>
<br>
<br>
================<br>
Comment at: include/llvm/Support/ReverseIteration.h:26<br>
+  return ReverseIterate<bool>::value &&<br>
+         std::is_pointer<T>::value;<br>
 }<br>
----------------<br>
mgrang wrote:<br>
> dblaikie wrote:<br>
> > 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)<br>
> 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.<br>
><br>
> 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).<br>
><br>
> 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.<br>
><br>
> Could you please suggest some ways I can work around these?<br>
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).<br></blockquote><div><br></div><div>SGTM - though I couldn't immediately think of a way to SFINAE On whether a type is complete or not. <br><br>(actually, for now at least, that wouldn't work - the base case of PointerLikeTypeTraits is defined, just with no members:<br><br>template<typename T> class PointerLikeTypeTraits { }<br><br>but that's /probably/ fixable... (maybe with no cost, maybe with a bunch of cleanup?))</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D35043" rel="noreferrer" target="_blank">https://reviews.llvm.org/D35043</a><br>
<br>
<br>
<br>
</blockquote></div></div>