[libcxx-commits] [PATCH] D132796: [libc++][PMR] Implement pmr::memory_resource

[Hui via Phabricator via libcxx-commits]

huixie90 requested changes to this revision.
huixie90 added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__memory_resource/memory_resource.h:32
+  memory_resource(const memory_resource&) = default;
+  _LIBCPP_FUNC_VIS virtual ~memory_resource();
+
----------------
Question : can we inline the =default it in the header? Or is there an abi requirement that the vtable must exist in the library?


================
Comment at: libcxx/include/__memory_resource/memory_resource.h:33
+  _LIBCPP_FUNC_VIS virtual ~memory_resource();
+
+  [[using __gnu__: __returns_nonnull__,
----------------
Is `memory_resource& operator=(const memory_resource&) = default;` missing?


================
Comment at: libcxx/include/__memory_resource/memory_resource.h:56
+
+_LIBCPP_HIDE_FROM_ABI inline bool operator==(const memory_resource& __lhs, const memory_resource& __rhs) {
+  return &__lhs == &__rhs || __lhs.is_equal(__rhs);
----------------
missing `noexcept`


================
Comment at: libcxx/include/__memory_resource/memory_resource.h:61
+#  if _LIBCPP_STD_VER == 17
+_LIBCPP_HIDE_FROM_ABI inline bool operator!=(const memory_resource& __lhs, const memory_resource& __rhs) {
+  return !(__lhs == __rhs);
----------------
`noexcept`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132796/new/

https://reviews.llvm.org/D132796