Implementation Feature Test Macros for P0722R3

[ckennelly]

Summary

P1353R0, adopted in San Diego, specified an implementation feature test macro for destroying delete (P0722R3).

The implementation of the feature (https://reviews.llvm.org/rL315662) is not guarded behind a flag, so the macro is not conditional on language version.

[rsmith]

Looks good. Is there anything in www/cxx_status.html that we should update to track that we've implemented this part of P1353R0?

[ckennelly]

Added update to cxx_status.html for partial implementation of feature test macros adopted in San Diego

[rsmith]

Thanks! Do you need someone to commit for you?

[ckennelly]

Yes, I need someone to commit for me.

[sberg]

One problem I found with the macro __cpp_impl_destroying_delete not being conditional on language version is the following: Recent GCC trunk (since https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=76b94d4ba654e9af1882865933343d11f5c3b18b "Implement P0722R3, destroying operator delete.") contains

#if __cpp_impl_destroying_delete
#define __cpp_lib_destroying_delete 201806L
namespace std
{
  struct destroying_delete_t
  {
    explicit destroying_delete_t() = default;
  };
  inline constexpr destroying_delete_t destroying_delete{};
}
#endif // destroying delete

at "top-level" (i.e., not in a C++20-only #if or similar) in libstdc++-v3/libsupc++/new. That means that when using Clang against that GCC toolchain, #include <new> in C++03 mode will cause error: unknown type name 'constexpr'.

[rsmith]

@jwakely, we should come to a common understanding of the right behavior here :) (and then probably report back to SG-10).

Clang unconditionally makes sized delete work if you have a suitable library type, and expects the library to decide whether it wants to provide the library type as an extension in earlier language modes. As a result, we currently define the __cpp_impl macro unconditionally, which breaks libstdc++'s approach.

[jwakely]

The simplest solution would be to change libstdc++'s <new> to:

#if __cplusplus >= 201703L && __cpp_impl_destroying_delete
#define __cpp_lib_destroying_delete 201806L

Or we could be fancier and do:

#if __cplusplus >= 201103L && __cpp_impl_destroying_delete
namespace std
{
  struct destroying_delete_t
  {
    explicit destroying_delete_t() = default;
  };
#if __cplusplus >= 201703L
# define __cpp_lib_destroying_delete 201806L
  inline constexpr destroying_delete_t destroying_delete{};
#endif
}
#endif // destroying delete

i.e. define the type for C++11 and later, and the inline variable (and feature-test macro) only for C++17 when inline variables are supported.

[ckennelly]

Conditioning off of C++17 (or part C++11 and part C++17) seems reasonable, although we may want consistency with libc++ (discussion on https://reviews.llvm.org/D55840).

[jwakely]

I'd be happy to restrict this to > C++17 only (which is automatically the case when using G++ because there's no -fdestroying-delete to enable it, you only get it with -std=c++2a, -std=gnu++2a etc.)

[ckennelly]

In D55741#1361040, @jwakely wrote:

I'd be happy to restrict this to > C++17 only (which is automatically the case when using G++ because there's no -fdestroying-delete to enable it, you only get it with -std=c++2a, -std=gnu++2a etc.)

I'd like to see this available earlier (say >=C++17) so that it is possible to pre-adopt the feature before switching to C++2a (assuming appropriate language support for the feature from the compiler). Otherwise, it's necessary to open up std to get to a simple type that happens to have magic language implications.

[jwakely]

As it looks like libc++ isn't going to define the type for earlier dialects, libstdc++ doesn't either (which seems right anyway because the names aren't reserved prior to C++20).

We now define the type and variable unconditionally for C++2a, but only define the __cpp_lib_destroying_delete macro if compiling for C++2a and the compiler advertises support via the impl macro.

Edit: oh, looks like I misread the last comment on https://reviews.llvm.org/D55840 ... but I still think not defining the names pre-C++2a is the right choice. In any case, using libstdc++ headers with a recent clang will no longer complain about using constexpr in C++98.

lib/Frontend/InitPreprocessor.cpp

   // C++20 features.
   if (LangOpts.Char8)
     Builder.defineMacro("__cpp_char8_t", "201811L");
+  Builder.defineMacro("__cpp_impl_destroying_delete", "201806L");
 
   // TS features.
   if (LangOpts.ConceptsTS)

test/Lexer/cxx-features.cpp

 #error "wrong value for __cpp_char8_t"
 #endif
 
+#if check(impl_destroying_delete, 201806, 201806, 201806, 201806, 201806)
+#error "wrong value for __cpp_impl_destroying_delete"
+#endif
+
 // --- C++17 features ---
 
 #if check(hex_float, 0, 0, 0, 201603, 201603)