Disallow ArrayRef assignment from temporaries

[jordan_rose]

Summary

Without this, the following statements will create ArrayRefs that refer to temporary storage that goes out of scope by the end of the line:

someArrayRef = getSingleElement();
someArrayRef = {elem1, elem2};

Note that the constructor still has this problem:

ArrayRef<Element> someArrayRef = getSingleElement();
ArrayRef<Element> someArrayRef = {elem1, elem2};

but that's a little harder to get rid of because we want to be able to use this in calls:

takesArrayRef(getSingleElement());
takesArrayRef({elem1, elem2});

Part of rdar://problem/16375365.

[jordan_rose]

FWIW, LLVM, Clang, and clang-tools-extra had no issues in this area. Swift had a few. (Unfortunately Swift also has had a few with the constructor involved, so it would be great if we could figure out how to make that safer as well.)

[jordan_rose]

This comment was removed by jordan_rose.

[jordan_rose]

LGTM (with or without the name) if you add std::is_assignable-based static_asserts to unittests/ADT/ArrayRefTest.cpp.

Ooh, I was wondering how to test it. That's a good one.

I'm a little sad but ultimately in favor of making those constructors explicit (or removed altogether with an error message pointing to makeArrayRef), but that's an incredibly breaking change for all clients of LLVM. This one at least should only catch true issues.

[jordan_rose]

Approved by Duncan via email. Committed as r283798.

include/llvm/ADT/ArrayRef.h

public:
     /// @}
     /// @name Operator Overloads
     /// @{
     const T &operator[](size_t Index) const {
       assert(Index < Length && "Invalid index!");
       return Data[Index];
     }
 
+    /// Disallow accidental assignment from a temporary.
+    ///
+    /// The declaration here is extra complicated so that "arrayRef = {}"
+    /// continues to select the move assignment operator.
+    template <typename U>
+    typename std::enable_if<std::is_same<U, T>::value, ArrayRef<T>>::type &
+    operator=(U &&Temporary) = delete;
+
+    /// Disallow accidental assignment from a temporary.
+    ///
+    /// The declaration here is extra complicated so that "arrayRef = {}"
+    /// continues to select the move assignment operator.
+    template <typename U>
+    typename std::enable_if<std::is_same<U, T>::value, ArrayRef<T>>::type &
+    operator=(std::initializer_list<U>) = delete;
+
     /// @}
     /// @name Expensive Operations
     /// @{
     std::vector<T> vec() const {
       return std::vector<T>(Data, Data+Length);
     }
 
     /// @}