[ADT] Make Twine's copy constructor private.

There's a lot of misuse of Twine scattered around LLVM.  This
ranges in severity from benign (returning a Twine from a function
by value that is just a string literal) to pretty sketchy (storing
a Twine by value in a class).  While there are some uses for
copying Twines, most of the very compelling ones are confined
to the Twine class implementation itself, and other uses are
either dubious or easily worked around.

This patch makes Twine's copy constructor private, and fixes up
all callsites.

Differential Revision: https://reviews.llvm.org/D38767

llvm-svn: 315530

24 files changed, 129 insertions, 110 deletions

diff --git a/clang/include/clang/Tooling/CompilationDatabase.h b/clang/include/clang/Tooling/CompilationDatabase.h
index 28af33a407d..d63afef1217 100644
--- a/clang/include/clang/Tooling/CompilationDatabase.h
+++ b/clang/include/clang/Tooling/CompilationDatabase.h
@@ -42,12 +42,10 @@ namespace tooling {
 /// \brief Specifies the working directory and command of a compilation.
 struct CompileCommand {
   CompileCommand() {}
-  CompileCommand(Twine Directory, Twine Filename,
-                 std::vector<std::string> CommandLine, Twine Output)
-      : Directory(Directory.str()),
-        Filename(Filename.str()),
-        CommandLine(std::move(CommandLine)),
-        Output(Output.str()){}
+  CompileCommand(const Twine &Directory, const Twine &Filename,
+                 std::vector<std::string> CommandLine, const Twine &Output)
+      : Directory(Directory.str()), Filename(Filename.str()),
+        CommandLine(std::move(CommandLine)), Output(Output.str()) {}
 
   /// \brief The working directory the command was executed from.
   std::string Directory;
@@ -178,13 +176,14 @@ public:
   /// \param Argv Points to the command line arguments.
   /// \param ErrorMsg Contains error text if the function returns null pointer.
   /// \param Directory The base directory used in the FixedCompilationDatabase.
-  static std::unique_ptr<FixedCompilationDatabase> loadFromCommandLine(
-      int &Argc, const char *const *Argv, std::string &ErrorMsg,
-      Twine Directory = ".");
+  static std::unique_ptr<FixedCompilationDatabase>
+  loadFromCommandLine(int &Argc, const char *const *Argv, std::string &ErrorMsg,
+                      const Twine &Directory = ".");
 
   /// \brief Constructs a compilation data base from a specified directory
   /// and command line.
-  FixedCompilationDatabase(Twine Directory, ArrayRef<std::string> CommandLine);
+  FixedCompilationDatabase(const Twine &Directory,
+                           ArrayRef<std::string> CommandLine);
 
   /// \brief Returns the given compile command.
   ///
diff --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
index 98435fefbd2..97a337a96fb 100644
--- a/clang/lib/CodeGen/CGObjCMac.cpp
+++ b/clang/lib/CodeGen/CGObjCMac.cpp
@@ -981,17 +981,17 @@ protected:
 
   /// EmitPropertyList - Emit the given property list. The return
   /// value has type PropertyListPtrTy.
-  llvm::Constant *EmitPropertyList(Twine Name,
-                                   const Decl *Container,
+  llvm::Constant *EmitPropertyList(const Twine &Name, const Decl *Container,
                                    const ObjCContainerDecl *OCD,
                                    const ObjCCommonTypesHelper &ObjCTypes,
                                    bool IsClassProperty);
 
   /// EmitProtocolMethodTypes - Generate the array of extended method type 
   /// strings. The return value has type Int8PtrPtrTy.
-  llvm::Constant *EmitProtocolMethodTypes(Twine Name, 
-                                          ArrayRef<llvm::Constant*> MethodTypes,
-                                       const ObjCCommonTypesHelper &ObjCTypes);
+  llvm::Constant *
+  EmitProtocolMethodTypes(const Twine &Name,
+                          ArrayRef<llvm::Constant *> MethodTypes,
+                          const ObjCCommonTypesHelper &ObjCTypes);
 
   /// GetProtocolRef - Return a reference to the internal protocol
   /// description, creating an empty one if it has not been
@@ -1021,11 +1021,11 @@ public:
   /// \param Align - The alignment for the variable, or 0.
   /// \param AddToUsed - Whether the variable should be added to
   ///   "llvm.used".
-  llvm::GlobalVariable *CreateMetadataVar(Twine Name,
+  llvm::GlobalVariable *CreateMetadataVar(const Twine &Name,
                                           ConstantStructBuilder &Init,
                                           StringRef Section, CharUnits Align,
                                           bool AddToUsed);
-  llvm::GlobalVariable *CreateMetadataVar(Twine Name,
+  llvm::GlobalVariable *CreateMetadataVar(const Twine &Name,
                                           llvm::Constant *Init,
                                           StringRef Section, CharUnits Align,
                                           bool AddToUsed);
@@ -1241,7 +1241,7 @@ private:
 
   /// EmitMethodList - Emit the method list for the given
   /// implementation. The return value has type MethodListPtrTy.
-  llvm::Constant *emitMethodList(Twine Name, MethodListType MLT,
+  llvm::Constant *emitMethodList(const Twine &Name, MethodListType MLT,
                                  ArrayRef<const ObjCMethodDecl *> Methods);
 
   /// GetOrEmitProtocol - Get the protocol object for the given
@@ -1265,7 +1265,7 @@ private:
 
   /// EmitProtocolList - Generate the list of referenced
   /// protocols. The return value has type ProtocolListPtrTy.
-  llvm::Constant *EmitProtocolList(Twine Name,
+  llvm::Constant *EmitProtocolList(const Twine &Name,
                                    ObjCProtocolDecl::protocol_iterator begin,
                                    ObjCProtocolDecl::protocol_iterator end);
 
@@ -1413,7 +1413,7 @@ private:
 
   /// Emit the method list for the given implementation. The return value
   /// has type MethodListnfABITy.
-  llvm::Constant *emitMethodList(Twine Name, MethodListType MLT,
+  llvm::Constant *emitMethodList(const Twine &Name, MethodListType MLT,
                                  ArrayRef<const ObjCMethodDecl *> Methods);
 
   /// EmitIvarList - Emit the ivar list for the given
@@ -1440,7 +1440,7 @@ private:
 
   /// EmitProtocolList - Generate the list of referenced
   /// protocols. The return value has type ProtocolListPtrTy.
-  llvm::Constant *EmitProtocolList(Twine Name,
+  llvm::Constant *EmitProtocolList(const Twine &Name,
                                    ObjCProtocolDecl::protocol_iterator begin,
                                    ObjCProtocolDecl::protocol_iterator end);
 
@@ -3057,7 +3057,7 @@ CGObjCMac::EmitProtocolExtension(const ObjCProtocolDecl *PD,
   };
 */
 llvm::Constant *
-CGObjCMac::EmitProtocolList(Twine name,
+CGObjCMac::EmitProtocolList(const Twine &Name,
                             ObjCProtocolDecl::protocol_iterator begin,
                             ObjCProtocolDecl::protocol_iterator end) {
   // Just return null for empty protocol lists
@@ -3090,7 +3090,7 @@ CGObjCMac::EmitProtocolList(Twine name,
     section = "__OBJC,__cat_cls_meth,regular,no_dead_strip";
 
   llvm::GlobalVariable *GV =
-      CreateMetadataVar(name, values, section, CGM.getPointerAlign(), false);
+      CreateMetadataVar(Name, values, section, CGM.getPointerAlign(), false);
   return llvm::ConstantExpr::getBitCast(GV, ObjCTypes.ProtocolListPtrTy);
 }
 
@@ -3123,11 +3123,9 @@ PushProtocolProperties(llvm::SmallPtrSet<const IdentifierInfo*,16> &PropertySet,
     struct _objc_property[prop_count];
   };
 */
-llvm::Constant *CGObjCCommonMac::EmitPropertyList(Twine Name,
-                                       const Decl *Container,
-                                       const ObjCContainerDecl *OCD,
-                                       const ObjCCommonTypesHelper &ObjCTypes,
-                                       bool IsClassProperty) {
+llvm::Constant *CGObjCCommonMac::EmitPropertyList(
+    const Twine &Name, const Decl *Container, const ObjCContainerDecl *OCD,
+    const ObjCCommonTypesHelper &ObjCTypes, bool IsClassProperty) {
   if (IsClassProperty) {
     // Make this entry NULL for OS X with deployment target < 10.11, for iOS
     // with deployment target < 9.0.
@@ -3198,10 +3196,9 @@ llvm::Constant *CGObjCCommonMac::EmitPropertyList(Twine Name,
   return llvm::ConstantExpr::getBitCast(GV, ObjCTypes.PropertyListPtrTy);
 }
 
-llvm::Constant *
-CGObjCCommonMac::EmitProtocolMethodTypes(Twine Name,
-                                         ArrayRef<llvm::Constant*> MethodTypes,
-                                         const ObjCCommonTypesHelper &ObjCTypes) {
+llvm::Constant *CGObjCCommonMac::EmitProtocolMethodTypes(
+    const Twine &Name, ArrayRef<llvm::Constant *> MethodTypes,
+    const ObjCCommonTypesHelper &ObjCTypes) {
   // Return null for empty list.
   if (MethodTypes.empty())
     return llvm::Constant::getNullValue(ObjCTypes.Int8PtrPtrTy);
@@ -3762,8 +3759,9 @@ void CGObjCMac::emitMethodConstant(ConstantArrayBuilder &builder,
 ///   int count;
 ///   struct objc_method_description list[count];
 /// };
-llvm::Constant *CGObjCMac::emitMethodList(Twine name, MethodListType MLT,
-                                 ArrayRef<const ObjCMethodDecl *> methods) {
+llvm::Constant *
+CGObjCMac::emitMethodList(const Twine &Name, MethodListType MLT,
+                          ArrayRef<const ObjCMethodDecl *> methods) {
   StringRef prefix;
   StringRef section;
   bool forProtocol = false;
@@ -3828,7 +3826,7 @@ llvm::Constant *CGObjCMac::emitMethodList(Twine name, MethodListType MLT,
     }
     methodArray.finishAndAddTo(values);
 
-    llvm::GlobalVariable *GV = CreateMetadataVar(prefix + name, values, section,
+    llvm::GlobalVariable *GV = CreateMetadataVar(prefix + Name, values, section,
                                                  CGM.getPointerAlign(), true);
     return llvm::ConstantExpr::getBitCast(GV,
                                           ObjCTypes.MethodDescriptionListPtrTy);
@@ -3845,7 +3843,7 @@ llvm::Constant *CGObjCMac::emitMethodList(Twine name, MethodListType MLT,
   }
   methodArray.finishAndAddTo(values);
 
-  llvm::GlobalVariable *GV = CreateMetadataVar(prefix + name, values, section,
+  llvm::GlobalVariable *GV = CreateMetadataVar(prefix + Name, values, section,
                                                CGM.getPointerAlign(), true);
   return llvm::ConstantExpr::getBitCast(GV, ObjCTypes.MethodListPtrTy);
 }
@@ -3868,11 +3866,9 @@ llvm::Function *CGObjCCommonMac::GenerateMethod(const ObjCMethodDecl *OMD,
   return Method;
 }
 
-llvm::GlobalVariable *CGObjCCommonMac::CreateMetadataVar(Twine Name,
-                                               ConstantStructBuilder &Init,
-                                                         StringRef Section,
-                                                         CharUnits Align,
-                                                         bool AddToUsed) {
+llvm::GlobalVariable *CGObjCCommonMac::CreateMetadataVar(
+    const Twine &Name, ConstantStructBuilder &Init, StringRef Section,
+    CharUnits Align, bool AddToUsed) {
   llvm::GlobalVariable *GV =
     Init.finishAndCreateGlobal(Name, Align, /*constant*/ false,
                                llvm::GlobalValue::PrivateLinkage);
@@ -3883,7 +3879,7 @@ llvm::GlobalVariable *CGObjCCommonMac::CreateMetadataVar(Twine Name,
   return GV;
 }
 
-llvm::GlobalVariable *CGObjCCommonMac::CreateMetadataVar(Twine Name,
+llvm::GlobalVariable *CGObjCCommonMac::CreateMetadataVar(const Twine &Name,
                                                          llvm::Constant *Init,
                                                          StringRef Section,
                                                          CharUnits Align,
@@ -6526,9 +6522,9 @@ void CGObjCNonFragileABIMac::emitMethodConstant(ConstantArrayBuilder &builder,
 ///   struct _objc_method method_list[method_count];
 /// }
 ///
-llvm::Constant *
-CGObjCNonFragileABIMac::emitMethodList(Twine name, MethodListType kind,
-                              ArrayRef<const ObjCMethodDecl *> methods) {
+llvm::Constant *CGObjCNonFragileABIMac::emitMethodList(
+    const Twine &Name, MethodListType kind,
+    ArrayRef<const ObjCMethodDecl *> methods) {
   // Return null for empty list.
   if (methods.empty())
     return llvm::Constant::getNullValue(ObjCTypes.MethodListnfABIPtrTy);
@@ -6585,7 +6581,7 @@ CGObjCNonFragileABIMac::emitMethodList(Twine name, MethodListType kind,
   }
   methodArray.finishAndAddTo(values);
 
-  auto *GV = values.finishAndCreateGlobal(prefix + name, CGM.getPointerAlign(),
+  auto *GV = values.finishAndCreateGlobal(prefix + Name, CGM.getPointerAlign(),
                                           /*constant*/ false,
                                           llvm::GlobalValue::PrivateLinkage);
   if (CGM.getTriple().isOSBinFormatMachO())
@@ -6872,10 +6868,9 @@ llvm::Constant *CGObjCNonFragileABIMac::GetOrEmitProtocol(
 /// }
 /// @endcode
 ///
-llvm::Constant *
-CGObjCNonFragileABIMac::EmitProtocolList(Twine Name,
-                                      ObjCProtocolDecl::protocol_iterator begin,
-                                      ObjCProtocolDecl::protocol_iterator end) {
+llvm::Constant *CGObjCNonFragileABIMac::EmitProtocolList(
+    const Twine &Name, ObjCProtocolDecl::protocol_iterator begin,
+    ObjCProtocolDecl::protocol_iterator end) {
   SmallVector<llvm::Constant *, 16> ProtocolRefs;
 
   // Just return null for empty protocol lists
diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 7f9c3771f14..cdaa1fd42ac 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -586,7 +586,7 @@ void BackendConsumer::UnsupportedDiagHandler(
   FullSourceLoc Loc =
       getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, Line, Column);
 
-  Diags.Report(Loc, diag::err_fe_backend_unsupported) << D.getMessage().str();
+  Diags.Report(Loc, diag::err_fe_backend_unsupported) << D.getMessage();
 
   if (BadDebugInfo)
     // If we were not able to translate the file:line:col information
diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp
index ae41ee9e22c..b0e2a7605eb 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -369,7 +369,7 @@ void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA,
 
     SmallString<128> ImplibName(Output.getFilename());
     llvm::sys::path::replace_extension(ImplibName, "lib");
-    CmdArgs.push_back(Args.MakeArgString(std::string("-implib:") + ImplibName));
+    CmdArgs.push_back(Args.MakeArgString(Twine("-implib:") + ImplibName));
   }
 
   if (TC.getSanitizerArgs().needsAsanRt()) {
diff --git a/clang/lib/Driver/ToolChains/MinGW.cpp b/clang/lib/Driver/ToolChains/MinGW.cpp
index 660b0c798ec..72a0ac4cf90 100644
--- a/clang/lib/Driver/ToolChains/MinGW.cpp
+++ b/clang/lib/Driver/ToolChains/MinGW.cpp
@@ -479,7 +479,7 @@ void toolchains::MinGW::AddClangCXXStdlibIncludeArgs(
     for (auto &CppIncludeBase : CppIncludeBases) {
       addSystemInclude(DriverArgs, CC1Args, CppIncludeBase);
       CppIncludeBase += llvm::sys::path::get_separator();
-      addSystemInclude(DriverArgs, CC1Args, CppIncludeBase + Arch);
+      addSystemInclude(DriverArgs, CC1Args, Twine(CppIncludeBase) + Arch);
       addSystemInclude(DriverArgs, CC1Args, CppIncludeBase + "backward");
     }
     break;
diff --git a/clang/lib/Tooling/CompilationDatabase.cpp b/clang/lib/Tooling/CompilationDatabase.cpp
index 0e835579e04..121e0014499 100644
--- a/clang/lib/Tooling/CompilationDatabase.cpp
+++ b/clang/lib/Tooling/CompilationDatabase.cpp
@@ -289,7 +289,7 @@ std::unique_ptr<FixedCompilationDatabase>
 FixedCompilationDatabase::loadFromCommandLine(int &Argc,
                                               const char *const *Argv,
                                               std::string &ErrorMsg,
-                                              Twine Directory) {
+                                              const Twine &Directory) {
   ErrorMsg.clear();
   if (Argc == 0)
     return nullptr;
@@ -306,8 +306,8 @@ FixedCompilationDatabase::loadFromCommandLine(int &Argc,
       new FixedCompilationDatabase(Directory, StrippedArgs));
 }
 
-FixedCompilationDatabase::
-FixedCompilationDatabase(Twine Directory, ArrayRef<std::string> CommandLine) {
+FixedCompilationDatabase::FixedCompilationDatabase(
+    const Twine &Directory, ArrayRef<std::string> CommandLine) {
   std::vector<std::string> ToolCommandLine(1, "clang-tool");
   ToolCommandLine.insert(ToolCommandLine.end(),
                          CommandLine.begin(), CommandLine.end());
diff --git a/clang/unittests/Tooling/TestVisitor.h b/clang/unittests/Tooling/TestVisitor.h
index fb6a76ccadd..a3a50cf0d49 100644
--- a/clang/unittests/Tooling/TestVisitor.h
+++ b/clang/unittests/Tooling/TestVisitor.h
@@ -128,7 +128,7 @@ public:
   /// \brief Expect 'Match' *not* to occur at the given 'Line' and 'Column'.
   ///
   /// Any number of matches can be disallowed.
-  void DisallowMatch(Twine Match, unsigned Line, unsigned Column) {
+  void DisallowMatch(const Twine &Match, unsigned Line, unsigned Column) {
     DisallowedMatches.push_back(MatchCandidate(Match, Line, Column));
   }
 
@@ -138,7 +138,7 @@ public:
   /// Each is expected to be matched 'Times' number of times. (This is useful in
   /// cases in which different AST nodes can match at the same source code
   /// location.)
-  void ExpectMatch(Twine Match, unsigned Line, unsigned Column,
+  void ExpectMatch(const Twine &Match, unsigned Line, unsigned Column,
                    unsigned Times = 1) {
     ExpectedMatches.push_back(ExpectedMatch(Match, Line, Column, Times));
   }
@@ -180,10 +180,10 @@ protected:
     unsigned LineNumber;
     unsigned ColumnNumber;
 
-    MatchCandidate(Twine Name, unsigned LineNumber, unsigned ColumnNumber)
-      : ExpectedName(Name.str()), LineNumber(LineNumber),
-        ColumnNumber(ColumnNumber) {
-    }
+    MatchCandidate(const Twine &Name, unsigned LineNumber,
+                   unsigned ColumnNumber)
+        : ExpectedName(Name.str()), LineNumber(LineNumber),
+          ColumnNumber(ColumnNumber) {}
 
     bool Matches(StringRef Name, FullSourceLoc const &Location) const {
       return MatchesName(Name) && MatchesLocation(Location);
@@ -211,7 +211,7 @@ protected:
   };
 
   struct ExpectedMatch {
-    ExpectedMatch(Twine Name, unsigned LineNumber, unsigned ColumnNumber,
+    ExpectedMatch(const Twine &Name, unsigned LineNumber, unsigned ColumnNumber,
                   unsigned Times)
         : Candidate(Name, LineNumber, ColumnNumber), TimesExpected(Times),
           TimesSeen(0) {}
diff --git a/llvm/include/llvm/ADT/Twine.h b/llvm/include/llvm/ADT/Twine.h
index f5f00dcfafe..c9cefa982d6 100644
--- a/llvm/include/llvm/ADT/Twine.h
+++ b/llvm/include/llvm/ADT/Twine.h
@@ -79,6 +79,10 @@ namespace llvm {
   /// overloads) to guarantee that particularly important cases (cstring plus
   /// StringRef) codegen as desired.
   class Twine {
+    friend Twine operator+(const char *LHS, const StringRef &RHS);
+    friend Twine operator+(const StringRef &LHS, const char *RHS);
+    friend Twine operator+(const StringRef &LHS, const StringRef &RHS);
+
     /// NodeKind - Represent the type of an argument.
     enum NodeKind : unsigned char {
       /// An empty string; the result of concatenating anything with it is also
@@ -169,6 +173,12 @@ namespace llvm {
       assert(isNullary() && "Invalid kind!");
     }
 
+    // While there are some valid use cases for copying Twines, most of them
+    // are confined to the implementation of Twine itself, and Twine itself is
+    // not intended to be publicly copyable since it can very easily lead to
+    // dangling pointers / references.
+    Twine(const Twine &) = default;
+
     /// Construct a binary twine.
     explicit Twine(const Twine &LHS, const Twine &RHS)
         : LHSKind(TwineKind), RHSKind(TwineKind) {
@@ -256,8 +266,6 @@ namespace llvm {
       assert(isValid() && "Invalid twine!");
     }
 
-    Twine(const Twine &) = default;
-
     /// Construct from a C string.
     ///
     /// We take care here to optimize "" into the empty twine -- this will be
@@ -274,6 +282,8 @@ namespace llvm {
       assert(isValid() && "Invalid twine!");
     }
 
+    Twine(Twine &&Other) = default;
+
     /// Construct from an std::string.
     /*implicit*/ Twine(const std::string &Str)
       : LHSKind(StdStringKind), RHSKind(EmptyKind) {
@@ -377,6 +387,14 @@ namespace llvm {
       assert(isValid() && "Invalid twine!");
     }
 
+    /// Construct as the concatenation of two StringRefs.
+    /*implicit*/ Twine(const StringRef &LHS, const StringRef &RHS)
+        : LHSKind(StringRefKind), RHSKind(StringRefKind) {
+      this->LHS.stringRef = &LHS;
+      this->RHS.stringRef = &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;
@@ -487,6 +505,10 @@ namespace llvm {
     /// Dump the representation of this twine to stderr.
     void dumpRepr() const;
 
+    friend inline Twine operator+(const Twine &LHS, const Twine &RHS) {
+      return LHS.concat(RHS);
+    }
+
     /// @}
   };
 
@@ -522,10 +544,6 @@ namespace llvm {
     return Twine(NewLHS, NewLHSKind, NewRHS, NewRHSKind);
   }
 
-  inline Twine operator+(const Twine &LHS, const Twine &RHS) {
-    return LHS.concat(RHS);
-  }
-
   /// Additional overload to guarantee simplified codegen; this is equivalent to
   /// concat().
 
@@ -533,13 +551,14 @@ namespace llvm {
     return Twine(LHS, RHS);
   }
 
-  /// Additional overload to guarantee simplified codegen; this is equivalent to
-  /// concat().
-
   inline Twine operator+(const StringRef &LHS, const char *RHS) {
     return Twine(LHS, RHS);
   }
 
+  inline Twine operator+(const StringRef &LHS, const StringRef &RHS) {
+    return Twine(LHS, RHS);
+  }
+
   inline raw_ostream &operator<<(raw_ostream &OS, const Twine &RHS) {
     RHS.print(OS);
     return OS;
diff --git a/llvm/include/llvm/IR/DiagnosticInfo.h b/llvm/include/llvm/IR/DiagnosticInfo.h
index 020b67d6b71..0ce9c2f9b11 100644
--- a/llvm/include/llvm/IR/DiagnosticInfo.h
+++ b/llvm/include/llvm/IR/DiagnosticInfo.h
@@ -962,7 +962,7 @@ public:
 /// Diagnostic information for unsupported feature in backend.
 class DiagnosticInfoUnsupported : public DiagnosticInfoWithLocationBase {
 private:
-  Twine Msg;
+  std::string Msg;
 
 public:
   /// \p Fn is the function where the diagnostic is being emitted. \p Loc is
@@ -976,13 +976,13 @@ public:
       const DiagnosticLocation &Loc = DiagnosticLocation(),
       DiagnosticSeverity Severity = DS_Error)
       : DiagnosticInfoWithLocationBase(DK_Unsupported, Severity, Fn, Loc),
-        Msg(Msg) {}
+        Msg(Msg.str()) {}
 
   static bool classof(const DiagnosticInfo *DI) {
     return DI->getKind() == DK_Unsupported;
   }
 
-  const Twine &getMessage() const { return Msg; }
+  StringRef getMessage() const { return Msg; }
 
   void print(DiagnosticPrinter &DP) const override;
 };
diff --git a/llvm/include/llvm/Object/Error.h b/llvm/include/llvm/Object/Error.h
index eb938338715..ef820bdba31 100644
--- a/llvm/include/llvm/Object/Error.h
+++ b/llvm/include/llvm/Object/Error.h
@@ -65,8 +65,8 @@ public:
 class GenericBinaryError : public ErrorInfo<GenericBinaryError, BinaryError> {
 public:
   static char ID;
-  GenericBinaryError(Twine Msg);
-  GenericBinaryError(Twine Msg, object_error ECOverride);
+  GenericBinaryError(const Twine &Msg);
+  GenericBinaryError(const Twine &Msg, object_error ECOverride);
   const std::string &getMessage() const { return Msg; }
   void log(raw_ostream &OS) const override;
 private:
diff --git a/llvm/include/llvm/Object/WindowsResource.h b/llvm/include/llvm/Object/WindowsResource.h
index 05fe10a770e..bf17ad3f8a8 100644
--- a/llvm/include/llvm/Object/WindowsResource.h
+++ b/llvm/include/llvm/Object/WindowsResource.h
@@ -87,7 +87,7 @@ struct WinResHeaderSuffix {
 
 class EmptyResError : public GenericBinaryError {
 public:
-  EmptyResError(Twine Msg, object_error ECOverride)
+  EmptyResError(const Twine &Msg, object_error ECOverride)
       : GenericBinaryError(Msg, ECOverride) {}
 };
 
diff --git a/llvm/include/llvm/Support/Error.h b/llvm/include/llvm/Support/Error.h
index bb40dbee053..63d441fa58e 100644
--- a/llvm/include/llvm/Support/Error.h
+++ b/llvm/include/llvm/Support/Error.h
@@ -931,7 +931,7 @@ Expected<T> handleExpected(Expected<T> ValOrErr, RecoveryFtor &&RecoveryPath,
 /// This is useful in the base level of your program to allow clean termination
 /// (allowing clean deallocation of resources, etc.), while reporting error
 /// information to the user.
-void logAllUnhandledErrors(Error E, raw_ostream &OS, Twine ErrorBanner);
+void logAllUnhandledErrors(Error E, raw_ostream &OS, const Twine &ErrorBanner);
 
 /// Write all error messages (if any) in E to a string. The newline character
 /// is used to separate error messages.
diff --git a/llvm/include/llvm/Support/FormatVariadicDetails.h b/llvm/include/llvm/Support/FormatVariadicDetails.h
index b4a564ffc26..9aa60089a80 100644
--- a/llvm/include/llvm/Support/FormatVariadicDetails.h
+++ b/llvm/include/llvm/Support/FormatVariadicDetails.h
@@ -28,7 +28,7 @@ public:
 };
 
 template <typename T> class provider_format_adapter : public format_adapter {
-  T Item;
+  T &Item;
 
 public:
   explicit provider_format_adapter(T &&Item) : Item(Item) {}
diff --git a/llvm/lib/Object/Error.cpp b/llvm/lib/Object/Error.cpp
index 7d43a84f3e0..d4ab9163886 100644
--- a/llvm/lib/Object/Error.cpp
+++ b/llvm/lib/Object/Error.cpp
@@ -60,9 +60,10 @@ std::string _object_error_category::message(int EV) const {
 char BinaryError::ID = 0;
 char GenericBinaryError::ID = 0;
 
-GenericBinaryError::GenericBinaryError(Twine Msg) : Msg(Msg.str()) {}
+GenericBinaryError::GenericBinaryError(const Twine &Msg) : Msg(Msg.str()) {}
 
-GenericBinaryError::GenericBinaryError(Twine Msg, object_error ECOverride)
+GenericBinaryError::GenericBinaryError(const Twine &Msg,
+                                       object_error ECOverride)
     : Msg(Msg.str()) {
   setErrorCode(make_error_code(ECOverride));
 }
diff --git a/llvm/lib/Support/Error.cpp b/llvm/lib/Support/Error.cpp
index bb02c03ff2b..fe8ffd817f4 100644
--- a/llvm/lib/Support/Error.cpp
+++ b/llvm/lib/Support/Error.cpp
@@ -54,7 +54,7 @@ char ErrorList::ID = 0;
 char ECError::ID = 0;
 char StringError::ID = 0;
 
-void logAllUnhandledErrors(Error E, raw_ostream &OS, Twine ErrorBanner) {
+void logAllUnhandledErrors(Error E, raw_ostream &OS, const Twine &ErrorBanner) {
   if (!E)
     return;
   OS << ErrorBanner;
diff --git a/llvm/lib/Support/Twine.cpp b/llvm/lib/Support/Twine.cpp
index d17cd4e6643..bf434f34e43 100644
--- a/llvm/lib/Support/Twine.cpp
+++ b/llvm/lib/Support/Twine.cpp
@@ -120,12 +120,10 @@ void Twine::printOneChildRepr(raw_ostream &OS, Child Ptr,
        << Ptr.cString << "\"";
     break;
   case Twine::StdStringKind:
-    OS << "std::string:\""
-       << Ptr.stdString << "\"";
+    OS << "std::string:\"" << *Ptr.stdString << "\"";
     break;
   case Twine::StringRefKind:
-    OS << "stringref:\""
-       << Ptr.stringRef << "\"";
+    OS << "stringref:\"" << *Ptr.stringRef << "\"";
     break;
   case Twine::SmallStringKind:
     OS << "smallstring:\"" << *Ptr.smallString << "\"";
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index b968cb8c892..c64c0408758 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -130,18 +130,15 @@ namespace {
 class IRBuilderPrefixedInserter : public IRBuilderDefaultInserter {
   std::string Prefix;
 
-  const Twine getNameWithPrefix(const Twine &Name) const {
-    return Name.isTriviallyEmpty() ? Name : Prefix + Name;
-  }
-
 public:
   void SetNamePrefix(const Twine &P) { Prefix = P.str(); }
 
 protected:
   void InsertHelper(Instruction *I, const Twine &Name, BasicBlock *BB,
                     BasicBlock::iterator InsertPt) const {
-    IRBuilderDefaultInserter::InsertHelper(I, getNameWithPrefix(Name), BB,
-                                           InsertPt);
+    const Twine &Prefixed = Prefix + Name;
+    IRBuilderDefaultInserter::InsertHelper(
+        I, Name.isTriviallyEmpty() ? Name : Prefixed, BB, InsertPt);
   }
 };
 
@@ -1355,7 +1352,8 @@ static void speculateSelectInstLoads(SelectInst &SI) {
 /// This will return the BasePtr if that is valid, or build a new GEP
 /// instruction using the IRBuilder if GEP-ing is needed.
 static Value *buildGEP(IRBuilderTy &IRB, Value *BasePtr,
-                       SmallVectorImpl<Value *> &Indices, Twine NamePrefix) {
+                       SmallVectorImpl<Value *> &Indices,
+                       const Twine &NamePrefix) {
   if (Indices.empty())
     return BasePtr;
 
@@ -1380,7 +1378,7 @@ static Value *buildGEP(IRBuilderTy &IRB, Value *BasePtr,
 static Value *getNaturalGEPWithType(IRBuilderTy &IRB, const DataLayout &DL,
                                     Value *BasePtr, Type *Ty, Type *TargetTy,
                                     SmallVectorImpl<Value *> &Indices,
-                                    Twine NamePrefix) {
+                                    const Twine &NamePrefix) {
   if (Ty == TargetTy)
     return buildGEP(IRB, BasePtr, Indices, NamePrefix);
 
@@ -1425,7 +1423,7 @@ static Value *getNaturalGEPRecursively(IRBuilderTy &IRB, const DataLayout &DL,
                                        Value *Ptr, Type *Ty, APInt &Offset,
                                        Type *TargetTy,
                                        SmallVectorImpl<Value *> &Indices,
-                                       Twine NamePrefix) {
+                                       const Twine &NamePrefix) {
   if (Offset == 0)
     return getNaturalGEPWithType(IRB, DL, Ptr, Ty, TargetTy, Indices,
                                  NamePrefix);
@@ -1498,7 +1496,7 @@ static Value *getNaturalGEPRecursively(IRBuilderTy &IRB, const DataLayout &DL,
 static Value *getNaturalGEPWithOffset(IRBuilderTy &IRB, const DataLayout &DL,
                                       Value *Ptr, APInt Offset, Type *TargetTy,
                                       SmallVectorImpl<Value *> &Indices,
-                                      Twine NamePrefix) {
+                                      const Twine &NamePrefix) {
   PointerType *Ty = cast<PointerType>(Ptr->getType());
 
   // Don't consider any GEPs through an i8* as natural unless the TargetTy is
@@ -1536,7 +1534,8 @@ static Value *getNaturalGEPWithOffset(IRBuilderTy &IRB, const DataLayout &DL,
 /// a single GEP as possible, thus making each GEP more independent of the
 /// surrounding code.
 static Value *getAdjustedPtr(IRBuilderTy &IRB, const DataLayout &DL, Value *Ptr,
-                             APInt Offset, Type *PointerTy, Twine NamePrefix) {
+                             APInt Offset, Type *PointerTy,
+                             const Twine &NamePrefix) {
   // Even though we don't look through PHI nodes, we could be called on an
   // instruction in an unreachable block, which may be on a cycle.
   SmallPtrSet<Value *, 4> Visited;
diff --git a/llvm/tools/llvm-nm/llvm-nm.cpp b/llvm/tools/llvm-nm/llvm-nm.cpp
index 4ad0d95d67f..6b9ae463979 100644
--- a/llvm/tools/llvm-nm/llvm-nm.cpp
+++ b/llvm/tools/llvm-nm/llvm-nm.cpp
@@ -195,12 +195,12 @@ bool HadError = false;
 std::string ToolName;
 } // anonymous namespace
 
-static void error(Twine Message, Twine Path = Twine()) {
+static void error(const Twine &Message, const Twine &Path = Twine()) {
   HadError = true;
   errs() << ToolName << ": " << Path << ": " << Message << ".\n";
 }
 
-static bool error(std::error_code EC, Twine Path = Twine()) {
+static bool error(std::error_code EC, const Twine &Path = Twine()) {
   if (EC) {
     error(EC.message(), Path);
     return true;
diff --git a/llvm/tools/llvm-objcopy/Object.cpp b/llvm/tools/llvm-objcopy/Object.cpp
index f9acf001ae9..f4c159cd1cb 100644
--- a/llvm/tools/llvm-objcopy/Object.cpp
+++ b/llvm/tools/llvm-objcopy/Object.cpp
@@ -428,15 +428,15 @@ void initRelocations(RelocationSection<ELFT> *Relocs,
   }
 }
 
-SectionBase *SectionTableRef::getSection(uint16_t Index, Twine ErrMsg) {
+SectionBase *SectionTableRef::getSection(uint16_t Index, const Twine &ErrMsg) {
   if (Index == SHN_UNDEF || Index > Sections.size())
     error(ErrMsg);
   return Sections[Index - 1].get();
 }
 
 template <class T>
-T *SectionTableRef::getSectionOfType(uint16_t Index, Twine IndexErrMsg,
-                                     Twine TypeErrMsg) {
+T *SectionTableRef::getSectionOfType(uint16_t Index, const Twine &IndexErrMsg,
+                                     const Twine &TypeErrMsg) {
   if (T *Sec = llvm::dyn_cast<T>(getSection(Index, IndexErrMsg)))
     return Sec;
   error(TypeErrMsg);
diff --git a/llvm/tools/llvm-objcopy/Object.h b/llvm/tools/llvm-objcopy/Object.h
index f6088434805..d266912db0b 100644
--- a/llvm/tools/llvm-objcopy/Object.h
+++ b/llvm/tools/llvm-objcopy/Object.h
@@ -29,11 +29,11 @@ public:
       : Sections(Secs) {}
   SectionTableRef(const SectionTableRef &) = default;
 
-  SectionBase *getSection(uint16_t Index, llvm::Twine ErrMsg);
+  SectionBase *getSection(uint16_t Index, const llvm::Twine &ErrMsg);
 
   template <class T>
-  T *getSectionOfType(uint16_t Index, llvm::Twine IndexErrMsg,
-                      llvm::Twine TypeErrMsg);
+  T *getSectionOfType(uint16_t Index, const llvm::Twine &IndexErrMsg,
+                      const llvm::Twine &TypeErrMsg);
 };
 
 class SectionBase {
diff --git a/llvm/tools/llvm-objcopy/llvm-objcopy.cpp b/llvm/tools/llvm-objcopy/llvm-objcopy.cpp
index 7f55a434b33..9fc2897959c 100644
--- a/llvm/tools/llvm-objcopy/llvm-objcopy.cpp
+++ b/llvm/tools/llvm-objcopy/llvm-objcopy.cpp
@@ -27,7 +27,7 @@ static StringRef ToolName;
 
 namespace llvm {
 
-LLVM_ATTRIBUTE_NORETURN void error(Twine Message) {
+LLVM_ATTRIBUTE_NORETURN void error(const Twine &Message) {
   errs() << ToolName << ": " << Message << ".\n";
   errs().flush();
   exit(1);
diff --git a/llvm/tools/llvm-objcopy/llvm-objcopy.h b/llvm/tools/llvm-objcopy/llvm-objcopy.h
index de7bf367ac8..d30b43a46a4 100644
--- a/llvm/tools/llvm-objcopy/llvm-objcopy.h
+++ b/llvm/tools/llvm-objcopy/llvm-objcopy.h
@@ -14,7 +14,7 @@
 
 namespace llvm {
 
-LLVM_ATTRIBUTE_NORETURN extern void error(Twine Message);
+LLVM_ATTRIBUTE_NORETURN extern void error(const Twine &Message);
 
 // This is taken from llvm-readobj.
 // [see here](llvm/tools/llvm-readobj/llvm-readobj.h:38)
diff --git a/llvm/unittests/ADT/TwineTest.cpp b/llvm/unittests/ADT/TwineTest.cpp
index 950eda2b058..ebe06917cee 100644
--- a/llvm/unittests/ADT/TwineTest.cpp
+++ b/llvm/unittests/ADT/TwineTest.cpp
@@ -88,6 +88,14 @@ TEST(TwineTest, Concat) {
             repr(Twine("a").concat(Twine(SmallString<3>("b")).concat(Twine("c")))));
 }
 
+TEST(TwineTest, Operators) {
+  EXPECT_EQ(R"((Twine cstring:"a" stringref:"b"))", repr("a" + StringRef("b")));
+
+  EXPECT_EQ(R"((Twine stringref:"a" cstring:"b"))", repr(StringRef("a") + "b"));
+  EXPECT_EQ(R"((Twine stringref:"a" stringref:"b"))",
+            repr(StringRef("a") + StringRef("b")));
+}
+
 TEST(TwineTest, toNullTerminatedStringRef) {
   SmallString<8> storage;
   EXPECT_EQ(0, *Twine("hello").toNullTerminatedStringRef(storage).end());
diff --git a/llvm/utils/TableGen/RegisterBankEmitter.cpp b/llvm/utils/TableGen/RegisterBankEmitter.cpp
index 293933ffb8d..36f66f4fe8b 100644
--- a/llvm/utils/TableGen/RegisterBankEmitter.cpp
+++ b/llvm/utils/TableGen/RegisterBankEmitter.cpp
@@ -169,7 +169,7 @@ void RegisterBankEmitter::emitBaseClassDefinition(
 ///                to the class.
 static void visitRegisterBankClasses(
     CodeGenRegBank &RegisterClassHierarchy, const CodeGenRegisterClass *RC,
-    const Twine Kind,
+    const Twine &Kind,
     std::function<void(const CodeGenRegisterClass *, StringRef)> VisitFn,
     SmallPtrSetImpl<const CodeGenRegisterClass *> &VisitedRCs) {
 
@@ -183,7 +183,7 @@ static void visitRegisterBankClasses(
 
   for (const auto &PossibleSubclass : RegisterClassHierarchy.getRegClasses()) {
     std::string TmpKind =
-        (Twine(Kind) + " (" + PossibleSubclass.getName() + ")").str();
+        (Kind + " (" + PossibleSubclass.getName() + ")").str();
 
     // Visit each subclass of an explicitly named class.
     if (RC != &PossibleSubclass && RC->hasSubClass(&PossibleSubclass))