[ADT] Delete Twine constructors which accept StringRef rvalues (NFC)

[vsk]

Summary

Twine stores a *pointer to a StringRef*, not a StringRef. Taking the address of a StringRef rvalue could easily give us a dangling pointer.

Adrian ran into this issue in the wild (see r286640). Deleting these constructors was his idea.

llvm/clang are teeming with Twine abuse, so landing this patch right now would break all of our builds. If this patch looks OK, I can start weeding out the issues.

[vsk]

• Upload a diff with context.

[vsk]

Hm, this doesn't seem workable. Deleting these constructors would make it harder to pass strings into functions.

Maybe it would be better to try something like r286139, and just delete Twine &operator=(T &&) when T is a std::string or a StringRef. @jordan_rose wdyt?

[jordan_rose]

I'm inclined to agree with @dblaikie's email comment: assigning a Twine is pretty much never safe anyway, so we should just delete operator= altogether. (I'd love to get rid of the copy-constructor too and just allow moves, but I think it still makes sense to copy-construct when combining a Twine argument with multiple different suffixes.)

[dexonsmith]

Agree with the above.

[vsk]

I won't be able to pick this back up anytime soon. If anyone would like to commandeer this patch, feel free to do so.

include/llvm/ADT/Twine.h

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!");
     }
 
+    /// Do not construct from StringRef rvalues. Twine stores a *pointer to a
+    /// StringRef*, not a StringRef. Taking the address of a StringRef rvalue
+    /// could easily give us a dangling pointer.
+    /*implicit*/ Twine(StringRef &&) = delete;
+    /*implicit*/ Twine(const char *, StringRef &&) = delete;
+    /*implicit*/ Twine(StringRef &&, const char *) = delete;
+
     /// Create a 'null' string, which is an empty string that always
     /// concatenates to form another empty string.
     static Twine createNull() {
       return Twine(NullKind);
     }
 
     /// @}
     /// @name Numeric Conversions
namespace llvm {
 
   /// Additional overload to guarantee simplified codegen; this is equivalent to
   /// concat().
 
   inline Twine operator+(const char *LHS, const StringRef &RHS) {
     return Twine(LHS, RHS);
   }
 
+  inline Twine operator+(const char *, StringRef &&) = delete;
+
   /// 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+(StringRef &&, const char *) = delete;
+
   inline raw_ostream &operator<<(raw_ostream &OS, const Twine &RHS) {
     RHS.print(OS);
     return OS;
   }
 
   /// @}
 }
 
 #endif