Revert "[ADT] Make Twine's copy constructor private."

This reverts commit 4e4ee1c507e2707bb3c208e1e1b6551c3015cbf5.

This is failing due to some code that isn't built on MSVC
so I didn't catch.  Not immediately obvious how to fix this
at first glance, so I'm reverting for now.

llvm-svn: 315536

include/llvm/ADT/Twine.h

@@ -79,10 +79,6 @@ 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
@@ -173,12 +169,6 @@ 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) {
@@ -266,6 +256,8 @@ 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
@@ -282,8 +274,6 @@ 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) {
@@ -387,14 +377,6 @@ 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;
@@ -505,10 +487,6 @@ 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);
-    }
-
     /// @}
   };
 
@@ -544,6 +522,10 @@ 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().
 
@@ -551,11 +533,10 @@ namespace llvm {
     return Twine(LHS, RHS);
   }
 
-  inline Twine operator+(const StringRef &LHS, const char *RHS) {
+  /// Additional overload to guarantee simplified codegen; this is equivalent to
-    return Twine(LHS, RHS);
+  /// concat().
-  }
 
-  inline Twine operator+(const StringRef &LHS, const StringRef &RHS) {
+  inline Twine operator+(const StringRef &LHS, const char *RHS) {
     return Twine(LHS, RHS);
   }
 

include/llvm/IR/DiagnosticInfo.h

@@ -962,7 +962,7 @@ public:
 /// Diagnostic information for unsupported feature in backend.
 class DiagnosticInfoUnsupported : public DiagnosticInfoWithLocationBase {
 private:
-  std::string Msg;
+  Twine 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.str()) {}
+        Msg(Msg) {}
 
   static bool classof(const DiagnosticInfo *DI) {
     return DI->getKind() == DK_Unsupported;
   }
 
-  StringRef getMessage() const { return Msg; }
+  const Twine &getMessage() const { return Msg; }
 
   void print(DiagnosticPrinter &DP) const override;
 };

include/llvm/Object/Error.h

@@ -65,8 +65,8 @@ public:
 class GenericBinaryError : public ErrorInfo<GenericBinaryError, BinaryError> {
 public:
   static char ID;
-  GenericBinaryError(const Twine &Msg);
+  GenericBinaryError(Twine Msg);
-  GenericBinaryError(const Twine &Msg, object_error ECOverride);
+  GenericBinaryError(Twine Msg, object_error ECOverride);
   const std::string &getMessage() const { return Msg; }
   void log(raw_ostream &OS) const override;
 private:

include/llvm/Object/WindowsResource.h

@@ -87,7 +87,7 @@ struct WinResHeaderSuffix {
 
 class EmptyResError : public GenericBinaryError {
 public:
-  EmptyResError(const Twine &Msg, object_error ECOverride)
+  EmptyResError(Twine Msg, object_error ECOverride)
       : GenericBinaryError(Msg, ECOverride) {}
 };
 

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, const Twine &ErrorBanner);
+void logAllUnhandledErrors(Error E, raw_ostream &OS, Twine ErrorBanner);
 
 /// Write all error messages (if any) in E to a string. The newline character
 /// is used to separate error messages.

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) {}

lib/Object/Error.cpp

