Consolidate string types into ptr and length representations.

[saugustine]

Summary

After rGbbbc4f110e35ac709b943efaa1c4c99ec073da30, we can move
any string type that has convenient pointer and length fields
into the PtrAndLengthKind, reducing the amount of code.

[dblaikie]

Looks good, with the StdStringKind case reverted/kept separately given its use in toNullTerminatedStringRef.

Thanks!

llvm/lib/Support/Twine.cpp

33

Ah, this feature might justify keeping StdStringKind separately.

llvm/include/llvm/ADT/Twine.h

enum NodeKind : unsigned char {
       EmptyKind,
 
       /// A pointer to a Twine instance.
       TwineKind,
 
       /// A pointer to a C string instance.
       CStringKind,
 
-      /// A pointer to an std::string instance.
-      StdStringKind,
-
-      /// A pointer to a StringRef instance.
-      StringRefKind,
-
-      /// A pointer to a SmallString instance.
-      SmallStringKind,
-
-      /// A Pointer and Length representation. Used for std::string_view.
-      /// Can't use a StringRef here because they are not trivally
-      /// constructible.
+      /// A Pointer and Length representation. Used for std::string_view,
+      /// StringRef, SmallString and std::string.  Can't use a StringRef here
+      /// because they are not trivally constructible.
       PtrAndLengthKind,
 
       /// A pointer to a formatv_object_base instance.
       FormatvObjectKind,
 
       /// A char value, to render as a character.
       CharKind,
 
class Twine {
     union Child
     {
       const Twine *twine;
       const char *cString;
       struct {
         const char *ptr;
         size_t length;
       } ptrAndLength;
-      const std::string *stdString;
-      const StringRef *stringRef;
-      const SmallVectorImpl<char> *smallString;
       const formatv_object_base *formatvObject;
       char character;
       unsigned int decUI;
       int decI;
       const unsigned long *decUL;
       const long *decL;
       const unsigned long long *decULL;
       const long long *decLL;
/*implicit*/ Twine(const char *Str) {
 
       assert(isValid() && "Invalid twine!");
     }
     /// Delete the implicit conversion from nullptr as Twine(const char *)
     /// cannot take nullptr.
     /*implicit*/ Twine(std::nullptr_t) = delete;
 
