Skip to content

Commit

Permalink
Disallow ArrayRef assignment from temporaries.
Browse files Browse the repository at this point in the history
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.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@283798 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
jrose-apple committed Oct 10, 2016
1 parent 779825d commit 31b90a7
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 0 deletions.
16 changes: 16 additions & 0 deletions include/llvm/ADT/ArrayRef.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
/// @{
Expand Down
23 changes: 23 additions & 0 deletions unittests/ADT/ArrayRefTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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};
Expand Down

0 comments on commit 31b90a7

Please sign in to comment.