[C++2a] Fix PR40576: Turn destroying delete off prior to C++2a. Add -fdestroying-delete

[EricWF]

Summary

Defining new names in namespace std is technically a non-conforming extensions. This makes it difficult for libc++ to backport C++20 destroying delete.
This patch should make it easier for libc++ to support destroying delete as an extension.

Currently Clang unconditionally defines __cpp_impl_destroying_delete. Unlike aligned allocation or sized deallocation which can be turned on or off by user.
I believe we should make destroying delete act similarly. It should be off prior to C++20 and we should provide flags for enabling destroying delete prior to C++20.
This patch does just that.

There are some weird corner cases though. What should Clang do when it sees a destroying delete overload but LangOpts.DestroyingDelete isn't enabled?
My current approach is to tolerate it's existence, but ignore it for the purposes of overload resolution (after emitting a warning).

With this patch, if Clang see
When a destroying delete function is found

[riccibruno]

lib/AST/DeclCXX.cpp

2070

Just use the AST context you have above ?

[EricWF]

This is the wrong approach. Abandoning.

include/clang/Basic/DiagnosticSemaKinds.td

   "parameter of %0 cannot have a default argument">;
 def err_operator_delete_dependent_param_type : Error<
   "%0 cannot take a dependent type as first parameter; use %1 instead">;
 def err_operator_delete_param_type : Error<
   "first parameter of %0 must have type %1">;
 def err_destroying_operator_delete_not_usual : Error<
   "destroying operator delete can have only an optional size and optional "
   "alignment parameter">;
+def warn_destroying_operator_delete_ignored : Warning<
+  "destroying operator delete is ignored prior to C++2a">, InGroup<CXXPre2aCompat>;
+def note_enable_destroying_delete_extension : Note<
+  "use -fdestroying-delete to enable destroying operator delete">;
 def note_implicit_delete_this_in_destructor_here : Note<
   "while checking implicit 'delete this' for virtual destructor">;
 def err_builtin_operator_new_delete_not_usual : Error<
   "call to '%select{__builtin_operator_new|__builtin_operator_delete}0' "
   "selects non-usual %select{allocation|deallocation}0 function">;
 def note_non_usual_function_declared_here : Note<
   "non-usual %0 declared here">;
 

include/clang/Basic/LangOptions.def

 LANGOPT(CUDAHostDeviceConstexpr, 1, 1, "treating unattributed constexpr functions as __host__ __device__")
 LANGOPT(CUDADeviceApproxTranscendentals, 1, 0, "using approximate transcendental functions")
 LANGOPT(GPURelocatableDeviceCode, 1, 0, "generate relocatable device code")
 
 LANGOPT(SizedDeallocation , 1, 0, "sized deallocation")
 LANGOPT(AlignedAllocation , 1, 0, "aligned allocation")
 LANGOPT(AlignedAllocationUnavailable, 1, 0, "aligned allocation functions are unavailable")
 LANGOPT(NewAlignOverride  , 32, 0, "maximum alignment guaranteed by '::operator new(size_t)'")
+LANGOPT(DestroyingDelete  , 1, 0, "destroying delete")
 LANGOPT(ConceptsTS , 1, 0, "enable C++ Extensions for Concepts")
 BENIGN_LANGOPT(ModulesCodegen , 1, 0, "Modules code generation")
 BENIGN_LANGOPT(ModulesDebugInfo , 1, 0, "Modules debug info")
 BENIGN_LANGOPT(ElideConstructors , 1, 1, "C++ copy constructor elision")
 BENIGN_LANGOPT(DumpRecordLayouts , 1, 0, "dumping the layout of IRgen'd records")
 BENIGN_LANGOPT(DumpRecordLayoutsSimple , 1, 0, "dumping the layout of IRgen'd records in a simple form")
 BENIGN_LANGOPT(DumpVTableLayouts , 1, 0, "dumping the layouts of emitted vtables")
 LANGOPT(NoConstantCFStrings , 1, 0, "no constant CoreFoundation strings")

include/clang/Driver/Options.td

   Group<f_Group>, Flags<[CC1Option]>;
 def fnew_alignment_EQ : Joined<["-"], "fnew-alignment=">,
   HelpText<"Specifies the largest alignment guaranteed by '::operator new(size_t)'">,
   MetaVarName<"<align>">, Group<f_Group>, Flags<[CC1Option]>;
 def : Separate<["-"], "fnew-alignment">, Alias<fnew_alignment_EQ>;
 def : Flag<["-"], "faligned-new">, Alias<faligned_allocation>;
 def : Flag<["-"], "fno-aligned-new">, Alias<fno_aligned_allocation>;
 def faligned_new_EQ : Joined<["-"], "faligned-new=">;
