[PATCH] Add new ubegin/uend API to StringRef/MDString

Duncan P. N. Exon Smith dexonsmith at apple.com
Thu Nov 13 04:39:07 PST 2014


> On 2014 Nov 12, at 16:55, Keno Fischer <kfischer at college.harvard.edu> wrote:
> 
> Hi dexonsmith,
> 
> In some instances (such as in BitCodeWriter - see http://reviews.llvm.org/D6184),
> it is useful to get an unsiged version of the data pointer instead. This adds an
> appropriate API to be used in BitCodeWriter in a follow up commit.
> 
> http://reviews.llvm.org/D6241
> 
> Files:
>  include/llvm/ADT/StringRef.h
>  include/llvm/IR/Metadata.h
> <D6241.16125.patch>

This LGTM.  I have minor nitpicks below that you can take or leave.

@David, does this API make sense to you given Keno's use case [1]?

[1]: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20141103/243658.html

> Index: include/llvm/ADT/StringRef.h
> ===================================================================
> --- include/llvm/ADT/StringRef.h
> +++ include/llvm/ADT/StringRef.h
> @@ -40,6 +40,7 @@
>    class StringRef {
>    public:
>      typedef const char *iterator;
> +    typedef unsigned const char *uiterator;

This spelling is strange to me; I'd expect:

    typedef const unsigned char *uiterator;

>      typedef const char *const_iterator;
>      static const size_t npos = ~size_t(0);
>      typedef size_t size_type;
> @@ -88,8 +89,12 @@
>      /// @{
>  
>      iterator begin() const { return Data; }
> +    uiterator ubegin() const { return reinterpret_cast<uiterator>(Data); }

I have a slight preference for:

    uiterator ubegin() const { return reinterpret_cast<uiterator>(begin()); }

But mainly just to match my change to `uend()` below.

>  
>      iterator end() const { return Data + Length; }
> +    uiterator uend() const {
> +      return reinterpret_cast<uiterator>(Data) + Length;
> +    }

I think this should be:

    uiterator uend() const { return reinterpret_cast<uiterator>(end()); }

>  
>      /// @}
>      /// @name String Operations
> Index: include/llvm/IR/Metadata.h
> ===================================================================
> --- include/llvm/IR/Metadata.h
> +++ include/llvm/IR/Metadata.h
> @@ -55,12 +55,15 @@
>    unsigned getLength() const { return (unsigned)getName().size(); }
>  
>    typedef StringRef::iterator iterator;
> +  typedef StringRef::uiterator uiterator;
>  
>    /// \brief Pointer to the first byte of the string.
>    iterator begin() const { return getName().begin(); }
> +  uiterator ubegin() const { return getName().ubegin(); }
>  
>    /// \brief Pointer to one byte past the end of the string.
>    iterator end() const { return getName().end(); }
> +  uiterator uend() const { return getName().uend(); }
>  
>    /// \brief Methods for support type inquiry through isa, cast, and dyn_cast.
>    static bool classof(const Value *V) {
> 





More information about the llvm-commits mailing list