[PATCH] D58088: [adt] Add raw_pointer_iterator to iterate over std::unique_ptr<> collections

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 21 13:30:52 PST 2019


dblaikie added a comment.

In D58088#1406356 <https://reviews.llvm.org/D58088#1406356>, @dsanders wrote:

> In D58088#1405089 <https://reviews.llvm.org/D58088#1405089>, @glenjofe wrote:
>
> > > That looks simple enough. I haven't gone through the spec but something as simple as:
> > > 
> > >   namespace llvm {
> > >   template<class Ptr>
> > >   auto to_address(const Ptr &P) noexcept { return P.operator->(); }
> > >   template<class T>
> > >   constexpr T* to_address(T* P) noexcept { return P; }
> > >   }
> > > 
> > > 
> > > seems to cover the majority of it (and definitely the bits I need). It's just the std::pointer_traits<Ptr>::to_address(p) based implementation of the fancy pointer overload that's missing.
> >
> > If it is named `to_address` and intended to convert any pointer to a raw pointer then it should be:
> >
> >   namespace llvm {
> >   template<class T>
> >   constexpr T* to_address(T* p) noexcept { return p; }
> >   template<class Ptr>
> >   typename std::pointer_traits<Ptr>::element_type* to_address(const Ptr &p) noexcept { return llvm::to_address(p.operator->()); }
> >   }
> >
>
>
> Well spotted. I'm about to revert the patch though as I've found a way of doing this without using C++20 or defining any new classes that's good enough (because there's no nullptrs) for the GISel Combiner code that wants to use this


I'd probably still go with this - seems like a nice general utility to have. (& I replied on the to_address review that I'm not sure the extra generality suggested is super important for LLVM so I'd be just as fine leaving it as-is for now) But up to you, for sure.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58088/new/

https://reviews.llvm.org/D58088





More information about the llvm-commits mailing list