ADT/ArrayRef: Add makeMutableArrayRef overloads

[nhaehnle]

Summary

Equivalent overloads already exist for makeArrayRef.

[efriedma]

llvm/include/llvm/ADT/ArrayRef.h

549

Missing doc comment?

llvm/unittests/ADT/MutableArrayRefTest.cpp

18 ↗

(On Diff #432120)

This comment doesn't really make sense.

[MaskRay]

llvm/unittests/ADT/MutableArrayRefTest.cpp

19 ↗

(On Diff #432120)

You may reuse ArrayRefTest.cpp since we use ArrayRef.h instead of MutableArrayRef.h.

There should be a test checking that the element is mutable.

[nhaehnle]

Address reviews:

• fix comments
• moved to ArrayRefTest.cpp
• add a check that the mutable array ref elements can be assigned to

[MaskRay]

llvm/include/llvm/ADT/ArrayRef.h

549

Adding a comment for each makeMutableArrayRef overload doesn't add much value to me. The argument types are obvious.

I think having one comment covering all the overloads suffices.

[nhaehnle]

Ping -- are there any substantial comments left? Based on previous feedback this is probably good to go with the changes I made last week?

[MaskRay]

There are two comments which are "Not Done". You need to address them and click "Done".

[nhaehnle]

llvm/include/llvm/ADT/ArrayRef.h

549

This just feels like a matter of taste. The patch as-is follows the existing convention in the file.

[nhaehnle]

Ping

[kuhar]

LGTM

llvm/include/llvm/ADT/ArrayRef.h

#endif
   }
 
   /// Construct a MutableArrayRef from a pointer and length.
   template<typename T>
   MutableArrayRef<T> makeMutableArrayRef(T *data, size_t length) {
     return MutableArrayRef<T>(data, length);
   }
 
+  /// Construct a MutableArrayRef from a SmallVector.
+  template <typename T>
+  MutableArrayRef<T> makeMutableArrayRef(SmallVectorImpl<T> &Vec) {
+    return Vec;
+  }
+
+  /// Construct a MutableArrayRef from a SmallVector.
+  template <typename T, unsigned N>
+  MutableArrayRef<T> makeMutableArrayRef(SmallVector<T, N> &Vec) {
+    return Vec;
+  }
+
+  /// Construct a MutableArrayRef from a std::vector.
+  template<typename T>
+  MutableArrayRef<T> makeMutableArrayRef(std::vector<T> &Vec) {
+    return Vec;
+  }
+
+  /// Construct a MutableArrayRef from a std::array.
+  template <typename T, std::size_t N>
+  MutableArrayRef<T> makeMutableArrayRef(std::array<T, N> &Arr) {
+    return Arr;
+  }
+
+  /// Construct a MutableArrayRef from a MutableArrayRef (no-op) (const)
+  template <typename T>
+  MutableArrayRef<T> makeMutableArrayRef(const MutableArrayRef<T> &Vec) {
+    return Vec;
+  }
+
+  /// Construct a MutableArrayRef from a C array.
+  template<typename T, size_t N>
+  MutableArrayRef<T> makeMutableArrayRef(T (&Arr)[N]) {
+    return MutableArrayRef<T>(Arr);
+  }
+
   /// @}
   /// @name ArrayRef Comparison Operators
   /// @{
 
   template<typename T>
   inline bool operator==(ArrayRef<T> LHS, ArrayRef<T> RHS) {
     return LHS.equals(RHS);
   }

llvm/unittests/ADT/ArrayRefTest.cpp

 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};
 
   // No copy expected for non-const ArrayRef (true no-op)
   ArrayRef<int> AR1(A1);
   ArrayRef<int> &AR1Ref = makeArrayRef(AR1);
   EXPECT_EQ(&AR1, &AR1Ref);
 
TEST(ArrayRefTest, makeArrayRefFromStdArray) {
   for (std::size_t i = 0; i < A1.size(); ++i) {
     EXPECT_EQ(A1[i], A2[i]);
   }
 }
 
 static_assert(std::is_trivially_copyable<ArrayRef<int>>::value,
               "trivially copyable");
 
+TEST(ArrayRefTest, makeMutableArrayRef) {
+  int A = 0;
+  auto AR = makeMutableArrayRef(A);
+  EXPECT_EQ(AR.data(), &A);
+  EXPECT_EQ(AR.size(), (size_t)1);
+
+  AR[0] = 1;
+  EXPECT_EQ(A, 1);
+
+  int B[] = {0, 1, 2, 3};
+  auto BR1 = makeMutableArrayRef(&B[0], 4);
+  auto BR2 = makeMutableArrayRef(B);
+  EXPECT_EQ(BR1.data(), &B[0]);
+  EXPECT_EQ(BR1.size(), (size_t)4);
+  EXPECT_EQ(BR2.data(), &B[0]);
+  EXPECT_EQ(BR2.size(), (size_t)4);
+
+  SmallVector<int> C1;
+  SmallVectorImpl<int> &C2 = C1;
+  C1.resize(5);
+  auto CR1 = makeMutableArrayRef(C1);
+  auto CR2 = makeMutableArrayRef(C2);
+  EXPECT_EQ(CR1.data(), C1.data());
+  EXPECT_EQ(CR1.size(), C1.size());
+  EXPECT_EQ(CR2.data(), C2.data());
+  EXPECT_EQ(CR2.size(), C2.size());
+
+  std::vector<int> D;
+  D.resize(5);
+  auto DR = makeMutableArrayRef(D);
+  EXPECT_EQ(DR.data(), D.data());
+  EXPECT_EQ(DR.size(), D.size());
+
+  std::array<int, 5> E;
+  auto ER = makeMutableArrayRef(E);
+  EXPECT_EQ(ER.data(), E.data());
+  EXPECT_EQ(ER.size(), E.size());
+}
+
 } // end anonymous namespace