+def fdestroying_delete : Flag<["-"], "fdestroying-delete">, Flags<[DriverOption, CC1Option]>,
+  HelpText<"Enable C++20 destroying delete">, Group<f_Group>;
 
 def fobjc_legacy_dispatch : Flag<["-"], "fobjc-legacy-dispatch">, Group<f_Group>;
 def fobjc_new_property : Flag<["-"], "fobjc-new-property">, Group<clang_ignored_f_Group>;
 def fobjc_infer_related_result_type : Flag<["-"], "fobjc-infer-related-result-type">,
                                       Group<f_Group>;
 def fno_objc_infer_related_result_type : Flag<["-"],
   "fno-objc-infer-related-result-type">, Group<f_Group>,
   HelpText<

lib/AST/DeclCXX.cpp

   // C++ [basic.stc.dynamic.deallocation]p2:
   //   If a class T has a member deallocation function named operator delete
   //   with exactly one parameter, then that function is a usual (non-placement)
   //   deallocation function. [...]
   if (getNumParams() == 1)
     return true;
   unsigned UsualParams = 1;
 
+  ASTContext &Context = getASTContext();
+
   // C++ P0722:
   //   A destroying operator delete is a usual deallocation function if
   //   removing the std::destroying_delete_t parameter and changing the
   //   first parameter type from T* to void* results in the signature of
   //   a usual deallocation function.
-  if (isDestroyingOperatorDelete())
+  if (isDestroyingOperatorDelete()) {
+    if (!getASTContext().getLangOpts().DestroyingDelete)
+      return false;
     ++UsualParams;
+  }
 
   // C++ <=14 [basic.stc.dynamic.deallocation]p2:
   //   [...] If class T does not declare such an operator delete but does
   //   declare a member deallocation function named operator delete with
   //   exactly two parameters, the second of which has type std::size_t (18.1),
   //   then this function is a usual deallocation function.
   //
   // C++17 says a usual deallocation function is one with the signature
   //   (void* [, size_t] [, std::align_val_t] [, ...])
   // and all such functions are usual deallocation functions. It's not clear
   // that allowing varargs functions was intentional.
