[llvm][ADT] Make `Twine` aware of `StringLiteral`

[jansvoboda11]

Summary

The const char * storage backing StringLiteral has static lifetime. Making Twine aware of that allows us to avoid allocating heap memory in some contexts (e.g. avoid passing it to StringSaver::save() in a follow-up Clang patch).

[benlangmuir]

I could be mistaken, but I think this is the same as the std::string case -- a combination of already null-terminated (so no allocation) and having a length, so no need to strlen in the StringRef constructors.

Would it make sense to combine them into a single NullTerminatedPtrAndLength representation?

[jansvoboda11]

I don't think so. The use-case I'm thinking of is having a bunch of const Twine & objects and wanting to create a SmallVector<const char *> with lifetime that's independent from the strings referenced by the Twine objects (using StringSaver for example). Knowing that the Twine refers to single const char * with static lifetime would allow you to safely skip saving/allocating the string buffer. You can't skip that for std::string, since that might deallocate its buffer. Does that make sense?

[benlangmuir]

In D157010#4558150, @jansvoboda11 wrote:

I don't think so. The use-case I'm thinking of is having a bunch of const Twine & objects and wanting to create a SmallVector<const char *> with lifetime that's independent from the strings referenced by the Twine objects (using StringSaver for example). Knowing that the Twine refers to single const char * with static lifetime would allow you to safely skip saving/allocating the string buffer. You can't skip that for std::string, since that might deallocate its buffer. Does that make sense?

Got it, sorry I thought this commit was self-contained and you were worried about the case in toNullTerminatedStringRef which would have allocated before but now does not, but I guess you have a follow-on commit that uses this new representation? In that case, LGTM.

[jansvoboda11]

Yes, clarified in the patch summary. Thanks!

[dblaikie]

I'm confused - this patch was motivated by potential improvements to StringSaver, but https://reviews.llvm.org/D157015 says that those improvements can't be made because StringSaver must copy the data?

[jansvoboda11]

In D157010#4564548, @dblaikie wrote:

I'm confused - this patch was motivated by potential improvements to StringSaver, but https://reviews.llvm.org/D157015 says that those improvements can't be made because StringSaver must copy the data?

That's right. This gets used to avoid calling StringSaver in CompilerInvocation.h (D157046).

llvm/include/llvm/ADT/Twine.h

enum NodeKind : unsigned char {
       /// A pointer to an std::string instance.
       StdStringKind,
 
       /// A Pointer and Length representation. Used for std::string_view,
       /// StringRef, and SmallString.  Can't use a StringRef here
       /// because they are not trivally constructible.
       PtrAndLengthKind,
 
+      /// A pointer and length representation that's also null-terminated.
+      /// Guaranteed to be constructed from a compile-time string literal.
+      StringLiteralKind,
+
       /// A pointer to a formatv_object_base instance.
       FormatvObjectKind,
 
       /// A char value, to render as a character.
       CharKind,
 
       /// An unsigned int value, to render as an unsigned decimal integer.
       DecUIKind,
public:
 
     /// Construct from a StringRef.
     /*implicit*/ Twine(const StringRef &Str) : LHSKind(PtrAndLengthKind) {
       LHS.ptrAndLength.ptr = Str.data();
       LHS.ptrAndLength.length = Str.size();
       assert(isValid() && "Invalid twine!");
     }
 
+    /// Construct from a StringLiteral.
+    /*implicit*/ Twine(const StringLiteral &Str)
+        : LHSKind(StringLiteralKind) {
+      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(PtrAndLengthKind) {
       LHS.ptrAndLength.ptr = Str.data();
       LHS.ptrAndLength.length = Str.size();
       assert(isValid() && "Invalid twine!");
     }
 
public:
     /// @{
 
     /// Check if this twine is trivially empty; a false return value does not
     /// necessarily mean the twine is empty.
     bool isTriviallyEmpty() const {
       return isNullary();
     }
 
+    /// Check if this twine is guaranteed to refer to single string literal.
+    bool isSingleStringLiteral() const {
+      return isUnary() && getLHSKind() == StringLiteralKind;
+    }
+
     /// 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 PtrAndLengthKind:
+      case StringLiteralKind:
         return true;
       default:
         return false;
       }
     }
 
     /// @}
     /// @name String Operations