@@ -60,10 +60,9 @@ std::string _object_error_category::message(int EV) const {
 char BinaryError::ID = 0;
 char GenericBinaryError::ID = 0;
 
-GenericBinaryError::GenericBinaryError(const Twine &Msg) : Msg(Msg.str()) {}
+GenericBinaryError::GenericBinaryError(Twine Msg) : Msg(Msg.str()) {}
 
-GenericBinaryError::GenericBinaryError(const Twine &Msg,
+GenericBinaryError::GenericBinaryError(Twine Msg, object_error ECOverride)
-                                       object_error ECOverride)
     : Msg(Msg.str()) {
   setErrorCode(make_error_code(ECOverride));
 }

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, const Twine &ErrorBanner) {
+void logAllUnhandledErrors(Error E, raw_ostream &OS, Twine ErrorBanner) {
   if (!E)
     return;
   OS << ErrorBanner;

lib/Support/Twine.cpp

@@ -120,10 +120,12 @@ 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 << "\"";

lib/Transforms/Scalar/SROA.cpp

@@ -130,15 +130,18 @@ 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 {
-    const Twine &Prefixed = Prefix + Name;
+    IRBuilderDefaultInserter::InsertHelper(I, getNameWithPrefix(Name), BB,
-    IRBuilderDefaultInserter::InsertHelper(
+                                           InsertPt);
-        I, Name.isTriviallyEmpty() ? Name : Prefixed, BB, InsertPt);
   }
 };
 
@@ -1352,8 +1355,7 @@ 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,
+                       SmallVectorImpl<Value *> &Indices, Twine NamePrefix) {
-                       const Twine &NamePrefix) {
   if (Indices.empty())
     return BasePtr;
 
@@ -1378,7 +1380,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,
-                                    const Twine &NamePrefix) {
+                                    Twine NamePrefix) {
   if (Ty == TargetTy)
     return buildGEP(IRB, BasePtr, Indices, NamePrefix);
 
@@ -1423,7 +1425,7 @@ static Value *getNaturalGEPRecursively(IRBuilderTy &IRB, const DataLayout &DL,
                                        Value *Ptr, Type *Ty, APInt &Offset,
                                        Type *TargetTy,
                                        SmallVectorImpl<Value *> &Indices,
-                                       const Twine &NamePrefix) {
+                                       Twine NamePrefix) {
   if (Offset == 0)
     return getNaturalGEPWithType(IRB, DL, Ptr, Ty, TargetTy, Indices,
                                  NamePrefix);
@@ -1496,7 +1498,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,
-                                      const Twine &NamePrefix) {
+                                      Twine NamePrefix) {
   PointerType *Ty = cast<PointerType>(Ptr->getType());
 
   // Don't consider any GEPs through an i8* as natural unless the TargetTy is
@@ -1534,8 +1536,7 @@ 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,
+                             APInt Offset, Type *PointerTy, Twine NamePrefix) {
-                             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;

tools/llvm-nm/llvm-nm.cpp

@@ -195,12 +195,12 @@ bool HadError = false;
 std::string ToolName;
 } // anonymous namespace
 
-static void error(const Twine &Message, const Twine &Path = Twine()) {
+static void error(Twine Message, Twine Path = Twine()) {
   HadError = true;
   errs() << ToolName << ": " << Path << ": " << Message << ".\n";
 }
 
-static bool error(std::error_code EC, const Twine &Path = Twine()) {
+static bool error(std::error_code EC, Twine Path = Twine()) {
   if (EC) {
     error(EC.message(), Path);
     return true;

tools/llvm-objcopy/Object.cpp

@@ -428,15 +428,15 @@ void initRelocations(RelocationSection<ELFT> *Relocs,
   }
 }
 
-SectionBase *SectionTableRef::getSection(uint16_t Index, const Twine &ErrMsg) {
+SectionBase *SectionTableRef::getSection(uint16_t Index, 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, const Twine &IndexErrMsg,
+T *SectionTableRef::getSectionOfType(uint16_t Index, Twine IndexErrMsg,
-                                     const Twine &TypeErrMsg) {
+                                     Twine TypeErrMsg) {
   if (T *Sec = llvm::dyn_cast<T>(getSection(Index, IndexErrMsg)))
     return Sec;
   error(TypeErrMsg);

tools/llvm-objcopy/Object.h

@@ -29,11 +29,11 @@ public:
       : Sections(Secs) {}
   SectionTableRef(const SectionTableRef &) = default;
 
-  SectionBase *getSection(uint16_t Index, const llvm::Twine &ErrMsg);
+  SectionBase *getSection(uint16_t Index, llvm::Twine ErrMsg);
 
   template <class T>
-  T *getSectionOfType(uint16_t Index, const llvm::Twine &IndexErrMsg,
+  T *getSectionOfType(uint16_t Index, llvm::Twine IndexErrMsg,
-                      const llvm::Twine &TypeErrMsg);
+                      llvm::Twine TypeErrMsg);
 };
 
 class SectionBase {

tools/llvm-objcopy/llvm-objcopy.cpp

@@ -27,7 +27,7 @@ static StringRef ToolName;
 
 namespace llvm {
 
-LLVM_ATTRIBUTE_NORETURN void error(const Twine &Message) {
+LLVM_ATTRIBUTE_NORETURN void error(Twine Message) {
   errs() << ToolName << ": " << Message << ".\n";
   errs().flush();
   exit(1);

tools/llvm-objcopy/llvm-objcopy.h

@@ -14,7 +14,7 @@
 
 namespace llvm {
 
-LLVM_ATTRIBUTE_NORETURN extern void error(const Twine &Message);
+LLVM_ATTRIBUTE_NORETURN extern void error(Twine Message);
 
 // This is taken from llvm-readobj.
 // [see here](llvm/tools/llvm-readobj/llvm-readobj.h:38)

unittests/ADT/TwineTest.cpp

@@ -88,14 +88,6 @@ 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());

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 =
-        (Kind + " (" + PossibleSubclass.getName() + ")").str();
+        (Twine(Kind) + " (" + PossibleSubclass.getName() + ")").str();
 
     // Visit each subclass of an explicitly named class.
     if (RC != &PossibleSubclass && RC->hasSubClass(&PossibleSubclass))