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

[Jonathan Wakely via Phabricator via libcxx-commits]

jwakely added inline comments.


================
Comment at: libcxx/include/__memory_resource/memory_resource.h:57
+_LIBCPP_HIDE_FROM_ABI inline bool operator==(const memory_resource& __lhs, const memory_resource& __rhs) {
+  return &__lhs == &__rhs || __lhs.is_equal(__rhs);
+}
----------------
Mordante wrote:
> 
Why bother to use `std::addressof`? There is no `memory_resource::operator&` and it's not a template, so ADL isn't going to find `operator&` in any user namespaces. Is that a policy? It seems like unnecessary obfuscation.


================
Comment at: libcxx/src/memory_resource.cpp:14
+namespace pmr {
+_LIBCPP_FUNC_VIS memory_resource::~memory_resource() = default;
+} // namespace pmr
----------------
Mordante wrote:
> What is the reason to put the destructor in the dylib?
> 
> When keeping it in the dylib it needs `_AVAILABLITY_MACROS`, grep for `_LIBCPP_AVAILABILITY_FORMAT` for an example.
> What is the reason to put the destructor in the dylib?

It's the key function, so the vtable will be generated where this function is defined. If it's inline, the vtable gets generated in every TU that uses `memory_resource`. Maybe that's OK though.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132796