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

[Jonathan Wakely via Phabricator via libcxx-commits]

jwakely added inline comments.


================
Comment at: include/new:169
+
+struct destroying_delete_t {
+  explicit destroying_delete_t() = default;
----------------
jwakely wrote:
> ldionne wrote:
> > EricWF wrote:
> > > mclow.lists wrote:
> > > > ckennelly wrote:
> > > > > 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?
> > > > Not reasonable, no.
> > > > I did this one, for something that I thought was a slam-dunk (`string_view` in pre-c++17) and it has been nothing but a series of small annoyances.
> > > > 
> > > This is a separate question from `std::string_view`. This is a core-language feature that needs library support. The compiler decides whether this feature is on or off.
> > > 
> > > Like aligned allocation and sized delete, the library should provide `destroying_delete_t` whenever the compiler defines its language feature test macro. Otherwise, Clang can't do its job. This approach allows users to opt-into important new language features as an extension, or to opt-out of them when availability is a concern (this is particularly important for allocation/deallocation functions).
> > > 
> > > We should enable the typedef when either `_LIBCPP_STD_VER > 17` or `defined(__cpp_impl_destroying_delete)`. The latter macro signify that Clang needs us to provide the type because *clang* has chosen to provide this language feature as an extension.
> > I agree, this should be `#if _LIBCPP_STD_VER > 17 || defined(__cpp_impl_destroying_delete)`.
> That will break for C++98 mode, because `constexpr` (and defaulted special members and inline variables) gives an error in C++98. That's the problem I was asked to fix in libstdc++, to be compatible with Clang.
> This approach allows users to opt-into important new language features as an extension, or to opt-out of them when availability is a concern (this is particularly important for allocation/deallocation functions).

If there was a `-fdestroying-delete` option to opt in or out of the compiler feature, I'd agree. But IIUC correctly, clang enables it unconditionally, so there's no opt-out, and no way for users to say "please don't define `destroying_delete` in C++11 mode, I use that name as a macro because I'm annoying like that".


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D55840