[llvm][ADT] Add deduction guides for `MutableArrayRef`

[jloser]

Summary

Similar to https://reviews.llvm.org/D140896, this adds deduction guides for the
counterpart of ArrayRef: MutableArrayRef. The update plan is the following:

1. Add deduction guides for MutableArrayRef.
2. Change uses in-tree from makeMutableArrayRef to use deduction guides
3. Mark makeMutableArrayRef as deprecated for some time before removing to give downstream users time to update.

The deduction guides are similar to those provided by the makeMutableArrayRef
function templates, except we don't need one explicitly from MutableArrayRef.

[jloser]

Add [llvm] to title

[dblaikie]

Test coverage might be nice - some static asserts about the type that's deduced is probably sufficient (static_assert(std::is_same_v<MutableArrayRef<int, 3>, MutableArrayRef(std::declval<int[3]>())>) or something like that for each deduction guide)?

[jloser]

Add tests for deduction guides and runtime behavior

[jloser]

In D141327#4038061, @dblaikie wrote:

Test coverage might be nice - some static asserts about the type that's deduced is probably sufficient (static_assert(std::is_same_v<MutableArrayRef<int, 3>, MutableArrayRef(std::declval<int[3]>())>) or something like that for each deduction guide)?

I added both compile time tests for each of the deduction guides and runtime tests similar to that of makeMutableArrayRef test above. Let me know what you think. Thanks for the nudge on testing.

[serge-sans-paille]

LGTM

llvm/include/llvm/ADT/ArrayRef.h

602

I don't think this is going to be a n-op : it's going to call the copy constructor.

[jloser]

Remove deduction guide from MutableArrayRef: it's not needed. Add a test from both const and non-const MutableArrayRef to show there's no issues, or ambiguous deduction guides like before.

[jloser]

llvm/include/llvm/ADT/ArrayRef.h

602

Oops, sorry this was a copy-paste from the makeMutableArrayRef function template comments.

Turns out we don't actually need this since a const MutableArrayRef would bind with the first deduction guide and give back a MutableArrayRef<T>. I added an additional test to show we can deduce correctly from both a const and non-const MutableArrayRef input.

If this still looks good to you @serge-sans-paille, I'll land it. Thanks!

[dblaikie]

I'd be OK with the testing just being the static_asserts without the rest of the testing (since that's all the deduction guide do - specifies what type to use, then the usual ctor rules kick in - and those ctors are already tested elsewhere) - but this is OK too, if you prefer it.

[jloser]

In D141327#4040331, @dblaikie wrote:

I'd be OK with the testing just being the static_asserts without the rest of the testing (since that's all the deduction guide do - specifies what type to use, then the usual ctor rules kick in - and those ctors are already tested elsewhere) - but this is OK too, if you prefer it.

I slightly prefer it actually (and it's what we do in libc++ too — both compile-time and runtime tests for testing deduction guides). Here, we plan on eventually removing makeMutableArrayRef (which tests runtime behavior above). When we remove those function templates and their tests, it'd be good to preserve similar behavior in my mind.

llvm/include/llvm/ADT/ArrayRef.h

#endif
 
   /// Construct an ArrayRef from a C array.
   template <typename T, size_t N>
   LLVM_DEPRECATED("Use deduction guide instead", "ArrayRef")
   ArrayRef<T> makeArrayRef(const T (&Arr)[N]) {
     return ArrayRef<T>(Arr);
   }
 
+  /// @name MutableArrayRef Deduction guides
+  /// @{
+  /// Deduction guide to construct a `MutableArrayRef` from a single element
+  template <class T> MutableArrayRef(T &OneElt) -> MutableArrayRef<T>;
+
+  /// Deduction guide to construct a `MutableArrayRef` from a pointer and
+  /// length.
+  template <class T>
+  MutableArrayRef(T *data, size_t length) -> MutableArrayRef<T>;
+
+  /// Deduction guide to construct a `MutableArrayRef` from a `SmallVector`.
+  template <class T>
+  MutableArrayRef(SmallVectorImpl<T> &Vec) -> MutableArrayRef<T>;
+
+  template <class T, unsigned N>
+  MutableArrayRef(SmallVector<T, N> &Vec) -> MutableArrayRef<T>;
+
+  /// Deduction guide to construct a `MutableArrayRef` from a `std::vector`.
+  template <class T> MutableArrayRef(std::vector<T> &Vec) -> MutableArrayRef<T>;
+
+  /// Deduction guide to construct a `MutableArrayRef` from a `std::array`.
+  template <class T, std::size_t N>
+  MutableArrayRef(std::array<T, N> &Vec) -> MutableArrayRef<T>;
+
+  /// Deduction guide to construct a `MutableArrayRef` from a C array.
+  template <typename T, size_t N>
+  MutableArrayRef(T (&Arr)[N]) -> MutableArrayRef<T>;
+
+  /// @}
+
   /// Construct a MutableArrayRef from a single element.
   template<typename T>
   MutableArrayRef<T> makeMutableArrayRef(T &OneElt) {
     return OneElt;
   }
 
   /// Construct a MutableArrayRef from a pointer and length.
   template<typename T>

llvm/unittests/ADT/ArrayRefTest.cpp

TEST(ArrayRefTest, makeMutableArrayRef) {
   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());
 }
 
