[libc++][memory_resource] Implements LWG3683. #100775

mordante · 2024-07-26T16:58:11Z

The polymorphic_allocator was added in C++17.
This issue was filed in 2022 so well after C++20. This issue adds an operator==.

Starting with C++20 this adds a compiler generated operator!=. To have the same behaviour in C++17 and C++20 (and later) a manual operator!= is defined in C++17.

Implements

LWG3683 operator== for polymorphic_allocator cannot deduce template argument in common cases

The polymorphic_allocator was added in C++17. This issue was filed in 2022 so well after C++20. This issue adds an operator==. Starting with C++20 this adds a compiler generated operator!=. To have the same behaviour in C++17 and C++20 (and later) a manual operator!= is defined in C++17. Implements - LWG3683 operator== for polymorphic_allocator cannot deduce template argument in common cases

llvmbot · 2024-07-26T16:58:45Z

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

The polymorphic_allocator was added in C++17.
This issue was filed in 2022 so well after C++20. This issue adds an operator==.

Starting with C++20 this adds a compiler generated operator!=. To have the same behaviour in C++17 and C++20 (and later) a manual operator!= is defined in C++17.

Implements

LWG3683 operator== for polymorphic_allocator cannot deduce template argument in common cases

Full diff: https://github.com/llvm/llvm-project/pull/100775.diff

3 Files Affected:

(modified) libcxx/docs/Status/Cxx23Issues.csv (+1-1)
(modified) libcxx/include/__memory_resource/polymorphic_allocator.h (+11)
(added) libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.class.general/equality.pass.cpp (+44)

diff --git a/libcxx/docs/Status/Cxx23Issues.csv b/libcxx/docs/Status/Cxx23Issues.csv
index e1a5aa6dd952a..02fcea1a606d1 100644
--- a/libcxx/docs/Status/Cxx23Issues.csv
+++ b/libcxx/docs/Status/Cxx23Issues.csv
@@ -166,7 +166,7 @@
 "`3670 <https://wg21.link/LWG3670>`__","``Cpp17InputIterators`` don't have integer-class difference types","July 2022","","","|ranges|"
 "`3671 <https://wg21.link/LWG3671>`__","``atomic_fetch_xor`` missing from ``stdatomic.h``","July 2022","",""
 "`3672 <https://wg21.link/LWG3672>`__","``common_iterator::operator->()`` should return by value","July 2022","|Complete|","19.0","|ranges|"
-"`3683 <https://wg21.link/LWG3683>`__","``operator==`` for ``polymorphic_allocator`` cannot deduce template argument in common cases","July 2022","",""
+"`3683 <https://wg21.link/LWG3683>`__","``operator==`` for ``polymorphic_allocator`` cannot deduce template argument in common cases","July 2022","|Complete|","20.0"
 "`3687 <https://wg21.link/LWG3687>`__","``expected<cv void, E>`` move constructor should move","July 2022","|Complete|","16.0"
 "`3692 <https://wg21.link/LWG3692>`__","``zip_view::iterator``'s ``operator<=>`` is overconstrained","July 2022","","","|ranges| |spaceship|"
 "`3701 <https://wg21.link/LWG3701>`__","Make ``formatter<remove_cvref_t<const charT[N]>, charT>`` requirement explicit","July 2022","|Complete|","15.0","|format|"
diff --git a/libcxx/include/__memory_resource/polymorphic_allocator.h b/libcxx/include/__memory_resource/polymorphic_allocator.h
index a71096d3e4784..a1a9e5ae8eee3 100644
--- a/libcxx/include/__memory_resource/polymorphic_allocator.h
+++ b/libcxx/include/__memory_resource/polymorphic_allocator.h
@@ -174,6 +174,17 @@ class _LIBCPP_AVAILABILITY_PMR _LIBCPP_TEMPLATE_VIS polymorphic_allocator {
 
   _LIBCPP_HIDE_FROM_ABI memory_resource* resource() const noexcept { return __res_; }
 
+  friend bool operator==(const polymorphic_allocator& __lhs, const polymorphic_allocator& __rhs) noexcept {
+    return *__lhs.resource() == *__rhs.resource();
+  }
+
+#  if _LIBCPP_STD_VER == 17
+  // This overload is not specified, it was added due to LWG3683.
+  friend bool operator!=(const polymorphic_allocator& __lhs, const polymorphic_allocator& __rhs) noexcept {
+    return *__lhs.resource() != *__rhs.resource();
+  }
+#  endif
+
 private:
   template <class... _Args, size_t... _Is>
   _LIBCPP_HIDE_FROM_ABI tuple<_Args&&...>
diff --git a/libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.class.general/equality.pass.cpp b/libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.class.general/equality.pass.cpp
new file mode 100644
index 0000000000000..3210b9b449a31
--- /dev/null
+++ b/libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.class.general/equality.pass.cpp
@@ -0,0 +1,44 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14
+// TODO: Change to XFAIL once https://github.com/llvm/llvm-project/issues/40340 is fixed
+// UNSUPPORTED: availability-pmr-missing
+
+// <memory_resource>
+
+// template <class T> class polymorphic_allocator
+
+// friend bool operator==(const polymorphic_allocator& a,
+//                        const polymorphic_allocator& b) noexcept
+
+#include <memory_resource>
+#include <cassert>
+#include <vector>
+
+#include "test_macros.h"
+
+int main(int, char**) {
+  std::pmr::unsynchronized_pool_resource a;
+  std::pmr::vector<int> vec(&a);
+
+  assert(vec.get_allocator() == &a);
+  static_assert(noexcept(vec.get_allocator() == &a));
+
+#if _LIBCPP_VERSION || TEST_STD_VER >= 20
+  // LWG3683 added operator== after C++20. In C++20 operator!= is generated by
+  // the compiler. Libc++ adds operator!= in C++17 as an extension.
+  std::pmr::unsynchronized_pool_resource b;
+
+  assert(vec.get_allocator() != &b);
+  static_assert(noexcept(vec.get_allocator() != &b));
+
+#endif // _LIBCPP_VERSION || TEST_STD_VER >= 20
+
+  return 0;
+}

philnik777 · 2024-07-27T07:23:11Z

libcxx/include/__memory_resource/polymorphic_allocator.h

+    return *__lhs.resource() == *__rhs.resource();
+  }
+
+#  if _LIBCPP_STD_VER == 17


