[llvm] r283798 - Disallow ArrayRef assignment from temporaries.

[Zachary Turner via llvm-commits]

Hi Jordan,

I have to revert this as it is causing build failures on MSVC 2015 with the
following messages:

    ArrayRefTest.cpp(38): error C2338: Assigning from single prvalue element
    ArrayRefTest.cpp(41): error C2338: Assigning from single xvalue element
    ArrayRefTest.cpp(47): error C2338: Assigning from an initializer list

Unfortunately I don't have time to dig further at the moment and we're
having bot failures pile up right now so I'm trying to get back to green.

On Mon, Oct 10, 2016 at 2:06 PM Jordan Rose via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: jrose
> Date: Mon Oct 10 15:57:33 2016
> New Revision: 283798
>
> URL: http://llvm.org/viewvc/llvm-project?rev=283798&view=rev
> Log:
> Disallow ArrayRef assignment from temporaries.
>
> 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. Reviewed by Duncan Exon Smith.
>
> Modified:
>     llvm/trunk/include/llvm/ADT/ArrayRef.h
>     llvm/trunk/unittests/ADT/ArrayRefTest.cpp
>
> Modified: llvm/trunk/include/llvm/ADT/ArrayRef.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ArrayRef.h?rev=283798&r1=283797&r2=283798&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/ArrayRef.h (original)
> +++ llvm/trunk/include/llvm/ADT/ArrayRef.h Mon Oct 10 15:57:33 2016
> @@ -219,6 +219,22 @@ namespace llvm {
>        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
>      /// @{
>
> Modified: llvm/trunk/unittests/ADT/ArrayRefTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/ArrayRefTest.cpp?rev=283798&r1=283797&r2=283798&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/ADT/ArrayRefTest.cpp (original)
> +++ llvm/trunk/unittests/ADT/ArrayRefTest.cpp Mon Oct 10 15:57:33 2016
> @@ -31,6 +31,21 @@ static_assert(
>      !std::is_convertible<ArrayRef<volatile int *>, ArrayRef<int
> *>>::value,
>      "Removing volatile");
>
> +// Check that we can't accidentally assign a temporary location to an
> ArrayRef.
> +// (Unfortunately we can't make use of the same thing with constructors.)
> +static_assert(
> +    !std::is_assignable<ArrayRef<int *>, int *>::value,
> +    "Assigning from single prvalue element");
> +static_assert(
> +    !std::is_assignable<ArrayRef<int *>, int * &&>::value,
> +    "Assigning from single xvalue element");
> +static_assert(
> +    std::is_assignable<ArrayRef<int *>, int * &>::value,
> +    "Assigning from single lvalue element");
> +static_assert(
> +    !std::is_assignable<ArrayRef<int *>, std::initializer_list<int
> *>>::value,
> +    "Assigning from an initializer list");
> +
>  namespace {
>
>  TEST(ArrayRefTest, AllocatorCopy) {
> @@ -161,6 +176,14 @@ TEST(ArrayRefTest, InitializerList) {
>    ArgTest12({1, 2});
>  }
>
> +TEST(ArrayRefTest, EmptyInitializerList) {
> +  ArrayRef<int> A = {};
> +  EXPECT_TRUE(A.empty());
> +
> +  A = {};
> +  EXPECT_TRUE(A.empty());
> +}
> +
>  // Test that makeArrayRef works on ArrayRef (no-op)
>  TEST(ArrayRefTest, makeArrayRef) {
>    static const int A1[] = {1, 2, 3, 4, 5, 6, 7, 8};
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161010/ba18466c/attachment-0001.html>