[PATCH] D25446: Disallow ArrayRef assignment from temporaries

[Duncan P. N. Exon Smith via llvm-commits]

> On 2016-Oct-10, at 12:35, Jordan Rose <jordan_rose at apple.com> wrote:
> 
> jordan_rose created this revision.
> jordan_rose added reviewers: chandlerc, dexonsmith.
> jordan_rose added a subscriber: llvm-commits.
> jordan_rose set the repository for this revision to rL LLVM.
> 
> Without this, the following statements will create ArrayRefs that refer to temporary storage that goes out of scope by the end of the line:
> 
>  c++
>  someArrayRef = getSingleElement();
>  someArrayRef = {elem1, elem2};
> 
> Note that the constructor still has this problem:
> 
>  c++
>  ArrayRef<Element> someArrayRef = getSingleElement();
>  ArrayRef<Element> someArrayRef = {elem1, elem2};

Long-term, I think these constructors should be explicit.

> but that's a little harder to get rid of because we want to be able to use this in calls:
> 
>  c++
>  takesArrayRef(getSingleElement());
>  takesArrayRef({elem1, elem2});

These would have to change to:

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

but I think that would be a fine tradeoff.

> 
> Part of rdar://problem/16375365.
> 
> 
> Repository:
>  rL LLVM
> 
> https://reviews.llvm.org/D25446
> 
> Files:
>  include/llvm/ADT/ArrayRef.h
> 
> 
> Index: include/llvm/ADT/ArrayRef.h
> ===================================================================
> --- include/llvm/ADT/ArrayRef.h
> +++ include/llvm/ADT/ArrayRef.h
> @@ -219,6 +219,22 @@
>       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;

I'm not sure the parameter name, "Temporary", is worth spelling out.

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

> +
> +    /// 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
>     /// @{
> 
> 
> <D25446.74161.patch>