Suggested change

# if _LIBCPP_STD_VER == 17

# if _LIBCPP_STD_VER <= 17

I find the current version quite confusing at first glance, since we don't do that usually.

I'm quite sure we used to do that is the past, but indeed it's very uncommon now. However I've no objection to change this.

philnik777 · 2024-07-27T07:24:59Z

.../utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.class.general/equality.pass.cpp

+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14
+// TODO: Change to XFAIL once https://github.com/llvm/llvm-project/issues/40340 is fixed


The linked bug is closed as wontfix.

This was copy pasted from another test. I'll make a separate patch to remove them. (There are 80 occurrences which would make finding the changes in this patch much harder.)

philnik777 · 2024-07-27T07:26:02Z

.../utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.class.general/equality.pass.cpp

+  assert(vec.get_allocator() == &a);
+  static_assert(noexcept(vec.get_allocator() == &a));
+
+#if _LIBCPP_VERSION || TEST_STD_VER >= 20


libstdc++ and the MSVC STL also back-ported this to 17 the same way. Should we just add this test unconditionally?

@philnik777

During the review of llvm#100775 @philnik777 mentioned the referred bug is closed as "Won't fix". Since the bug will never be fixed the TODO will never be addres; remove them instead.

ldionne · 2024-07-30T16:51:14Z

.../utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.class.general/equality.pass.cpp

+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14
+// UNSUPPORTED: availability-pmr-missing


This comment still belongs here I think:

// TODO: Change to XFAIL once https://github.com/llvm/llvm-project/issues/40340 is fixed

Per my comment in https://github.com/llvm/llvm-project/pull/100884/files

Fixed in 2fe08ed

This addresses the post-commit review comment in #100775

This addresses the post-commit review comment in llvm/llvm-project#100775 NOKEYCHECK=True GitOrigin-RevId: 2fe08ed35a968dcfd797cbd31ae56bb0b1447faf

mordante requested a review from a team as a code owner July 26, 2024 16:58

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jul 26, 2024

philnik777 approved these changes Jul 27, 2024

View reviewed changes

Addresses review comments.

5e09567

mordante mentioned this pull request Jul 27, 2024

[NFC][libc++][test] Removes a TODO. #100884

Closed

mordante merged commit 2711618 into llvm:main Jul 28, 2024
58 checks passed

mordante deleted the review/LWG3683 branch July 28, 2024 10:23

ldionne reviewed Jul 30, 2024

View reviewed changes

mordante added a commit that referenced this pull request Aug 4, 2024

[libc++][test] Adds a TODO.

2fe08ed

This addresses the post-commit review comment in #100775

blueboxd pushed a commit to blueboxd/libcxx that referenced this pull request Apr 19, 2025

[libc++][test] Adds a TODO.

245e6db

This addresses the post-commit review comment in llvm/llvm-project#100775 NOKEYCHECK=True GitOrigin-RevId: 2fe08ed35a968dcfd797cbd31ae56bb0b1447faf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++][memory_resource] Implements LWG3683. #100775

[libc++][memory_resource] Implements LWG3683. #100775

Uh oh!

mordante commented Jul 26, 2024

Uh oh!

llvmbot commented Jul 26, 2024

Uh oh!

philnik777 Jul 27, 2024

Uh oh!

mordante Jul 27, 2024

Uh oh!

philnik777 Jul 27, 2024

Uh oh!

mordante Jul 27, 2024

Uh oh!

philnik777 Jul 27, 2024

Uh oh!

Uh oh!

ldionne Jul 30, 2024

Uh oh!

mordante Aug 4, 2024

Uh oh!

Uh oh!

[libc++][memory_resource] Implements LWG3683. #100775

[libc++][memory_resource] Implements LWG3683. #100775

Uh oh!

Conversation

mordante commented Jul 26, 2024

Uh oh!

llvmbot commented Jul 26, 2024

Uh oh!

philnik777 Jul 27, 2024

Choose a reason for hiding this comment

Uh oh!

mordante Jul 27, 2024

Choose a reason for hiding this comment

Uh oh!

philnik777 Jul 27, 2024

Choose a reason for hiding this comment

Uh oh!

mordante Jul 27, 2024

Choose a reason for hiding this comment

Uh oh!

philnik777 Jul 27, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ldionne Jul 30, 2024

Choose a reason for hiding this comment

Uh oh!

mordante Aug 4, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!