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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne created this revision.
Herald added a project: All.
ldionne requested review of this revision.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150119

Files:
  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


Index: libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_pair_cxx03.pass.cpp
===================================================================
--- libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_pair_cxx03.pass.cpp
+++ libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_pair_cxx03.pass.cpp
@@ -29,20 +29,37 @@
 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;
Index: libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_const_pair_U_V.pass.cpp
===================================================================
--- libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_const_pair_U_V.pass.cpp
+++ libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_const_pair_U_V.pass.cpp
@@ -74,6 +74,16 @@
     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;
 }
Index: libcxx/include/__utility/pair.h
===================================================================
--- libcxx/include/__utility/pair.h
+++ libcxx/include/__utility/pair.h
@@ -105,6 +105,20 @@
         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&...>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150119.520370.patch
Type: text/x-patch
Size: 3686 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20230508/da94be94/attachment.bin>