[libcxx-commits] [PATCH] D89057: Add the C++17 <memory_resource> header (mono-patch)

[Doug Wyatt via Phabricator via libcxx-commits]

dougsonos added inline comments.


================
Comment at: libcxx/include/__memory_resource/polymorphic_allocator.h:146-149
+    template <class _Tp>
+    _LIBCPP_HIDE_FROM_ABI
+    void destroy(_Tp *__p)
+        { __p->~_Tp(); }
----------------
jloser wrote:
> Quuxplusone wrote:
> > >>! @jloser wrote:
> > > @Quuxplusone can you please remove destroy() (and its tests) from polymorphic_allocator? Then you can mark LWG3036 as done and we avoid the issue of whether to mark destroy() as deprecated vs hard removal. Simply not including it when we ship this for the first time is the easiest :)
> > 
> > Sadly, it appears that `destroy` is only deprecated in C++23, not removed, so if we want to be conforming we should still have it (just marked as `[[deprecated]]`).
> > http://eel.is/c++draft/depr#mem.poly.allocator.mem
> > 
> > Or is your point that we could just kinda "forget" to implement `destroy` (the same way we've "forgotten" to implement all of `<memory_resource>` for the past 4 years ;)), and just wait for the Standard to catch up to our non-implementation? @ldionne thoughts?
> Yeah, I was hoping we could skip the deprecated step since `libc++` doesn't have `std::pmr::polymorphic_allocator` right now - it's in the experimental namespace.
By this same logic, libc++ could neglect to implement all the variants of construct() which were added in C++17 and removed in C++20 :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89057