[llvm-commits] [PATCH] StringRef class.

[Douglas Gregor]

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.

Very nice. A few nitpicks:

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

One gets more uniform conversions for the left- and right-hand  
arguments if these are written as friend function definitions, e.g.,

   friend bool operator==(const StringRef& LHS, const StringRef &RHS) {
     return LHS.compare(RHS) == 0;
   }

Also, for the concatenation operators, one can make them slightly more  
efficient with, e.g.,

   inline std::string operator+(std::string LHS, const StringRef &RHS) {
      LHS.append(RHS.begin(), RHS.end());
      return LHS;
    }

That eliminates one unnecessary temporary (RHS.str()), and in some  
cases allows copy elision when passing a parameter to LHS. It probably  
doesn't matter much.

> The downside is that combining unowned memory and implicit
> construction provides some opportunities for shooting oneself in the
> foot. To mitigate this risk, the general idiom is that one should
> generally avoid explicitly naming the StringRef and only use it as a
> compiler generated temporary. Storage of a StringRef should be limited
> to contexts when the reference strings are available in some long term
> storage (like a MemoryBuffer). Overall I think this is a good
> compromise, and will allow us to have cleaner, more uniform, and
> somewhat more efficient APIs, with a minimum of risk.

I'm not particularly concerned by this downside, since LLVM doesn't  
perform *that* much string manipulation.

   - Doug