[libcxx-commits] [libcxx] 0acafcd - [libc++] Provide an assignment operator from pair<U, V> in C++03

[Louis Dionne via libcxx-commits]

Author: Louis Dionne
Date: 2023-05-09T08:35:16-04:00
New Revision: 0acafcd8043412b028be89ce68ce301ce329d3eb

URL: https://github.com/llvm/llvm-project/commit/0acafcd8043412b028be89ce68ce301ce329d3eb
DIFF: https://github.com/llvm/llvm-project/commit/0acafcd8043412b028be89ce68ce301ce329d3eb.diff

LOG: [libc++] Provide an assignment operator from pair<U, V> in C++03

This adds an extension to std::pair in C++03 mode where we provide an
assignment operator from a pair<U, V>. Previously, any code trying to
trigger such an assignment operator would have tried using the
`operator=(pair const&)` copy assignment operator, which would then
have tried creating a `pair const&` by using the unconstrained
pair<U, V> constructor.

After this patch, pair will instead go through operator= directly if
its member types are assignable. If they are not assignable, the extension
operator= is disabled with SFINAE and the pair(pair<U, V>) constructor
will be used. Since that constructor is unconstrained, that will either
work or give a hard error.

This should be pretty transparent to users, but this is technically a
behavior change in C++03. Indeed, if a type has both a valid cross-type
assignment operator *and* a valid cross-type constructor, the library
will now prefer the cross-type assignment instead of going through the
cross-type constructor and then using the copy-constructor. Since this
is the mandated behavior in C++11, however, one could argue that any user
broken by that needs to change their code.

The motivation for this change is to allow simplifying the definition
of std::map's value_type, which requires handling assignment to a pair
of references properly. This patch will allow removing complexity from
https://llvm.org/D121485 instead of adding complexity in that patch.

Differential Revision: https://reviews.llvm.org/D150119

Added: 
    

Modified: 
    libcxx/include/__utility/pair.h
    libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_const_pair_U_V.pass.cpp
    libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_pair_cxx03.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__utility/pair.h b/libcxx/include/__utility/pair.h
index 0d009061575b3..65a4cc078023d 100644
--- a/libcxx/include/__utility/pair.h
+++ b/libcxx/include/__utility/pair.h
@@ -105,6 +105,20 @@ struct _LIBCPP_TEMPLATE_VIS pair
         second = __p.second;
         return *this;
     }
+
+    // Extension: This is provided in C++03 because it allows properly handling the
+    //            assignment to a pair containing references, which would be a hard
+    //            error otherwise.
+    template <class _U1, class _U2, class = __enable_if_t<
+        is_assignable<first_type&, _U1 const&>::value &&
+        is_assignable<second_type&, _U2 const&>::value
+    > >
+    _LIBCPP_HIDE_FROM_ABI
+    pair& operator=(pair<_U1, _U2> const& __p) {
+        first = __p.first;
+        second = __p.second;
+        return *this;
+    }
 #else
     struct _CheckArgs {
       template <int&...>

diff  --git a/libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_const_pair_U_V.pass.cpp b/libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_const_pair_U_V.pass.cpp
index 2df3372493b3a..d7303e556042a 100644
--- a/libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_const_pair_U_V.pass.cpp
+++ b/libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_const_pair_U_V.pass.cpp
@@ -74,6 +74,16 @@ TEST_CONSTEXPR_CXX20 bool test() {
     static_assert(!std::is_assignable<T&, P&>::value, "");
     static_assert(!std::is_assignable<P&, T&>::value, "");
   }
+#endif
+#if TEST_STD_VER >= 11 || defined(_LIBCPP_VERSION) // valid in C++11, provided in C++03 with libc++ as an extension
+  {
+    int i = 0, j = 0;
+    std::pair<int&, int&> p(i, j);
+    const std::pair<const int, const int> from(11, 12);
+    p = from;
+    assert(i == 11);
+    assert(j == 12);
+  }
 #endif
   return true;
 }

diff  --git a/libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_pair_cxx03.pass.cpp b/libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_pair_cxx03.pass.cpp
index 4c076c869794b..2444131b02a0c 100644
--- a/libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_pair_cxx03.pass.cpp
+++ b/libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_pair_cxx03.pass.cpp
@@ -29,20 +29,37 @@ struct NonAssignable {
 struct Incomplete;
 extern Incomplete inc_obj;
 
+struct ConstructibleFromInt {
+  ConstructibleFromInt() : value(-1) { }
+  explicit ConstructibleFromInt(int v) : value(v) { }
+  int value;
+};
+
 int main(int, char**)
 {
     {
-    // Test that we don't constrain the assignment operator in C++03 mode.
-    // Since we don't have access control SFINAE having pair evaluate SFINAE
-    // may cause a hard error.
-    typedef std::pair<int, NonAssignable> P;
-    static_assert(std::is_copy_assignable<P>::value, "");
+      // Test that we don't constrain the assignment operator in C++03 mode.
+      // Since we don't have access control SFINAE having pair evaluate SFINAE
+      // may cause a hard error.
+      typedef std::pair<int, NonAssignable> P;
+      static_assert(std::is_copy_assignable<P>::value, "");
+    }
+    {
+      typedef std::pair<int, Incomplete&> P;
+      static_assert(std::is_copy_assignable<P>::value, "");
+      P p(42, inc_obj);
+      assert(&p.second == &inc_obj);
     }
     {
-    typedef std::pair<int, Incomplete&> P;
-    static_assert(std::is_copy_assignable<P>::value, "");
-    P p(42, inc_obj);
-    assert(&p.second == &inc_obj);
+      // The type is constructible from int, but not assignable from int.
+      // This ensures that operator=(pair const&) can be used in conjunction with
+      // pair(pair<U, V> const&) to mimic operator=(pair<U, V> const&) in C++03.
+      // This is weird but valid in C++03.
+      std::pair<ConstructibleFromInt, char> p;
+      std::pair<int, char> from(11, 'x');
+      p = from;
+      assert(p.first.value == 11);
+      assert(p.second == 'x');
     }
 
   return 0;