+TEST(ArrayRefTest, MutableArrayRefDeductionGuides) {
+  // Single element
+  {
+    int x = 0;
+    auto aref = MutableArrayRef(x);
+    static_assert(std::is_same_v<MutableArrayRef<int>, decltype(aref)>);
+    EXPECT_EQ(aref.data(), &x);
+    EXPECT_EQ(aref.size(), 1u);
+
+    // Make sure it's mutable still
+    aref[0] = 1;
+    EXPECT_EQ(x, 1);
+  }
+
+  // Pointer + length
+  {
+    int x[] = {0, 1, 2, 3};
+    auto aref = MutableArrayRef(&x[0], 4);
+    static_assert(std::is_same_v<MutableArrayRef<int>, decltype(aref)>);
+    EXPECT_EQ(aref.data(), &x[0]);
+    EXPECT_EQ(aref.size(), 4u);
+  }
+
+  // // Pointer + pointer
+  {
+    int x[] = {0, 1, 2, 3};
+    auto aref = MutableArrayRef(std::begin(x), std::end(x));
+    static_assert(std::is_same_v<MutableArrayRef<int>, decltype(aref)>);
+    EXPECT_EQ(aref.data(), &x[0]);
+    EXPECT_EQ(aref.size(), 4u);
+  }
+
+  // SmallVector
+  {
+    SmallVector<int> sv1;
+    SmallVectorImpl<int> &sv2 = sv1;
+    sv1.resize(5);
+    auto aref1 = MutableArrayRef(sv1);
+    auto aref2 = MutableArrayRef(sv2);
+    static_assert(std::is_same_v<MutableArrayRef<int>, decltype(aref1)>);
+    static_assert(std::is_same_v<MutableArrayRef<int>, decltype(aref2)>);
+    EXPECT_EQ(aref1.data(), sv1.data());
+    EXPECT_EQ(aref1.size(), sv1.size());
+    EXPECT_EQ(aref2.data(), sv2.data());
+    EXPECT_EQ(aref2.size(), sv2.size());
+  }
+
+  // std::vector
+  {
+    std::vector<int> x(5);
+    auto aref = MutableArrayRef(x);
+    static_assert(std::is_same_v<MutableArrayRef<int>, decltype(aref)>);
+    EXPECT_EQ(aref.data(), x.data());
+    EXPECT_EQ(aref.size(), x.size());
+  }
+
+  // std::array
+  {
+    std::array<int, 5> x{};
+    auto aref = MutableArrayRef(x);
+    static_assert(std::is_same_v<MutableArrayRef<int>, decltype(aref)>);
+    EXPECT_EQ(aref.data(), x.data());
+    EXPECT_EQ(aref.size(), x.size());
+  }
+
+  // MutableArrayRef
+  {
+    MutableArrayRef<int> x{};
+    auto aref = MutableArrayRef(x);
+    static_assert(std::is_same_v<MutableArrayRef<int>, decltype(aref)>);
+    EXPECT_EQ(aref.data(), x.data());
+    EXPECT_EQ(aref.size(), x.size());
+
+    const MutableArrayRef<int> y{};
+    auto aref2 = MutableArrayRef(y);
+    static_assert(std::is_same_v<MutableArrayRef<int>, decltype(aref2)>);
+    EXPECT_EQ(aref2.data(), y.data());
+    EXPECT_EQ(aref2.size(), y.size());
+  }
+
+  // C-style array
+  {
+    int x[] = {0, 1, 2, 3};
+    auto aref = MutableArrayRef(x);
+    static_assert(std::is_same_v<MutableArrayRef<int>, decltype(aref)>);
+    EXPECT_EQ(aref.data(), &x[0]);
+    EXPECT_EQ(aref.size(), 4u);
+  }
+}
+
 } // end anonymous namespace