StringRef getSingleStringRef() const {
       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 PtrAndLengthKind:
+      case StringLiteralKind:
         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.
     StringRef toStringRef(SmallVectorImpl<char> &Out) const {

llvm/lib/Support/Twine.cpp

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());
     }
+    case StringLiteralKind:
+      return StringRef(LHS.ptrAndLength.ptr, LHS.ptrAndLength.length);
     default:
       break;
     }
   }
   toVector(Out);
   Out.push_back(0);
   Out.pop_back();
   return StringRef(Out.data(), Out.size());
case Twine::TwineKind:
     break;
   case Twine::CStringKind:
     OS << Ptr.cString;
     break;
   case Twine::StdStringKind:
     OS << *Ptr.stdString;
     break;
   case Twine::PtrAndLengthKind:
+  case Twine::StringLiteralKind:
     OS << StringRef(Ptr.ptrAndLength.ptr, Ptr.ptrAndLength.length);
     break;
   case Twine::FormatvObjectKind:
     OS << *Ptr.formatvObject;
     break;
   case Twine::CharKind:
     OS << Ptr.character;
     break;
void Twine::printOneChildRepr(raw_ostream &OS, Child Ptr,
   case Twine::StdStringKind:
     OS << "std::string:\""
        << Ptr.stdString << "\"";
     break;
   case Twine::PtrAndLengthKind:
     OS << "ptrAndLength:\""
        << StringRef(Ptr.ptrAndLength.ptr, Ptr.ptrAndLength.length) << "\"";
     break;
+  case Twine::StringLiteralKind:
+    OS << "constexprPtrAndLength:\""
+       << StringRef(Ptr.ptrAndLength.ptr, Ptr.ptrAndLength.length) << "\"";
+    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, Construction) {
   EXPECT_EQ("", Twine().str());
   EXPECT_EQ("hi", Twine("hi").str());
   EXPECT_EQ("hi", Twine(std::string("hi")).str());
   EXPECT_EQ("hi", Twine(StringRef("hi")).str());
   EXPECT_EQ("hi", Twine(StringRef(std::string("hi"))).str());
   EXPECT_EQ("hi", Twine(StringRef("hithere", 2)).str());
+  EXPECT_EQ("hi", Twine(StringLiteral("hi")).str());
   EXPECT_EQ("hi", Twine(SmallString<4>("hi")).str());
   EXPECT_EQ("hi", Twine(formatv("{0}", "hi")).str());
   EXPECT_EQ("hi", Twine(std::string_view("hi")).str());
 }
 
 TEST(TwineTest, Numbers) {
   EXPECT_EQ("123", Twine(123U).str());
   EXPECT_EQ("123", Twine(123).str());
EXPECT_EQ(
       repr(Twine("a").concat(Twine(SmallString<3>("b")).concat(Twine("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(StringLiteral("hello")).toNullTerminatedStringRef(storage).end());
   EXPECT_EQ(0, *Twine(SmallString<11>("hello"))
                     .toNullTerminatedStringRef(storage)
                     .end());
   EXPECT_EQ(0, *Twine(formatv("{0}{1}", "how", "dy"))
                     .toNullTerminatedStringRef(storage)
                     .end());
 }
 
+TEST(TwineTest, isSingleStringLiteral) {
+  EXPECT_TRUE(Twine(StringLiteral("hi")).isSingleStringLiteral());
+  EXPECT_FALSE(Twine("hi").isSingleStringLiteral());
+  EXPECT_FALSE(Twine(StringRef("hi")).isSingleStringLiteral());
+}
+
 TEST(TwineTest, LazyEvaluation) {
   struct formatter : FormatAdapter<int> {
     explicit formatter(int &Count) : FormatAdapter(0), Count(Count) {}
     int &Count;
 
     void format(raw_ostream &OS, StringRef Style) override { ++Count; }
   };