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

[David Blaikie via llvm-commits]

Does this not break obvious/correct code like

StringRef src();
void sink(Twine);
...
sink(src());

Why not?

On Fri, Nov 11, 2016 at 3:43 PM Vedant Kumar <vsk at apple.com> wrote:

> vsk created this revision.
> vsk added reviewers: aprantl, dblaikie.
> vsk added a subscriber: llvm-commits.
>
> 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.
>
>
> https://reviews.llvm.org/D26568
>
> Files:
>   include/llvm/ADT/Twine.h
>
>
> Index: include/llvm/ADT/Twine.h
> ===================================================================
> --- include/llvm/ADT/Twine.h
> +++ include/llvm/ADT/Twine.h
> @@ -370,6 +370,13 @@
>        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() {
> @@ -522,6 +529,8 @@
>      return Twine(LHS, RHS);
>    }
>
> +  inline Twine operator+(const char *, StringRef &&) = delete;
> +
>    /// Additional overload to guarantee simplified codegen; this is
> equivalent to
>    /// concat().
>
> @@ -529,6 +538,8 @@
>      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;
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161111/b9174df8/attachment.html>