Adding SmallString support to Twine

[yaron.keren]

Summary

Teach Twine to support SmallString to save on .str() calls and later remove str() usage for these frequent cases.
Broke from http://reviews.llvm.org/D6336 for easier review.

[yaron.keren]

Correct missing empty line.

[yaron.keren]

Rebased, added unit tests and some overloads to avoid ambiguity.

[chfast]

include/llvm/MC/MCContext.h

231–232

If Twine can handle SmallString now, wouldn't it be better to reduce the number of overloads in this case?

[yaron.keren]

include/llvm/MC/MCContext.h

231–232

The preferred function is GetOrCreateSymbol(StringRef) in which Name directly indexes Symbols[]. GetOrCreateSymbol(Twine) is a slower convenience function, using a temporary buffer to print out the Twine into a SmallString and then call the StringRef version.

With MakeArgString the (StringRef) function is virtual and all other versions defer work to it.
I did notice MakeArgString(std::string Str) should take Str by reference and will update this.

[yaron.keren]

Make MakeArgString and CopyString accept argument by reference.

[chfast]

lib/MC/MCContext.cpp

172–173

Do you think it is worth to check if Name isSingleStringRef?

[yaron.keren]

Implemented Paweł suggestion to use isSingleStringRef() to reduce the number of overloads. GetOrCreateSymbol, CopyString and MakeArgString accept only Twine now, virtual MakeArgString(StringRef) is still required and was renamed to MakeArgStringRef to avoid ambiguity with the Twine version.

isSingleStringRef() is used through the toStringRef(), which was inlined to give the compiler a chance at optimizing out the Twine construction-isSingleStringRef testing-getSingleStringRef code sequence for simple arguments.

Removed function names from Twine.h comments.

[chfast]

tools/clang/lib/Sema/CodeCompleteConsumer.cpp

253–254

I liked the argument name String better than T. I know the type has changed but CopyString copies strings.

[yaron.keren]

Updated CopyString argument name to 'String'.

[chfast]

LGTM

[yaron.keren]

Committed revision 232465.

