Make Twine's copy constructor private

[zturner]

Summary

I was running across a lot of abuse of Twine, some of which probably worked, and some of which came very close to skirting the line of safety. Examples of this include:

1. Accepting Twine by value instead of by const reference.
2. Returning Twines by value from functions.
3. Storing Twines by value in classes.

and others. The 3rd of those mentioned was particularly onerous because it can happen without you even knowing it, for example if you have:

template<class T>
struct storage {
  T t;
};

and then you use a function such as make_storage to perfectly forward args through, and one of those happens to be a twine.

Ultimately, Twine really should not be copyable. The header file even hints at this by saying "functions should only accept Twine by const reference". But since this rule is not enforced, many violations of this were occurring.

I've made the copy constructor private here (since Twine internally has a few valid use cases for a copy constructor), and fixed up all uses.

In testing this, I noticed that there were no unit tests for Twine's operator helpers, so I added some there (which incidentally exposed a bug in the implementation of printOneRepr, which is also now fixed)

[dblaikie]

llvm/include/llvm/ADT/Twine.h

176 ↗

(On Diff #118492)

I'd probably skip the "Copy constructor is private" - as I'm guessing that's fairly obvious from context.

& "We need" - I think it's usually better to impersonalize comments "The copy constructor is used internally but Twines are not intended to be publicly copyable." perhaps.

cfe/trunk/include/clang/Tooling/CompilationDatabase.h

 #include <vector>
 
 namespace clang {
 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;
 
   /// The source file associated with the command.
   std::string Filename;
 
   /// \brief The command line that was executed.
public:
   /// }
   ///
   /// \param Argc The number of command line arguments - will be changed to
   /// the number of arguments before "--", if "--" was found in the argument
   /// list.
   /// \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.
   ///
   /// Will always return a vector with one entry that contains the directory
   /// and command line specified at construction with "clang-tool" as argv[0]
   /// and 'FilePath' as positional argument.
   std::vector<CompileCommand>
   getCompileCommands(StringRef FilePath) const override;

cfe/trunk/lib/CodeGen/CGObjCMac.cpp

protected:
   
   /// GetIvarLayoutName - Returns a unique constant for the given
   /// ivar layout bitmap.
   llvm::Constant *GetIvarLayoutName(IdentifierInfo *Ident,
                                     const ObjCCommonTypesHelper &ObjCTypes);
 
   /// 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,
 
   /// GetProtocolRef - Return a reference to the internal protocol
   /// description, creating an empty one if it has not been
   /// defined. The return value has type ProtocolPtrTy.
   llvm::Constant *GetProtocolRef(const ObjCProtocolDecl *PD);
 
   /// Return a reference to the given Class using runtime calls rather than
   /// by a symbol reference.
public:
   ///
   /// \param Name - The variable name.
   /// \param Init - The variable initializer; this is also used to
   ///   define the type of the variable.
   /// \param Section - The section the variable should go into, or empty.
   /// \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);
 
   llvm::GlobalVariable *CreateCStringLiteral(StringRef Name,
                                              ObjCLabelType LabelType,
                                              bool ForceNonFragileABI = false,
                                              bool NullTerminate = true);
private:
   void emitMethodConstant(ConstantArrayBuilder &builder,
                           const ObjCMethodDecl *MD);
 
   void emitMethodDescriptionConstant(ConstantArrayBuilder &builder,
                                      const ObjCMethodDecl *MD);
 
   /// 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
   /// declaration, emitting it if necessary. The return value has type
   /// ProtocolPtrTy.
   llvm::Constant *GetOrEmitProtocol(const ObjCProtocolDecl *PD) override;
 
   /// GetOrEmitProtocolRef - Get a forward reference to the protocol
   /// object for the given declaration, emitting it if needed. These
   /// forward references will be filled in with empty bodies if no
   /// definition is seen. The return value has type ProtocolPtrTy.
   llvm::Constant *GetOrEmitProtocolRef(const ObjCProtocolDecl *PD) override;
 
   /// EmitProtocolExtension - Generate the protocol extension
   /// structure used to store optional instance and class methods, and
   /// protocol properties. The return value has type
   /// ProtocolExtensionPtrTy.
   llvm::Constant *
   EmitProtocolExtension(const ObjCProtocolDecl *PD,
                         const ProtocolMethodLists &methodLists);
 
   /// 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);
 
   /// EmitSelector - Return a Value*, of type ObjCTypes.SelectorPtrTy,
   /// for the given selector.
   llvm::Value *EmitSelector(CodeGenFunction &CGF, Selector Sel);
   Address EmitSelectorAddr(CodeGenFunction &CGF, Selector Sel);
 
llvm::GlobalVariable *BuildClassObject(const ObjCInterfaceDecl *CI,
                                          bool HiddenVisibility);
 
   void emitMethodConstant(ConstantArrayBuilder &builder,
                             const ObjCMethodDecl *MD,
                             bool forProtocol);
 
   /// 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
   /// implementation. If ForClass is true the list of class ivars
   /// (i.e. metaclass ivars) is emitted, otherwise the list of
   /// interface ivars will be emitted. The return value has type
   /// IvarListnfABIPtrTy.
   llvm::Constant *EmitIvarList(const ObjCImplementationDecl *ID);
private:
   /// GetOrEmitProtocolRef - Get a forward reference to the protocol
   /// object for the given declaration, emitting it if needed. These
   /// forward references will be filled in with empty bodies if no
   /// definition is seen. The return value has type ProtocolPtrTy.
   llvm::Constant *GetOrEmitProtocolRef(const ObjCProtocolDecl *PD) override;
 
   /// 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);
 
   CodeGen::RValue EmitVTableMessageSend(CodeGen::CodeGenFunction &CGF,
                                         ReturnValueSlot Return,
                                         QualType ResultType,
                                         Selector Sel,
                                         llvm::Value *Receiver,

 /*
   struct objc_protocol_list {
     struct objc_protocol_list *next;
     long count;
     Protocol *list[];
   };
 */
 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
   if (begin == end)
     return llvm::Constant::getNullValue(ObjCTypes.ProtocolListPtrTy);
 
   ConstantInitBuilder builder(CGM);
   auto values = builder.beginStruct();
CGObjCMac::EmitProtocolList(const Twine &Name,
   refsArray.finishAndAddTo(values);
   values.fillPlaceholderWithInt(countSlot, ObjCTypes.LongTy, count);
 
   StringRef section;
   if (CGM.getTriple().isOSBinFormatMachO())
     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);
 }
 
 static void 
 PushProtocolProperties(llvm::SmallPtrSet<const IdentifierInfo*,16> &PropertySet,
                        SmallVectorImpl<const ObjCPropertyDecl *> &Properties,
                        const ObjCProtocolDecl *Proto,
                        bool IsClassProperty) {
/*
   };
 
   struct _objc_property_list {
     uint32_t entsize; // sizeof (struct _objc_property)
     uint32_t prop_count;
     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.
     const llvm::Triple &Triple = CGM.getTarget().getTriple();
     if ((Triple.isMacOSX() && Triple.isMacOSXVersionLT(10, 11)) ||
         (Triple.isiOS() && Triple.isOSVersionLT(9)))
       return llvm::Constant::getNullValue(ObjCTypes.PropertyListPtrTy);
   }
if (CGM.getTriple().isOSBinFormatMachO())
     Section = (ObjCABI == 2) ? "__DATA, __objc_const"
                              : "__OBJC,__property,regular,no_dead_strip";
 
   llvm::GlobalVariable *GV =
       CreateMetadataVar(Name, values, Section, CGM.getPointerAlign(), true);
   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,
   // Return null for empty list.
   if (MethodTypes.empty())
     return llvm::Constant::getNullValue(ObjCTypes.Int8PtrPtrTy);
 
   llvm::ArrayType *AT = llvm::ArrayType::get(ObjCTypes.Int8PtrTy,
                                              MethodTypes.size());
   llvm::Constant *Init = llvm::ConstantArray::get(AT, MethodTypes);
 

 ///   int count;
 ///   struct objc_method methods_list[count];
 /// };
 ///
 /// struct objc_method_description_list {
 ///   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,
   StringRef prefix;
   StringRef section;
   bool forProtocol = false;
   switch (MLT) {
   case MethodListType::CategoryInstanceMethods:
     prefix = "OBJC_CATEGORY_INSTANCE_METHODS_";
     section = "__OBJC,__cat_inst_meth,regular,no_dead_strip";
     forProtocol = false;
if (forProtocol) {
     auto values = builder.beginStruct();
     values.addInt(ObjCTypes.IntTy, methods.size());
     auto methodArray = values.beginArray(ObjCTypes.MethodDescriptionTy);
     for (auto MD : methods) {
       emitMethodDescriptionConstant(methodArray, MD);
     }
     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);
   }
 
   // Otherwise, it's an objc_method_list.
   ConstantInitBuilder builder(CGM);
   auto values = builder.beginStruct();
   values.addNullPointer(ObjCTypes.Int8PtrTy);
   values.addInt(ObjCTypes.IntTy, methods.size());
   auto methodArray = values.beginArray(ObjCTypes.MethodTy);
   for (auto MD : methods) {
     emitMethodConstant(methodArray, MD);
   }
   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);
 }
 
 llvm::Function *CGObjCCommonMac::GenerateMethod(const ObjCMethodDecl *OMD,
                                                 const ObjCContainerDecl *CD) {
   SmallString<256> Name;
   GetNameForMethod(OMD, CD, Name);
 
   CodeGenTypes &Types = CGM.getTypes();
   llvm::FunctionType *MethodTy =
     Types.GetFunctionType(Types.arrangeObjCMethodDeclaration(OMD));
   llvm::Function *Method =
     llvm::Function::Create(MethodTy,
                            llvm::GlobalValue::InternalLinkage,
                            Name.str(),
                            &CGM.getModule());
   MethodDefinitions.insert(std::make_pair(OMD, Method));
 
   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);
   if (!Section.empty())
     GV->setSection(Section);
   if (AddToUsed)
     CGM.addCompilerUsedGlobal(GV);
   return GV;
 }
 
-llvm::GlobalVariable *CGObjCCommonMac::CreateMetadataVar(Twine Name,
+llvm::GlobalVariable *CGObjCCommonMac::CreateMetadataVar(const Twine &Name,
                                                          llvm::Constant *Init,
                                                          StringRef Section,
                                                          CharUnits Align,
                                                          bool AddToUsed) {
   llvm::Type *Ty = Init->getType();
   llvm::GlobalVariable *GV =
     new llvm::GlobalVariable(CGM.getModule(), Ty, false,
                              llvm::GlobalValue::PrivateLinkage, Init, Name);

 /// Build meta-data for method declarations.
 ///
 /// struct _method_list_t {
 ///   uint32_t entsize;  // sizeof(struct _objc_method)
 ///   uint32_t method_count;
 ///   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,
   // Return null for empty list.
   if (methods.empty())
     return llvm::Constant::getNullValue(ObjCTypes.MethodListnfABIPtrTy);
 
   StringRef prefix;
   bool forProtocol;
   switch (kind) {
   case MethodListType::CategoryInstanceMethods:
llvm::Constant *CGObjCNonFragileABIMac::emitMethodList(
   // method_count
   values.addInt(ObjCTypes.IntTy, methods.size());
   auto methodArray = values.beginArray(ObjCTypes.MethodTy);
   for (auto MD : methods) {
     emitMethodConstant(methodArray, MD, forProtocol);
   }
   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())
     GV->setSection("__DATA, __objc_const");
   CGM.addCompilerUsedGlobal(GV);
   return llvm::ConstantExpr::getBitCast(GV, ObjCTypes.MethodListnfABIPtrTy);
 }
 

 /// EmitProtocolList - Generate protocol list meta-data:
 /// @code
 /// struct _protocol_list_t {
 ///   long protocol_count;   // Note, this is 32/64 bit
 ///   struct _protocol_t[protocol_count];
 /// }
 /// @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,
   SmallVector<llvm::Constant *, 16> ProtocolRefs;
 
   // Just return null for empty protocol lists
   if (begin == end)
     return llvm::Constant::getNullValue(ObjCTypes.ProtocolListnfABIPtrTy);
 
   // FIXME: We shouldn't need to do this lookup here, should we?
   SmallString<256> TmpName;

cfe/trunk/lib/CodeGen/CodeGenAction.cpp

void BackendConsumer::UnsupportedDiagHandler(
   assert(D.getSeverity() == llvm::DS_Error);
 
   StringRef Filename;
   unsigned Line, Column;
   bool BadDebugInfo = false;
   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
     // back to a SourceLocation, at least emit a note stating that
     // we could not translate this location. This can happen in the
     // case of #line directives.
     Diags.Report(Loc, diag::note_fe_backend_invalid_loc)
         << Filename << Line << Column;

cfe/trunk/lib/Driver/ToolChains/MSVC.cpp

void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA,
 
   bool DLL = Args.hasArg(options::OPT__SLASH_LD, options::OPT__SLASH_LDd,
                          options::OPT_shared);
   if (DLL) {
     CmdArgs.push_back(Args.MakeArgString("-dll"));
 
     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()) {
     CmdArgs.push_back(Args.MakeArgString("-debug"));
     CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
     if (TC.getSanitizerArgs().needsSharedRt() ||
         Args.hasArg(options::OPT__SLASH_MD, options::OPT__SLASH_MDd)) {
       for (const auto &Lib : {"asan_dynamic", "asan_dynamic_runtime_thunk"})

cfe/trunk/lib/Driver/ToolChains/MinGW.cpp

case ToolChain::CST_Libstdcxx:
     llvm::sys::path::append(CppIncludeBases[1], Arch, "include", "c++", Ver);
     CppIncludeBases.emplace_back(Base);
     llvm::sys::path::append(CppIncludeBases[2], "include", "c++", Ver);
     CppIncludeBases.emplace_back(GccLibDir);
     llvm::sys::path::append(CppIncludeBases[3], "include", "c++");
     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;
   }
 }

cfe/trunk/lib/Tooling/CompilationDatabase.cpp

static bool stripPositionalArgs(std::vector<const char *> Args,
   Result = std::vector<std::string>(Args.begin() + 1, End);
   return true;
 }
 
 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;
   const char *const *DoubleDash = std::find(Argv, Argv + Argc, StringRef("--"));
   if (DoubleDash == Argv + Argc)
     return nullptr;
   std::vector<const char *> CommandLine(DoubleDash + 1, Argv + Argc);
   Argc = DoubleDash - Argv;
 
   std::vector<std::string> StrippedArgs;
   if (!stripPositionalArgs(CommandLine, StrippedArgs, ErrorMsg))
     return nullptr;
   return std::unique_ptr<FixedCompilationDatabase>(
       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());
   CompileCommands.emplace_back(Directory, StringRef(),
                                std::move(ToolCommandLine),
                                StringRef());
 }
 

cfe/trunk/unittests/Tooling/TestVisitor.h

 /// and allows simple creation of test visitors running matches on only a small
 /// subset of the Visit* methods.
 template <typename T, template <typename> class Visitor = TestVisitor>
 class ExpectedLocationVisitor : public Visitor<T> {
 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));
   }
 
   /// \brief Expect 'Match' to occur at the given 'Line' and 'Column'.
   ///
   /// Any number of expected matches can be set by calling this repeatedly.
   /// 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));
   }
 
   /// \brief Checks that all expected matches have been found.
   ~ExpectedLocationVisitor() override {
     for (typename std::vector<ExpectedMatch>::const_iterator
              It = ExpectedMatches.begin(), End = ExpectedMatches.end();
protected:
   }
 
  private:
   struct MatchCandidate {
     std::string ExpectedName;
     unsigned LineNumber;
     unsigned ColumnNumber;
 
-    MatchCandidate(Twine Name, unsigned LineNumber, unsigned ColumnNumber)
+    MatchCandidate(const Twine &Name, unsigned LineNumber,
+                   unsigned ColumnNumber)
         : ExpectedName(Name.str()), LineNumber(LineNumber),
-        ColumnNumber(ColumnNumber) {
-    }
+          ColumnNumber(ColumnNumber) {}
 
     bool Matches(StringRef Name, FullSourceLoc const &Location) const {
       return MatchesName(Name) && MatchesLocation(Location);
     }
 
     bool PartiallyMatches(StringRef Name, FullSourceLoc const &Location) const {
       return MatchesName(Name) || MatchesLocation(Location);
     }
struct MatchCandidate {
     friend std::ostream &operator<<(std::ostream &Stream,
                                     MatchCandidate const &Match) {
       return Stream << Match.ExpectedName
                     << " at " << Match.LineNumber << ":" << Match.ColumnNumber;
     }
   };
 
   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) {}
 
     void UpdateFor(StringRef Name, FullSourceLoc Location, SourceManager &SM) {
       if (Candidate.Matches(Name, Location)) {
         EXPECT_LT(TimesSeen, TimesExpected);
         ++TimesSeen;

llvm/trunk/include/llvm/ADT/Twine.h

namespace llvm {
   /// concatenations of leaf nodes can be optimized into stores directly into a
   /// single stack allocated object.
   ///
   /// In practice, not all compilers can be trusted to optimize concat() fully,
   /// so we provide two additional methods (and accompanying operator+
   /// 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
       /// empty.
       NullKind,
 
       /// The empty string.
       EmptyKind,
class Twine {
     NodeKind RHSKind;
 
     /// Construct a nullary twine; the kind must be NullKind or EmptyKind.
     explicit Twine(NodeKind Kind)
       : LHSKind(Kind), RHSKind(EmptyKind) {
       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) {
       this->LHS.twine = &LHS;
       this->RHS.twine = &RHS;
       assert(isValid() && "Invalid twine!");
     }
 
public:
     /// @name Constructors
     /// @{
 
     /// Construct from an empty string.
     /*implicit*/ Twine() : LHSKind(EmptyKind), RHSKind(EmptyKind) {
       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
     /// optimized out for string constants. This allows Twine arguments have
     /// default "" values, without introducing unnecessary string constants.
     /*implicit*/ Twine(const char *Str)
       : RHSKind(EmptyKind) {
       if (Str[0] != '\0') {
         LHS.cString = Str;
         LHSKind = CStringKind;
       } else
         LHSKind = EmptyKind;
 
       assert(isValid() && "Invalid twine!");
     }
 
+    Twine(Twine &&Other) = default;
+
     /// Construct from an std::string.
     /*implicit*/ Twine(const std::string &Str)
       : LHSKind(StdStringKind), RHSKind(EmptyKind) {
       LHS.stdString = &Str;
       assert(isValid() && "Invalid twine!");
     }
 
     /// Construct from a StringRef.
public:
     /// 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;
       this->RHS.cString = RHS;
       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;
 
     /// Create a 'null' string, which is an empty string that always
     /// concatenates to form another empty string.
     static Twine createNull() {
       return Twine(NullKind);
public:
     void dump() const;
 
     /// Write the representation of this twine to the stream \p OS.
     void printRepr(raw_ostream &OS) const;
 
     /// 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);
+    }
+
     /// @}
   };
 
   /// @name Twine Inline Implementations
   /// @{
 
   inline Twine Twine::concat(const Twine &Suffix) const {
     // Concatenation with null is null.
inline Twine Twine::concat(const Twine &Suffix) const {
     if (Suffix.isUnary()) {
       NewRHS = Suffix.LHS;
       NewRHSKind = Suffix.getLHSKind();
     }
 
     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().
 
   inline Twine operator+(const char *LHS, const StringRef &RHS) {
     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;
   }
 
   /// @}
 
 } // end namespace llvm
 
 #endif // LLVM_ADT_TWINE_H

llvm/trunk/include/llvm/IR/DiagnosticInfo.h

public:
 
   /// \see DiagnosticInfoOptimizationBase::isEnabled.
   bool isEnabled() const override;
 };
 
 /// 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
   /// the location information to use in the diagnostic. If line table
   /// information is available, the diagnostic will include the source code
   /// location. \p Msg is the message to show. Note that this class does not
   /// copy this message, so this reference must be valid for the whole life time
   /// of the diagnostic.
   DiagnosticInfoUnsupported(
       const Function &Fn, const Twine &Msg,
       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;
 };
 
 namespace yaml {
 template <> struct MappingTraits<DiagnosticInfoOptimizationBase *> {
   static void mapping(IO &io, DiagnosticInfoOptimizationBase *&OptDiag);
 };
 } // namespace yaml
 
 } // end namespace llvm
 
 #endif // LLVM_IR_DIAGNOSTICINFO_H

llvm/trunk/include/llvm/Object/Error.h

 
 /// Generic binary error.
 ///
 /// For errors that don't require their own specific sub-error (most errors)
 /// this class can be used to describe the error via a string message.
 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:
   std::string Msg;
 };
 
 /// isNotObjectErrorInvalidFileType() is used when looping through the children
 /// of an archive after calling getAsBinary() on the child and it returns an

llvm/trunk/include/llvm/Object/WindowsResource.h

struct WinResHeaderSuffix {
   support::ulittle16_t MemoryFlags;
   support::ulittle16_t Language;
   support::ulittle32_t Version;
   support::ulittle32_t Characteristics;
 };
 
 class EmptyResError : public GenericBinaryError {
 public:
-  EmptyResError(Twine Msg, object_error ECOverride)
+  EmptyResError(const Twine &Msg, object_error ECOverride)
       : GenericBinaryError(Msg, ECOverride) {}
 };
 
 class ResourceEntryRef {
 public:
   Error moveNext(bool &End);
   bool checkTypeString() const { return IsStringType; }
   ArrayRef<UTF16> getTypeString() const { return Type; }

llvm/trunk/include/llvm/Support/Error.h

 
 /// Log all errors (if any) in E to OS. If there are any errors, ErrorBanner
 /// will be printed before the first one is logged. A newline will be printed
 /// after each error.
 ///
 /// 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.
 inline std::string toString(Error E) {
   SmallVector<std::string, 2> Errors;
   handleAllErrors(std::move(E), [&Errors](const ErrorInfoBase &EI) {
     Errors.push_back(EI.message());
   });

llvm/trunk/include/llvm/Support/FormatVariadicDetails.h

 protected:
   virtual ~format_adapter() {}
 
 public:
   virtual void format(raw_ostream &S, StringRef Options) = 0;
 };
 
 template <typename T> class provider_format_adapter : public format_adapter {
-  T Item;
+  T &Item;
 
 public:
   explicit provider_format_adapter(T &&Item) : Item(Item) {}
 
   void format(llvm::raw_ostream &S, StringRef Options) override {
     format_provider<typename std::decay<T>::type>::format(Item, S, Options);
   }
 };

llvm/trunk/lib/Object/Error.cpp

std::string _object_error_category::message(int EV) const {
   }
   llvm_unreachable("An enumerator of object_error does not have a message "
                    "defined.");
 }
 
 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));
 }
 
 void GenericBinaryError::log(raw_ostream &OS) const {
   OS << Msg;
 }
 

