[llvm-commits] [PATCH] StringRef class.

[Chris Lattner]

On Jul 20, 2009, at 5:05 PM, Daniel Dunbar wrote:

> Hi,
>
> The attached patch provides a new StringRef helper class, which is
> used to represent a constant string (pointer & length) which is not
> owned.

Great, thanks for working on this!

+  /// StringRef - Represent a constant reference to a string, i.e. a  
character
+  /// array and a length.
+  ///
+  /// This class does not own the string data, it is expected to be  
used in
+  /// situations where the character data resides in some other  
buffer, whose
+  /// lifetime extends past that of the StringRef. For this reason,  
it is not in
+  /// general safe to store a StringRef.

Please also mention that the stringref is not necessarily nul- 
terminated.

+    /// Construct a string ref from a cstring.
+    /*implicit*/ StringRef(const char *Str)
+      : Data(Str), Length(::strlen(Str)) {}

No magic array template version that avoids strlen? :)


Instead of:

+    const char *begin() const { return Data; }
+    const char *end() const { return Data + Length; }

How about:

+    iterator begin() const { return Data; }
+    iterator end() const { return Data + Length; }



+    /// size - Get the string size.
+    unsigned size() const { return Length; }
+
+    /// length - Get the string size.
+    unsigned length() const { return Length; }

I don't see a reason to have length(), lets just go with size(), which  
is more standard.

+    int compare(const StringRef &RHS) const {
+      // Check the prefix for a mismatch.
+      if (int Res = memcmp(Data, RHS.Data, std::min(Length,  
RHS.Length)))
+        return Res;

Is memcmp guaranteed to return -1/0/1?  I thought it was just  
"negative, 0, positive"?  This will foul up your operator<.


+    bool operator==(const StringRef &RHS) const { return compare(RHS)  
== 0; }
+    bool operator!=(const StringRef &RHS) const { return  
compare(RHS) != 0; }

These are expensive equality checks.  For equality (but not ordering),  
you can do a quick reject based on the string length.

+    bool operator<(const StringRef &RHS) const { return compare(RHS)  
== -1; }

No >, <=, etc?

Otherwise, looks great.  Thanks Daniel!  Bonus points if you actually  
change the Value "name" apis to use it. :)

-Chris