include/llvm/ADT/Twine.h

 //===-- Twine.h - Fast Temporary String Concatenation -----------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_ADT_TWINE_H
 #define LLVM_ADT_TWINE_H
 
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cassert>
 #include <string>
 
 namespace llvm {
-  template <typename T>
-  class SmallVectorImpl;
-  class StringRef;
   class raw_ostream;
 
   /// Twine - A lightweight data structure for efficiently representing the
   /// concatenation of temporary values as strings.
   ///
   /// A Twine is a kind of rope, it represents a concatenated string using a
   /// binary-tree, where the string is the preorder of the nodes. Since the
   /// Twine can be efficiently rendered into a buffer when its result is used,
enum NodeKind : unsigned char {
       CStringKind,
 
       /// A pointer to an std::string instance.
       StdStringKind,
 
       /// A pointer to a StringRef instance.
       StringRefKind,
 
+      /// A pointer to a SmallString instance.
+      SmallStringKind,
+
       /// A char value reinterpreted as a pointer, to render as a character.
       CharKind,
 
       /// An unsigned int value reinterpreted as a pointer, to render as an
       /// unsigned decimal integer.
       DecUIKind,
 
       /// An int value reinterpreted as a pointer, to render as a signed
class Twine {
     };
 
     union Child
     {
       const Twine *twine;
       const char *cString;
       const std::string *stdString;
       const StringRef *stringRef;
+      const SmallVectorImpl<char> *smallString;
       char character;
       unsigned int decUI;
       int decI;
       const unsigned long *decUL;
       const long *decL;
       const unsigned long long *decULL;
       const long long *decLL;
       const uint64_t *uHex;
explicit Twine(Child LHS, NodeKind LHSKind, Child RHS, NodeKind RHSKind)
         : LHS(LHS), RHS(RHS), LHSKind(LHSKind), RHSKind(RHSKind) {
       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 &Other) = delete;
 
-    /// isNull - Check for the null twine.
+    /// Check for the null twine.
     bool isNull() const {
       return getLHSKind() == NullKind;
     }
 
-    /// isEmpty - Check for the empty twine.
+    /// Check for the empty twine.
     bool isEmpty() const {
       return getLHSKind() == EmptyKind;
     }
 
-    /// isNullary - Check if this is a nullary twine (null or empty).
+    /// Check if this is a nullary twine (null or empty).
     bool isNullary() const {
       return isNull() || isEmpty();
     }
 
-    /// isUnary - Check if this is a unary twine.
+    /// Check if this is a unary twine.
     bool isUnary() const {
       return getRHSKind() == EmptyKind && !isNullary();
     }
 
-    /// isBinary - Check if this is a binary twine.
+    /// Check if this is a binary twine.
     bool isBinary() const {
       return getLHSKind() != NullKind && getRHSKind() != EmptyKind;
     }
 
-    /// isValid - Check if this is a valid twine (satisfying the invariants on
+    /// Check if this is a valid twine (satisfying the invariants on
     /// order and number of arguments).
     bool isValid() const {
       // Nullary twines always have Empty on the RHS.
       if (isNullary() && getRHSKind() != EmptyKind)
         return false;
 
       // Null should never appear on the RHS.
       if (getRHSKind() == NullKind)
bool isValid() const {
         return false;
       if (getRHSKind() == TwineKind &&
           !RHS.twine->isBinary())
         return false;
 
       return true;
     }
 
-    /// getLHSKind - Get the NodeKind of the left-hand side.
+    /// Get the NodeKind of the left-hand side.
     NodeKind getLHSKind() const { return LHSKind; }
 
-    /// getRHSKind - Get the NodeKind of the right-hand side.
+    /// Get the NodeKind of the right-hand side.
     NodeKind getRHSKind() const { return RHSKind; }
 
-    /// printOneChild - Print one child from a twine.
+    /// Print one child from a twine.
     void printOneChild(raw_ostream &OS, Child Ptr, NodeKind Kind) const;
 
-    /// printOneChildRepr - Print the representation of one child from a twine.
+    /// Print the representation of one child from a twine.
     void printOneChildRepr(raw_ostream &OS, Child Ptr,
                            NodeKind Kind) const;
 
   public:
     /// @name Constructors
     /// @{
 
     /// Construct from an empty string.
public:
 
     /// Construct from a StringRef.
     /*implicit*/ Twine(const StringRef &Str)
       : LHSKind(StringRefKind), RHSKind(EmptyKind) {
       LHS.stringRef = &Str;
       assert(isValid() && "Invalid twine!");
     }
 
+    /// Construct from a SmallString.
+    /*implicit*/ Twine(const SmallVectorImpl<char> &Str)
+      : LHSKind(SmallStringKind), RHSKind(EmptyKind) {
+      LHS.smallString = &Str;
+      assert(isValid() && "Invalid twine!");
+    }
+
     /// Construct from a char.
     explicit Twine(char Val)
       : LHSKind(CharKind), RHSKind(EmptyKind) {
       LHS.character = Val;
     }
 
     /// Construct from a signed char.
     explicit Twine(signed char Val)
static Twine utohexstr(const uint64_t &Val) {
       RHS.twine = nullptr;
       return Twine(LHS, UHexKind, RHS, EmptyKind);
     }
 
     /// @}
     /// @name Predicate Operations
     /// @{
 
-    /// isTriviallyEmpty - Check if this twine is trivially empty; a false
-    /// return value does not necessarily mean the twine is empty.
+    /// Check if this twine is trivially empty; a false return value does not
+    /// necessarily mean the twine is empty.
     bool isTriviallyEmpty() const {
       return isNullary();
     }
 
-    /// isSingleStringRef - Return true if this twine can be dynamically
-    /// accessed as a single StringRef value with getSingleStringRef().
+    /// 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:
         return true;
       default:
         return false;
       }
     }
 
     /// @}
     /// @name String Operations
     /// @{
 
     Twine concat(const Twine &Suffix) const;
 
     /// @}
     /// @name Output & Conversion.
     /// @{
 
-    /// str - Return the twine contents as a std::string.
+    /// Return the twine contents as a std::string.
     std::string str() const;
 
-    /// toVector - Write the concatenated string into the given SmallString or
-    /// SmallVector.
+    /// Write the concatenated string into the given SmallString or SmallVector.
     void toVector(SmallVectorImpl<char> &Out) const;
 
-    /// getSingleStringRef - This returns the twine as a single StringRef.  This
-    /// method is only valid if isSingleStringRef() is true.
+    /// This returns the twine as a single StringRef.  This method is only valid
+    /// if isSingleStringRef() is true.
     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());
       }
     }
 