     /// Construct from an std::string.
-    /*implicit*/ Twine(const std::string &Str) : LHSKind(StdStringKind) {
-      LHS.stdString = &Str;
+    /*implicit*/ Twine(const std::string &Str) : LHSKind(PtrAndLengthKind) {
+      LHS.ptrAndLength.ptr = Str.data();
+      LHS.ptrAndLength.length = Str.length();
       assert(isValid() && "Invalid twine!");
     }
 
 #if __cplusplus > 201402L
     /// Construct from an std::string_view by converting it to a pointer and
     /// length.  This handles string_views on a pure API basis, and avoids
     /// storing one (or a pointer to one) inside a Twine, which avoids problems
     /// when mixing code compiled under various C++ standards.
     /*implicit*/ Twine(const std::string_view &Str)
         : LHSKind(PtrAndLengthKind) {
       LHS.ptrAndLength.ptr = Str.data();
       LHS.ptrAndLength.length = Str.length();
       assert(isValid() && "Invalid twine!");
     }
 #endif
 
     /// Construct from a StringRef.
-    /*implicit*/ Twine(const StringRef &Str) : LHSKind(StringRefKind) {
-      LHS.stringRef = &Str;
+    /*implicit*/ Twine(const StringRef &Str) : LHSKind(PtrAndLengthKind) {
+      LHS.ptrAndLength.ptr = Str.data();
+      LHS.ptrAndLength.length = Str.size();
       assert(isValid() && "Invalid twine!");
     }
 
     /// Construct from a SmallString.
     /*implicit*/ Twine(const SmallVectorImpl<char> &Str)
-        : LHSKind(SmallStringKind) {
-      LHS.smallString = &Str;
+        : LHSKind(PtrAndLengthKind) {
+      LHS.ptrAndLength.ptr = Str.data();
+      LHS.ptrAndLength.length = Str.size();
       assert(isValid() && "Invalid twine!");
     }
 
     /// Construct from a formatv_object_base.
     /*implicit*/ Twine(const formatv_object_base &Fmt)
         : LHSKind(FormatvObjectKind) {
       LHS.formatvObject = &Fmt;
       assert(isValid() && "Invalid twine!");
#endif
 
     // FIXME: Unfortunately, to make sure this is as efficient as possible we
     // need extra binary constructors from particular types. We can't rely on
     // the compiler to be smart enough to fold operator+()/concat() down to the
     // right thing. Yet.
 
     /// Construct as the concatenation of a C string and a StringRef.
     /*implicit*/ Twine(const char *LHS, const StringRef &RHS)
-        : LHSKind(CStringKind), RHSKind(StringRefKind) {
+        : LHSKind(CStringKind), RHSKind(PtrAndLengthKind) {
       this->LHS.cString = LHS;
-      this->RHS.stringRef = &RHS;
+      this->RHS.ptrAndLength.ptr = RHS.data();
+      this->RHS.ptrAndLength.length = RHS.size();
       assert(isValid() && "Invalid twine!");
     }
 
     /// Construct as the concatenation of a StringRef and a C string.
     /*implicit*/ Twine(const StringRef &LHS, const char *RHS)
-        : LHSKind(StringRefKind), RHSKind(CStringKind) {
-      this->LHS.stringRef = &LHS;
+        : LHSKind(PtrAndLengthKind), RHSKind(CStringKind) {
+      this->LHS.ptrAndLength.ptr = LHS.data();
+      this->LHS.ptrAndLength.length = LHS.size();
       this->RHS.cString = RHS;
       assert(isValid() && "Invalid twine!");
     }
 
     /// Since the intended use of twines is as temporary objects, assignments
     /// when concatenating might cause undefined behavior or stack corruptions
     Twine &operator=(const Twine &) = delete;
 
#endif
     /// Return true if this twine can be dynamically accessed as a single
     /// StringRef value with getSingleStringRef().
     bool isSingleStringRef() const {
       if (getRHSKind() != EmptyKind) return false;
 
       switch (getLHSKind()) {
       case EmptyKind:
       case CStringKind:
-      case StdStringKind:
-      case StringRefKind:
-      case SmallStringKind:
       case PtrAndLengthKind:
         return true;
       default:
         return false;
       }
     }
 
     /// @}
#endif
     StringRef getSingleStringRef() const {
       assert(isSingleStringRef() &&"This cannot be had as a single stringref!");
       switch (getLHSKind()) {
       default: llvm_unreachable("Out of sync with isSingleStringRef");
       case EmptyKind:
         return StringRef();
       case CStringKind:
         return StringRef(LHS.cString);
-      case StdStringKind:
-        return StringRef(*LHS.stdString);
-      case StringRefKind:
-        return *LHS.stringRef;
-      case SmallStringKind:
-        return StringRef(LHS.smallString->data(), LHS.smallString->size());
       case PtrAndLengthKind:
         return StringRef(LHS.ptrAndLength.ptr, LHS.ptrAndLength.length);
       }
     }
 
     /// This returns the twine as a single StringRef if it can be
     /// represented as such. Otherwise the twine is written into the given
     /// SmallVector and a StringRef to the SmallVector's data is returned.

llvm/lib/Support/Twine.cpp

 #include "llvm/ADT/SmallString.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
 std::string Twine::str() const {
-  // If we're storing only a std::string, just return it.
-  if (LHSKind == StdStringKind && RHSKind == EmptyKind)
-    return *LHS.stdString;
-
   // If we're storing a formatv_object, we can avoid an extra copy by formatting
   // it immediately and returning the result.
   if (LHSKind == FormatvObjectKind && RHSKind == EmptyKind)
     return LHS.formatvObject->str();
 
   // Otherwise, flatten and copy the contents first.
   SmallString<256> Vec;
   return toStringRef(Vec).str();
 }
 
 void Twine::toVector(SmallVectorImpl<char> &Out) const {
   raw_svector_ostream OS(Out);
   print(OS);
 }
 
 StringRef Twine::toNullTerminatedStringRef(SmallVectorImpl<char> &Out) const {
-  if (isUnary()) {
-    switch (getLHSKind()) {
-    case CStringKind:
-      // Already null terminated, yay!
-      return StringRef(LHS.cString);
-    case StdStringKind: {
-      const std::string *str = LHS.stdString;
-      return StringRef(str->c_str(), str->size());
-    }
-    default:
-      break;
-    }
+  if (isUnary() && getLHSKind() == CStringKind) {
   }
   toVector(Out);
   Out.push_back(0);
   Out.pop_back();
   return StringRef(Out.data(), Out.size());
 }
 
 void Twine::printOneChild(raw_ostream &OS, Child Ptr,
                           NodeKind Kind) const {
   switch (Kind) {
   case Twine::NullKind: break;
   case Twine::EmptyKind: break;
   case Twine::TwineKind:
     Ptr.twine->print(OS);
     break;
   case Twine::CStringKind:
     OS << Ptr.cString;
     break;
   case Twine::PtrAndLengthKind:
     OS << StringRef(Ptr.ptrAndLength.ptr, Ptr.ptrAndLength.length);
     break;
-  case Twine::StdStringKind:
-    OS << *Ptr.stdString;
-    break;
-  case Twine::StringRefKind:
-    OS << *Ptr.stringRef;
-    break;
-  case Twine::SmallStringKind:
-    OS << *Ptr.smallString;
-    break;
   case Twine::FormatvObjectKind:
     OS << *Ptr.formatvObject;
     break;
   case Twine::CharKind:
     OS << Ptr.character;
     break;
   case Twine::DecUIKind:
     OS << Ptr.decUI;
void Twine::printOneChildRepr(raw_ostream &OS, Child Ptr,
   case Twine::CStringKind:
     OS << "cstring:\""
        << Ptr.cString << "\"";
     break;
   case Twine::PtrAndLengthKind:
     OS << "ptrAndLength:\""
        << StringRef(Ptr.ptrAndLength.ptr, Ptr.ptrAndLength.length) << "\"";
     break;
-  case Twine::StdStringKind:
-    OS << "std::string:\""
-       << Ptr.stdString << "\"";
-    break;
-  case Twine::StringRefKind:
-    OS << "stringref:\""
-       << Ptr.stringRef << "\"";
-    break;
-  case Twine::SmallStringKind:
-    OS << "smallstring:\"" << *Ptr.smallString << "\"";
-    break;
   case Twine::FormatvObjectKind:
     OS << "formatv:\"" << *Ptr.formatvObject << "\"";
     break;
   case Twine::CharKind:
     OS << "char:\"" << Ptr.character << "\"";
     break;
   case Twine::DecUIKind:
     OS << "decUI:\"" << Ptr.decUI << "\"";

llvm/unittests/ADT/TwineTest.cpp

TEST(TwineTest, Concat) {
   EXPECT_EQ("(Twine null empty)", 
             repr(Twine::createNull().concat(Twine("hi"))));
   
   // Concat with empty.
   EXPECT_EQ("(Twine cstring:\"hi\" empty)", 
             repr(Twine("hi").concat(Twine())));
   EXPECT_EQ("(Twine cstring:\"hi\" empty)", 
             repr(Twine().concat(Twine("hi"))));
-  EXPECT_EQ("(Twine smallstring:\"hi\" empty)", 
+  EXPECT_EQ("(Twine ptrAndLength:\"hi\" empty)",
             repr(Twine().concat(Twine(SmallString<5>("hi")))));
   EXPECT_EQ("(Twine formatv:\"howdy\" empty)",
             repr(Twine(formatv("howdy")).concat(Twine())));
   EXPECT_EQ("(Twine formatv:\"howdy\" empty)",
             repr(Twine().concat(Twine(formatv("howdy")))));
-  EXPECT_EQ("(Twine smallstring:\"hey\" cstring:\"there\")", 
+  EXPECT_EQ("(Twine ptrAndLength:\"hey\" cstring:\"there\")",
             repr(Twine(SmallString<7>("hey")).concat(Twine("there"))));
 #if __cplusplus > 201402L
   EXPECT_EQ("(Twine ptrAndLength:\"hey\" cstring:\"there\")",
             repr(Twine(std::string_view("hey")).concat(Twine("there"))));
 #endif
 
   // Concatenation of unary ropes.
   EXPECT_EQ("(Twine cstring:\"a\" cstring:\"b\")", 
             repr(Twine("a").concat(Twine("b"))));
 
   // Concatenation of other ropes.
   EXPECT_EQ("(Twine rope:(Twine cstring:\"a\" cstring:\"b\") cstring:\"c\")", 
             repr(Twine("a").concat(Twine("b")).concat(Twine("c"))));
   EXPECT_EQ("(Twine cstring:\"a\" rope:(Twine cstring:\"b\" cstring:\"c\"))",
             repr(Twine("a").concat(Twine("b").concat(Twine("c")))));
-  EXPECT_EQ("(Twine cstring:\"a\" rope:(Twine smallstring:\"b\" cstring:\"c\"))",
-            repr(Twine("a").concat(Twine(SmallString<3>("b")).concat(Twine("c")))));
+  EXPECT_EQ(
+      "(Twine cstring:\"a\" rope:(Twine Pt:\"b\" cstring:\"c\"))",
 }
 
 TEST(TwineTest, toNullTerminatedStringRef) {
   SmallString<8> storage;
   EXPECT_EQ(0, *Twine("hello").toNullTerminatedStringRef(storage).end());
   EXPECT_EQ(0,
            *Twine(StringRef("hello")).toNullTerminatedStringRef(storage).end());
   EXPECT_EQ(0, *Twine(SmallString<11>("hello"))