-  ASTContext &Context = getASTContext();
   if (UsualParams < getNumParams() &&
       Context.hasSameUnqualifiedType(getParamDecl(UsualParams)->getType(),
                                      Context.getSizeType()))
     ++UsualParams;
 
   if (UsualParams < getNumParams() &&
       getParamDecl(UsualParams)->getType()->isAlignValT())
     ++UsualParams;
 
   if (UsualParams != getNumParams())
     return false;
 
   // In C++17 onwards, all potential usual deallocation functions are actual
   // usual deallocation functions. Honor this behavior when post-C++14
   // deallocation functions are offered as extensions too.
   // FIXME(EricWF): Destrying Delete should be a language option. How do we
   // handle when destroying delete is used prior to C++17?
   if (Context.getLangOpts().CPlusPlus17 ||
       Context.getLangOpts().AlignedAllocation ||
-      isDestroyingOperatorDelete())
+      Context.getLangOpts().DestroyingDelete)
     return true;
 
   // This function is a usual deallocation function if there are no
   // single-parameter deallocation functions of the same kind.
   DeclContext::lookup_result R = getDeclContext()->lookup(getDeclName());
   bool Result = true;
   for (const auto *D : R) {
     if (const auto *FD = dyn_cast<FunctionDecl>(D)) {

lib/Driver/ToolChains/Clang.cpp

 
   // The default new alignment can be specified using a dedicated option or via
   // a GCC-compatible option that also turns on aligned allocation.
   if (Arg *A = Args.getLastArg(options::OPT_fnew_alignment_EQ,
                                options::OPT_faligned_new_EQ))
     CmdArgs.push_back(
         Args.MakeArgString(Twine("-fnew-alignment=") + A->getValue()));
 
+  // -fdestroying-delete is on by default in C++2a onwards and otherwise
+  // it's off by default.
+  Args.AddAllArgs(CmdArgs, options::OPT_fdestroying_delete);
+
   // -fconstant-cfstrings is default, and may be subject to argument translation
   // on Darwin.
   if (!Args.hasFlag(options::OPT_fconstant_cfstrings,
                     options::OPT_fno_constant_cfstrings) ||
       !Args.hasFlag(options::OPT_mconstant_cfstrings,
                     options::OPT_mno_constant_cfstrings))
     CmdArgs.push_back("-fno-constant-cfstrings");
 

lib/Frontend/CompilerInvocation.cpp

 
   // C++ has wchar_t keyword.
   Opts.WChar = Opts.CPlusPlus;
 
   Opts.GNUKeywords = Opts.GNUMode;
   Opts.CXXOperatorNames = Opts.CPlusPlus;
 
   Opts.AlignedAllocation = Opts.CPlusPlus17;
+  Opts.DestroyingDelete = Opts.CPlusPlus2a;
 
   Opts.DollarIdents = !Opts.AsmPreprocessor;
 }
 
 /// Attempt to parse a visibility value out of the given argument.
 static Visibility parseVisibility(Arg *arg, ArgList &args,
                                   DiagnosticsEngine &diags) {
   StringRef value = arg->getValue();

   Opts.NewAlignOverride =
       getLastArgIntValue(Args, OPT_fnew_alignment_EQ, 0, Diags);
   if (Opts.NewAlignOverride && !llvm::isPowerOf2_32(Opts.NewAlignOverride)) {
     Arg *A = Args.getLastArg(OPT_fnew_alignment_EQ);
     Diags.Report(diag::err_fe_invalid_alignment) << A->getAsString(Args)
                                                  << A->getValue();
     Opts.NewAlignOverride = 0;
   }
+  Opts.DestroyingDelete =
+      Opts.DestroyingDelete || Args.hasArg(OPT_fdestroying_delete);
   Opts.ConceptsTS = Args.hasArg(OPT_fconcepts_ts);
   Opts.HeinousExtensions = Args.hasArg(OPT_fheinous_gnu_extensions);
   Opts.AccessControl = !Args.hasArg(OPT_fno_access_control);
   Opts.ElideConstructors = !Args.hasArg(OPT_fno_elide_constructors);
   Opts.MathErrno = !Opts.OpenCL && Args.hasArg(OPT_fmath_errno);
   Opts.InstantiationDepth =
       getLastArgIntValue(Args, OPT_ftemplate_depth, 1024, Diags);
   Opts.ArrowDepth =

lib/Frontend/InitPreprocessor.cpp

static void InitializeCPlusPlusFeatureTestMacros(const LangOptions &LangOpts,
   if (LangOpts.AlignedAllocation && !LangOpts.AlignedAllocationUnavailable)
     Builder.defineMacro("__cpp_aligned_new", "201606L");
   if (LangOpts.RelaxedTemplateTemplateArgs)
     Builder.defineMacro("__cpp_template_template_args", "201611L");
 
   // C++20 features.
   if (LangOpts.Char8)
     Builder.defineMacro("__cpp_char8_t", "201811L");
+  if (LangOpts.DestroyingDelete)
     Builder.defineMacro("__cpp_impl_destroying_delete", "201806L");
 
   // TS features.
   if (LangOpts.ConceptsTS)
     Builder.defineMacro("__cpp_experimental_concepts", "1L");
   if (LangOpts.CoroutinesTS)
     Builder.defineMacro("__cpp_coroutines", "201703L");
 }
 

lib/Sema/SemaDeclCXX.cpp

   // C++ [basic.stc.dynamic.deallocation]p2:
   //   Each deallocation function shall return void
   if (CheckOperatorNewDeleteTypes(
           SemaRef, FnDecl, SemaRef.Context.VoidTy, ExpectedFirstParamType,
           diag::err_operator_delete_dependent_param_type,
           diag::err_operator_delete_param_type))
     return true;
 
-  // C++ P0722:
-  //   A destroying operator delete shall be a usual deallocation function.
-  if (MD && !MD->getParent()->isDependentContext() &&
-      MD->isDestroyingOperatorDelete() &&
-      !SemaRef.isUsualDeallocationFunction(MD)) {
-    SemaRef.Diag(MD->getLocation(),
-                 diag::err_destroying_operator_delete_not_usual);
-    return true;
+  if (MD && MD->isDestroyingOperatorDelete()) {
+    // C++ P0722:
+    //   A destroying operator delete shall be a usual deallocation function.
+    if (SemaRef.getLangOpts().DestroyingDelete &&
+        !MD->getParent()->isDependentContext() &&
+        !SemaRef.isUsualDeallocationFunction(MD)) {
+      SemaRef.Diag(MD->getLocation(),
+                   diag::err_destroying_operator_delete_not_usual);
+      return true;
+    }
+    if (!SemaRef.LangOpts.DestroyingDelete) {
+      SemaRef.Diag(MD->getLocation(),
+                   diag::warn_destroying_operator_delete_ignored)
+          << MD->getParamDecl(1)->getSourceRange();
+      SemaRef.Diag(MD->getLocation(),
+                   diag::note_enable_destroying_delete_extension);
+    }
   }
 
   return false;
 }
 
 /// CheckOverloadedOperatorDeclaration - Check whether the declaration
 /// of this overloaded operator is well-formed. If so, returns false;
 /// otherwise, emits appropriate diagnostics and returns true.

lib/Sema/SemaExprCXX.cpp

     UsualDeallocFnInfo(Sema &S, DeclAccessPair Found)
         : Found(Found), FD(dyn_cast<FunctionDecl>(Found->getUnderlyingDecl())),
           Destroying(false), HasSizeT(false), HasAlignValT(false),
           CUDAPref(Sema::CFP_Native) {
       // A function template declaration is never a usual deallocation function.
       if (!FD)
         return;
       unsigned NumBaseParams = 1;
-      if (FD->isDestroyingOperatorDelete()) {
+      if (FD->isDestroyingOperatorDelete() && S.LangOpts.DestroyingDelete) {
         Destroying = true;
         ++NumBaseParams;
       }
 
       if (NumBaseParams < FD->getNumParams() &&
           S.Context.hasSameUnqualifiedType(
               FD->getParamDecl(NumBaseParams)->getType(),
               S.Context.getSizeType())) {

test/CodeGenCXX/cxx2a-destroying-delete.cpp

 // RUN: %clang_cc1 -std=c++2a -emit-llvm %s -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ITANIUM
+// RUN: %clang_cc1 -std=c++11 -faligned-allocation -fdestroying-delete -emit-llvm %s -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ITANIUM
 // RUN: %clang_cc1 -std=c++2a -emit-llvm %s -triple x86_64-windows -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-MSABI
 
 namespace std {
   using size_t = decltype(sizeof(0));
   enum class align_val_t : size_t;
   struct destroying_delete_t {};
 }
 

test/SemaCXX/cxx2a-destroying-delete.cpp

 // RUN: %clang_cc1 -std=c++2a -fexceptions -verify %s
 // RUN: %clang_cc1 -std=c++2a  -verify %s
+// RUN: %clang_cc1 -std=c++17 -fexceptions -fdestroying-delete -verify %s
 
 namespace std {
   using size_t = decltype(sizeof(0));
   enum class align_val_t : size_t;
 
   struct destroying_delete_t {
     struct __construct { explicit __construct() = default; };
     explicit destroying_delete_t(__construct) {}

test/SemaCXX/extended-usual-deallocation-functions.cpp

 // RUN: %clang_cc1 -fexceptions -std=c++2a -fsized-deallocation -fno-aligned-allocation -verify %s
-// RUN: %clang_cc1 -fexceptions -std=c++17 -fsized-deallocation -fno-aligned-allocation -verify %s
-// RUN: %clang_cc1 -fexceptions -std=c++14 -fsized-deallocation -faligned-allocation -DHAS_ALIGN -verify %s
-// RUN: %clang_cc1 -fexceptions -std=c++11 -fsized-deallocation -faligned-allocation -DHAS_ALIGN -verify %s
+// RUN: %clang_cc1 -fexceptions -std=c++17 -fdestroying-delete -fsized-deallocation -fno-aligned-allocation -verify %s
+// RUN: %clang_cc1 -fexceptions -std=c++14 -fdestroying-delete -fsized-deallocation -faligned-allocation -DHAS_ALIGN -verify %s
+// RUN: %clang_cc1 -fexceptions -std=c++11 -fdestroying-delete -fsized-deallocation -faligned-allocation -DHAS_ALIGN -verify %s
 
 // Test that we handle aligned deallocation, sized deallocation, and destroying
 // delete as usual deallocation functions even if they are used as extensions
 // prior to C++17.
 
 namespace std {
 using size_t = decltype(sizeof(0));
 enum class align_val_t : size_t;

test/SemaCXX/warn-destroying-delete-extension.cpp

+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -verify %s
+namespace std {
+struct destroying_delete_t {
+};
+} // namespace std
+struct C {
+  // expected-warning@+2 {{destroying operator delete is ignored prior to C++2a}}
+  // expected-note@+1 {{use -fdestroying-delete to enable destroying operator delete}}
+  void operator delete(C *, std::destroying_delete_t);
+};