-    /// toStringRef - This returns the twine as a single StringRef if it can be
+    /// 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;
+    StringRef toStringRef(SmallVectorImpl<char> &Out) const {
+      if (isSingleStringRef())
+        return getSingleStringRef();
+      toVector(Out);
+      return StringRef(Out.data(), Out.size());
+    }
 
-    /// toNullTerminatedStringRef - This returns the twine as a single null
-    /// terminated 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.
+    /// This returns the twine as a single null terminated 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.
     ///
     /// The returned StringRef's size does not include the null terminator.
     StringRef toNullTerminatedStringRef(SmallVectorImpl<char> &Out) const;
 
     /// Write the concatenated string represented by this twine to the
     /// stream \p OS.
     void print(raw_ostream &OS) const;
 

include/llvm/MC/MCContext.h

 #ifndef LLVM_MC_MCCONTEXT_H
 #define LLVM_MC_MCCONTEXT_H
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/Twine.h"
 #include "llvm/MC/MCDwarf.h"
 #include "llvm/MC/SectionKind.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
 #include <map>
 #include <tuple>
 #include <vector> // FIXME: Shouldn't be needed.
 
 namespace llvm {
   class MCAsmInfo;
   class MCExpr;
   class MCSection;
   class MCSymbol;
   class MCLabel;
   struct MCDwarfFile;
   class MCDwarfLoc;
   class MCObjectFileInfo;
   class MCRegisterInfo;
   class MCLineSection;
   class SMLoc;
-  class StringRef;
-  class Twine;
   class MCSectionMachO;
   class MCSectionELF;
   class MCSectionCOFF;
 
   /// Context object for machine code objects.  This class owns all of the
   /// sections that it creates.
   ///
   class MCContext {
public:
 
     /// Create and return a directional local symbol for numbered label (used
     /// for "1b" or 1f" references).
     MCSymbol *GetDirectionalLocalSymbol(unsigned LocalLabelVal, bool Before);
 
     /// Lookup the symbol inside with the specified @p Name.  If it exists,
     /// return it.  If not, create a forward reference and return it.
     ///
     /// @param Name - The symbol name, which must be unique across all symbols.
-    MCSymbol *GetOrCreateSymbol(StringRef Name);
     MCSymbol *GetOrCreateSymbol(const Twine &Name);
 
     MCSymbol *getOrCreateSectionSymbol(const MCSectionELF &Section);
 
     MCSymbol *getOrCreateFrameAllocSymbol(StringRef FuncName, unsigned Idx);
 
     /// Get the symbol for \p Name, or null.
     MCSymbol *LookupSymbol(StringRef Name) const;
     MCSymbol *LookupSymbol(const Twine &Name) const;

include/llvm/Option/ArgList.h

 //===--- ArgList.h - Argument List Management -------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_OPTION_ARGLIST_H
 #define LLVM_OPTION_ARGLIST_H
 
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
 #include "llvm/Option/OptSpecifier.h"
 #include "llvm/Option/Option.h"
 #include <list>
 #include <memory>
 #include <string>
 #include <vector>
 
 namespace llvm {
public:
   /// ClaimAllArgs - Claim all arguments.
   ///
   void ClaimAllArgs() const;
 
   /// @}
   /// @name Arg Synthesis
   /// @{
 
-  /// MakeArgString - Construct a constant string pointer whose
+  /// Construct a constant string pointer whose
   /// lifetime will match that of the ArgList.
-  virtual const char *MakeArgString(StringRef Str) const = 0;
-  const char *MakeArgString(const char *Str) const {
-    return MakeArgString(StringRef(Str));
+  virtual const char *MakeArgStringRef(StringRef Str) const = 0;
+  const char *MakeArgString(const Twine &Str) const {
+    SmallString<256> Buf;
+    return MakeArgStringRef(Str.toStringRef(Buf));
   }
-  const char *MakeArgString(std::string Str) const {
-    return MakeArgString(StringRef(Str));
-  }
-  const char *MakeArgString(const Twine &Str) const;
 
   /// \brief Create an arg string for (\p LHS + \p RHS), reusing the
   /// string at \p Index if possible.
   const char *GetOrMakeJoinedArgString(unsigned Index, StringRef LHS,
                                         StringRef RHS) const;
 
   /// @}
 };
public:
   /// @{
 
 public:
   /// MakeIndex - Get an index for the given string(s).
   unsigned MakeIndex(StringRef String0) const;
   unsigned MakeIndex(StringRef String0, StringRef String1) const;
 
   using ArgList::MakeArgString;
-  const char *MakeArgString(StringRef Str) const override;
+  const char *MakeArgStringRef(StringRef Str) const override;
 
   /// @}
 };
 
 /// DerivedArgList - An ordered collection of driver arguments,
 /// whose storage may be in another argument list.
 class DerivedArgList : public ArgList {
   const InputArgList &BaseArgs;
public:
   /// @name Arg Synthesis
   /// @{
 
   /// AddSynthesizedArg - Add a argument to the list of synthesized arguments
   /// (to be freed).
   void AddSynthesizedArg(Arg *A);
 
   using ArgList::MakeArgString;
-  const char *MakeArgString(StringRef Str) const override;
+  const char *MakeArgStringRef(StringRef Str) const override;
 
   /// AddFlagArg - Construct a new FlagArg for the given option \p Id and
   /// append it to the argument list.
   void AddFlagArg(const Arg *BaseArg, const Option Opt) {
     append(MakeFlagArg(BaseArg, Opt));
   }
 
   /// AddPositionalArg - Construct a new Positional arg for the given option

lib/MC/MCContext.cpp

void MCContext::reset() {
   GenDwarfForAssembly = false;
   GenDwarfFileNumber = 0;
 }
 
 //===----------------------------------------------------------------------===//
 // Symbol Manipulation
 //===----------------------------------------------------------------------===//
 
-MCSymbol *MCContext::GetOrCreateSymbol(StringRef Name) {
-  assert(!Name.empty() && "Normal symbols cannot be unnamed!");
+MCSymbol *MCContext::GetOrCreateSymbol(const Twine &Name) {
+  SmallString<128> NameSV;
+  StringRef NameRef = Name.toStringRef(NameSV);
 
-  MCSymbol *&Sym = Symbols[Name];
+  assert(!NameRef.empty() && "Normal symbols cannot be unnamed!");
 
+  MCSymbol *&Sym = Symbols[NameRef];
   if (!Sym)
-    Sym = CreateSymbol(Name);
+    Sym = CreateSymbol(NameRef);
 
   return Sym;
 }
 
 MCSymbol *MCContext::getOrCreateSectionSymbol(const MCSectionELF &Section) {
   MCSymbol *&Sym = SectionSymbols[&Section];
   if (Sym)
     return Sym;
MCSymbol *MCContext::CreateSymbol(StringRef Name) {
   return Result;
 }
 
 MCSymbol *MCContext::createTempSymbol(const Twine &Name) {
   SmallString<128> NameSV;
   raw_svector_ostream(NameSV) << MAI->getPrivateGlobalPrefix() << Name;
   return CreateSymbol(NameSV);
 }
 
-MCSymbol *MCContext::GetOrCreateSymbol(const Twine &Name) {
-  SmallString<128> NameSV;
-  return GetOrCreateSymbol(Name.toStringRef(NameSV));
-}
-
 MCSymbol *MCContext::CreateLinkerPrivateTempSymbol() {
   SmallString<128> NameSV;
   raw_svector_ostream(NameSV)
     << MAI->getLinkerPrivateGlobalPrefix() << "tmp" << NextUniqueID++;
   return CreateSymbol(NameSV);
 }
 
 MCSymbol *MCContext::CreateTempSymbol() {
   SmallString<128> NameSV;

lib/Option/ArgList.cpp

 }
 
 void ArgList::ClaimAllArgs() const {
   for (const_iterator it = begin(), ie = end(); it != ie; ++it)
     if (!(*it)->isClaimed())
       (*it)->claim();
 }
 
-const char *ArgList::MakeArgString(const Twine &T) const {
-  SmallString<256> Str;
-  return MakeArgString(T.toStringRef(Str));
-}
-
 const char *ArgList::GetOrMakeJoinedArgString(unsigned Index,
                                               StringRef LHS,
                                               StringRef RHS) const {
   StringRef Cur = getArgString(Index);
   if (Cur.size() == LHS.size() + RHS.size() &&
       Cur.startswith(LHS) && Cur.endswith(RHS))
     return Cur.data();
 
unsigned InputArgList::MakeIndex(StringRef String0,
                                  StringRef String1) const {
   unsigned Index0 = MakeIndex(String0);
   unsigned Index1 = MakeIndex(String1);
   assert(Index0 + 1 == Index1 && "Unexpected non-consecutive indices!");
   (void) Index1;
   return Index0;
 }
 
-const char *InputArgList::MakeArgString(StringRef Str) const {
+const char *InputArgList::MakeArgStringRef(StringRef Str) const {
   return getArgString(MakeIndex(Str));
 }
 
 //
 
 DerivedArgList::DerivedArgList(const InputArgList &BaseArgs)
     : BaseArgs(BaseArgs) {}
 
 DerivedArgList::~DerivedArgList() {}
 
-const char *DerivedArgList::MakeArgString(StringRef Str) const {
+const char *DerivedArgList::MakeArgStringRef(StringRef Str) const {
   return BaseArgs.MakeArgString(Str);
 }
 
 void DerivedArgList::AddSynthesizedArg(Arg *A) {
   SynthesizedArgs.push_back(std::unique_ptr<Arg>(A));
 }
 
 Arg *DerivedArgList::MakeFlagArg(const Arg *BaseArg, const Option Opt) const {

lib/Support/Twine.cpp

std::string Twine::str() const {
   return toStringRef(Vec).str();
 }
 
 void Twine::toVector(SmallVectorImpl<char> &Out) const {
   raw_svector_ostream OS(Out);
   print(OS);
 }
 
-StringRef Twine::toStringRef(SmallVectorImpl<char> &Out) const {
-  if (isSingleStringRef())
-    return getSingleStringRef();
-  toVector(Out);
-  return StringRef(Out.data(), Out.size());
-}
-
 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;
case Twine::CStringKind:
     OS << Ptr.cString;
     break;
   case Twine::StdStringKind:
     OS << *Ptr.stdString;
     break;
   case Twine::StringRefKind:
     OS << *Ptr.stringRef;
     break;
+  case Twine::SmallStringKind:
+    OS << *Ptr.smallString;
+    break;
   case Twine::CharKind:
     OS << Ptr.character;
     break;
   case Twine::DecUIKind:
     OS << Ptr.decUI;
     break;
   case Twine::DecIKind:
     OS << Ptr.decI;
void Twine::printOneChildRepr(raw_ostream &OS, Child Ptr,
   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::CharKind:
     OS << "char:\"" << Ptr.character << "\"";
     break;
   case Twine::DecUIKind:
     OS << "decUI:\"" << Ptr.decUI << "\"";
     break;
   case Twine::DecIKind:
     OS << "decI:\"" << Ptr.decI << "\"";

tools/clang/include/clang/Sema/CodeCompleteConsumer.h

public:
   /// which is mainly useful for debugging.
   std::string getAsString() const;
 };
 
 /// \brief An allocator used specifically for the purpose of code completion.
 class CodeCompletionAllocator : public llvm::BumpPtrAllocator {
 public:
   /// \brief Copy the given string into this allocator.
-  const char *CopyString(StringRef String);
-
-  /// \brief Copy the given string into this allocator.
-  const char *CopyString(Twine String);
-
-  // \brief Copy the given string into this allocator.
-  const char *CopyString(const char *String) {
-    return CopyString(StringRef(String));
-  }
-
-  /// \brief Copy the given string into this allocator.
-  const char *CopyString(const std::string &String) {
-    return CopyString(StringRef(String));
-  }
+  const char *CopyString(const Twine &String);
 };
 
 /// \brief Allocator for a cached set of global code completions.
 class GlobalCodeCompletionAllocator 
   : public CodeCompletionAllocator,
     public RefCountedBase<GlobalCodeCompletionAllocator>
 {
 

tools/clang/lib/Sema/CodeCompleteConsumer.cpp

 
 const char *CodeCompletionString::getTypedText() const {
   for (iterator C = begin(), CEnd = end(); C != CEnd; ++C)
     if (C->Kind == CK_TypedText)
       return C->Text;
 
   return nullptr;
 }
 
-const char *CodeCompletionAllocator::CopyString(StringRef String) {
-  char *Mem = (char *)Allocate(String.size() + 1, 1);
-  std::copy(String.begin(), String.end(), Mem);
-  Mem[String.size()] = 0;
-  return Mem;
-}
-
-const char *CodeCompletionAllocator::CopyString(Twine String) {
+const char *CodeCompletionAllocator::CopyString(const Twine &String) {
+  SmallString<128> Data;
+  StringRef Ref = String.toStringRef(Data);
   // FIXME: It would be more efficient to teach Twine to tell us its size and
   // then add a routine there to fill in an allocated char* with the contents
   // of the string.
-  SmallString<128> Data;
-  return CopyString(String.toStringRef(Data));
+  char *Mem = (char *)Allocate(Ref.size() + 1, 1);
+  std::copy(Ref.begin(), Ref.end(), Mem);
+  Mem[Ref.size()] = 0;
+  return Mem;
 }
 
 StringRef CodeCompletionTUInfo::getParentName(const DeclContext *DC) {
   const NamedDecl *ND = dyn_cast<NamedDecl>(DC);
   if (!ND)
     return StringRef();
   
   // Check whether we've already cached the parent name.

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(SmallString<4>("hi")).str());
 }
 
 TEST(TwineTest, Numbers) {
   EXPECT_EQ("123", Twine(123U).str());
   EXPECT_EQ("123", Twine(123).str());
   EXPECT_EQ("-123", Twine(-123).str());
   EXPECT_EQ("123", Twine(123).str());
   EXPECT_EQ("-123", Twine(-123).str());
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)", 
+            repr(Twine().concat(Twine(SmallString<5>("hi")))));
+  EXPECT_EQ("(Twine smallstring:\"hey\" cstring:\"there\")", 
+            repr(Twine(SmallString<7>("hey")).concat(Twine("there"))));
 
   // 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")))));
 }
 
 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"))
+                    .toNullTerminatedStringRef(storage)
+                    .end());
 }
 
   // I suppose linking in the entire code generator to add a unit test to check
   // the code size of the concat operation is overkill... :)
 
 } // end anonymous namespace