llvm/trunk/lib/Support/Error.cpp

 namespace llvm {
 
 void ErrorInfoBase::anchor() {}
 char ErrorInfoBase::ID = 0;
 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;
   handleAllErrors(std::move(E), [&](const ErrorInfoBase &EI) {
     EI.log(OS);
     OS << "\n";
   });
 }

llvm/trunk/lib/Support/Twine.cpp

case Twine::TwineKind:
     OS << "rope:";
     Ptr.twine->printRepr(OS);
     break;
   case Twine::CStringKind:
     OS << "cstring:\""
        << 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 << "\"";
     break;
   case Twine::FormatvObjectKind:
     OS << "formatv:\"" << *Ptr.formatvObject << "\"";
     break;
   case Twine::CharKind:

llvm/trunk/lib/Transforms/Scalar/SROA.cpp

 
 namespace {
 
 /// \brief A custom IRBuilder inserter which prefixes all names, but only in
 /// Assert builds.
 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);
   }
 };
 
 /// \brief Provide a type for IRBuilder that drops names in release builds.
 using IRBuilderTy = IRBuilder<ConstantFolder, IRBuilderPrefixedInserter>;
 
 /// \brief A used slice of an alloca.
 ///
static void speculateSelectInstLoads(SelectInst &SI) {
   SI.eraseFromParent();
 }
 
 /// \brief Build a GEP out of a base pointer and indices.
 ///
 /// 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;
 
   // A single zero index is a no-op, so check for this and avoid building a GEP
   // in that case.
   if (Indices.size() == 1 && cast<ConstantInt>(Indices.back())->isZero())
     return BasePtr;
 
   return IRB.CreateInBoundsGEP(nullptr, BasePtr, Indices,
                                NamePrefix + "sroa_idx");
 }
 
 /// \brief Get a natural GEP off of the BasePtr walking through Ty toward
 /// TargetTy without changing the offset of the pointer.
 ///
 /// This routine assumes we've already established a properly offset GEP with
 /// Indices, and arrived at the Ty type. The goal is to continue to GEP with
 /// zero-indices down through type layers until we find one the same as
 /// TargetTy. If we can't find one with the same type, we at least try to use
 /// one with the same size. If none of that works, we just produce the GEP as
 /// indicated by Indices to have the correct offset.
 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);
 
   // Pointer size to use for the indices.
   unsigned PtrSize = DL.getPointerTypeSizeInBits(BasePtr->getType());
 
   // See if we can descend into a struct and locate a field with the correct
   // type.

 /// \brief Recursively compute indices for a natural GEP.
 ///
 /// This is the recursive step for getNaturalGEPWithOffset that walks down the
 /// element types adding appropriate indices for the GEP.
 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);
 
   // We can't recurse through pointer types.
   if (Ty->isPointerTy())
     return nullptr;
 

 /// a pointer. TargetTy is the element type the returned GEP should point-to if
 /// possible. We recurse by decreasing Offset, adding the appropriate index to
 /// Indices, and setting Ty to the result subtype.
 ///
 /// If no natural GEP can be constructed, this function returns null.
 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
   // an i8.
   if (Ty == IRB.getInt8PtrTy(Ty->getAddressSpace()) && TargetTy->isIntegerTy(8))
     return nullptr;
 
   Type *ElementTy = Ty->getElementType();

 ///
 /// The strategy for finding the more natural GEPs is to peel off layers of the
 /// pointer, walking back through bit casts and GEPs, searching for a base
 /// pointer from which we can compute a natural GEP with the desired
 /// properties. The algorithm tries to fold as many constant indices into
 /// 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;
   Visited.insert(Ptr);
   SmallVector<Value *, 4> Indices;
 
   // We may end up computing an offset pointer that has the wrong type. If we
   // never are able to compute one directly that has the correct type, we'll

