EricWF created this revision.
EricWF added reviewers: ckennelly, rsmith.
Herald added a project: clang.
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
Repository:
rC Clang
https://reviews.llvm.org/D57645
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/LangOptions.def
include/clang/Driver/Options.td
lib/AST/DeclCXX.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/InitPreprocessor.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExprCXX.cpp
test/CodeGenCXX/cxx2a-destroying-delete.cpp
test/SemaCXX/cxx2a-destroying-delete.cpp
test/SemaCXX/extended-usual-deallocation-functions.cpp
test/SemaCXX/warn-destroying-delete-extension.cpp
Index: test/SemaCXX/warn-destroying-delete-extension.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/warn-destroying-delete-extension.cpp
@@ -0,0 +1,10 @@
+// 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);
+};
Index: test/SemaCXX/extended-usual-deallocation-functions.cpp
===================================================================
--- test/SemaCXX/extended-usual-deallocation-functions.cpp
+++ test/SemaCXX/extended-usual-deallocation-functions.cpp
@@ -1,7 +1,7 @@
// 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
Index: test/SemaCXX/cxx2a-destroying-delete.cpp
===================================================================
--- test/SemaCXX/cxx2a-destroying-delete.cpp
+++ test/SemaCXX/cxx2a-destroying-delete.cpp
@@ -1,5 +1,6 @@
// 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));
Index: test/CodeGenCXX/cxx2a-destroying-delete.cpp
===================================================================
--- test/CodeGenCXX/cxx2a-destroying-delete.cpp
+++ test/CodeGenCXX/cxx2a-destroying-delete.cpp
@@ -1,4 +1,5 @@
// 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 {
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -1506,7 +1506,7 @@
if (!FD)
return;
unsigned NumBaseParams = 1;
- if (FD->isDestroyingOperatorDelete()) {
+ if (FD->isDestroyingOperatorDelete() && S.LangOpts.DestroyingDelete) {
Destroying = true;
++NumBaseParams;
}
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -13311,14 +13311,23 @@
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;
Index: lib/Frontend/InitPreprocessor.cpp
===================================================================
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -542,7 +542,8 @@
// C++20 features.
if (LangOpts.Char8)
Builder.defineMacro("__cpp_char8_t", "201811L");
- Builder.defineMacro("__cpp_impl_destroying_delete", "201806L");
+ if (LangOpts.DestroyingDelete)
+ Builder.defineMacro("__cpp_impl_destroying_delete", "201806L");
// TS features.
if (LangOpts.ConceptsTS)
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2172,6 +2172,7 @@
Opts.CXXOperatorNames = Opts.CPlusPlus;
Opts.AlignedAllocation = Opts.CPlusPlus17;
+ Opts.DestroyingDelete = Opts.CPlusPlus2a;
Opts.DollarIdents = !Opts.AsmPreprocessor;
}
@@ -2633,6 +2634,8 @@
<< 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);
Index: lib/Driver/ToolChains/Clang.cpp
===================================================================
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4855,6 +4855,10 @@
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,
Index: lib/AST/DeclCXX.cpp
===================================================================
--- lib/AST/DeclCXX.cpp
+++ lib/AST/DeclCXX.cpp
@@ -2059,13 +2059,18 @@
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
@@ -2077,7 +2082,6 @@
// (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()))
@@ -2097,7 +2101,7 @@
// 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
Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1517,6 +1517,8 @@
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>;
Index: include/clang/Basic/LangOptions.def
===================================================================
--- include/clang/Basic/LangOptions.def
+++ include/clang/Basic/LangOptions.def
@@ -219,6 +219,7 @@
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")
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7654,6 +7654,10 @@
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<
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
