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

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 24 15:08:47 PDT 2017


On Mon, Jul 24, 2017 at 12:29 PM Richard Smith - zygoloid via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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).
>

SGTM - though I couldn't immediately think of a way to SFINAE On whether a
type is complete or not.

(actually, for now at least, that wouldn't work - the base case of
PointerLikeTypeTraits is defined, just with no members:

template<typename T> class PointerLikeTypeTraits { }

but that's /probably/ fixable... (maybe with no cost, maybe with a bunch of
cleanup?))


>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D35043
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170724/d0767d57/attachment.html>


More information about the llvm-commits mailing list