[PATCH] D55840: P0722R3: Implement library support for destroying delete

[Chris Kennelly via Phabricator]

ckennelly marked an inline comment as done.
ckennelly added a comment.

In D55840#1348416 <https://reviews.llvm.org/D55840#1348416>, @ldionne wrote:

> In D55840#1348304 <https://reviews.llvm.org/D55840#1348304>, @mclow.lists wrote:
>
> > Another question - why do we need to check if clang supports destroying delete to define the type `destroying_delete_t`? Why not define it always, and just define the feature test macro when we have compiler support?
>
>
> I'm not a big fan of this approach. It seems better to define `__cpp_lib_destroying_delete` if and only if we provide `destroying_delete_t`, which makes sense if and only if the compiler has support for it (i.e. `__cpp_impl_destroying_delete` is defined).


P1353 <https://reviews.llvm.org/P1353> describes the `impl` macro as "whether the library feature can be provided," so that implied guarding exposing the library features with that macro as well.



================
Comment at: include/new:169
+
+struct destroying_delete_t {
+  explicit destroying_delete_t() = default;
----------------
ldionne wrote:
> Please guard with > C++17. We should not enable features in older standards unless we have a compelling reason to.
If there's compiler support (we're checking _impl), would it be reasonable to provide it with C++17 as well?


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D55840