llvm/trunk/tools/llvm-nm/llvm-nm.cpp

 
 bool MultipleFiles = false;
 
 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;
   }
   return false;
 }
 
 // This version of error() prints the archive name and member name, for example:

llvm/trunk/tools/llvm-objcopy/Object.h

 private:
   llvm::ArrayRef<std::unique_ptr<SectionBase>> Sections;
 
 public:
   SectionTableRef(llvm::ArrayRef<std::unique_ptr<SectionBase>> Secs)
       : 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 {
 public:
   llvm::StringRef Name;
   Segment *ParentSegment = nullptr;
   uint64_t HeaderOffset;
   uint64_t OriginalOffset;

llvm/trunk/tools/llvm-objcopy/Object.cpp

for (const auto &Rel : RelRange) {
     ToAdd.Offset = Rel.r_offset;
     getAddend(ToAdd.Addend, Rel);
     ToAdd.Type = Rel.getType(false);
     ToAdd.RelocSymbol = SymbolTable->getSymbolByIndex(Rel.getSymbol(false));
     Relocs->addRelocation(ToAdd);
   }
 }
 
-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);
 }
 
 template <class ELFT>
 std::unique_ptr<SectionBase>
 Object<ELFT>::makeSection(const llvm::object::ELFFile<ELFT> &ElfFile,

llvm/trunk/tools/llvm-objcopy/llvm-objcopy.h

 //===- llvm-objcopy.h -------------------------------------------*- C++ -*-===//
 //
 //                      The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_OBJCOPY_H
 #define LLVM_OBJCOPY_H
 
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Error.h"
 
 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)
 template <class T> T unwrapOrError(Expected<T> EO) {
   if (EO)
     return *EO;
   std::string Buf;
   raw_string_ostream OS(Buf);
   logAllUnhandledErrors(EO.takeError(), OS, "");
   OS.flush();
   error(Buf);
 }
 }
 
 #endif

llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp

 using namespace object;
 using namespace ELF;
 
 // The name this program was invoked as.
 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);
 }
 
 LLVM_ATTRIBUTE_NORETURN void reportError(StringRef File, std::error_code EC) {
   assert(EC);
   errs() << ToolName << ": '" << File << "': " << EC.message() << ".\n";

llvm/trunk/unittests/ADT/TwineTest.cpp

TEST(TwineTest, Concat) {
   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, 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());
   EXPECT_EQ(0,
            *Twine(StringRef("hello")).toNullTerminatedStringRef(storage).end());
   EXPECT_EQ(0, *Twine(SmallString<11>("hello"))
                     .toNullTerminatedStringRef(storage)
                     .end());

llvm/trunk/utils/TableGen/RegisterBankEmitter.cpp

 ///
 /// \param RC The register class to search.
 /// \param Kind A debug string containing the path the visitor took to reach RC.
 /// \param VisitFn The action to take for each class visited. It may be called
 ///                multiple times for a given class if there are multiple paths
 ///                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) {
 
   // Make sure we only visit each class once to avoid infinite loops.
   if (VisitedRCs.count(RC))
     return;
   VisitedRCs.insert(RC);
 
   // Visit each explicitly named class.
   VisitFn(RC, Kind.str());
 
   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))
       visitRegisterBankClasses(RegisterClassHierarchy, &PossibleSubclass,
                                TmpKind + " " + RC->getName() + " subclass",
                                VisitFn, VisitedRCs);
 
     // Visit each class that contains only subregisters of RC with a common