Skip to content

Commit

Permalink
[clang] Enable sized deallocation by default in C++14 onwards
Browse files Browse the repository at this point in the history
Since C++14 has been released for about nine years and most standard
libraries have implemented sized deallocation functions, it's time to
make this feature default again.

This is another try of https://reviews.llvm.org/D112921.

The original commit cf5a8b4 was reverted by 2e5035a due to some
failures (see #83774).

Fixes #60061
  • Loading branch information
wangpc-pp committed Apr 28, 2024
1 parent 2c1c887 commit cc72243
Show file tree
Hide file tree
Showing 42 changed files with 523 additions and 113 deletions.
4 changes: 3 additions & 1 deletion clang-tools-extra/clangd/unittests/FindTargetTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,9 @@ TEST_F(TargetDeclTest, OverloadExpr) {
[[delete]] x;
}
)cpp";
EXPECT_DECLS("CXXDeleteExpr", "void operator delete(void *) noexcept");
// Sized deallocation is enabled by default in C++14 onwards.
EXPECT_DECLS("CXXDeleteExpr",
"void operator delete(void *, unsigned long) noexcept");
}

TEST_F(TargetDeclTest, DependentExprs) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,6 @@ struct S {
// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: declaration of 'operator new' has no matching declaration of 'operator delete' at the same scope
void *operator new(size_t size) noexcept(false);

struct T {
// Sized deallocations are not enabled by default, and so this new/delete pair
// does not match. However, we expect only one warning, for the new, because
// the operator delete is a placement delete and we do not warn on mismatching
// placement operations.
// CHECK-MESSAGES: :[[@LINE+1]]:9: warning: declaration of 'operator new' has no matching declaration of 'operator delete' at the same scope
void *operator new(size_t size) noexcept;
void operator delete(void *ptr, size_t) noexcept; // ok only if sized deallocation is enabled
};

struct U {
void *operator new(size_t size) noexcept;
void operator delete(void *ptr) noexcept;
Expand Down
5 changes: 5 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ C++17 Feature Support
files because they may not be stable across multiple TUs (the values may vary
based on compiler version as well as CPU tuning). #GH60174

C++14 Feature Support
^^^^^^^^^^^^^^^^^^^^^
- Sized deallocation is enabled by default in C++14 onwards. The user may specify
``-fno-sized-deallocation`` to disable it if there are some regressions.

C++20 Feature Support
^^^^^^^^^^^^^^^^^^^^^

Expand Down
8 changes: 4 additions & 4 deletions clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,7 @@ class MarshallingInfoVisibility<KeyPathAndMacro kpm, code default>
// Key paths that are constant during parsing of options with the same key path prefix.
defvar cplusplus = LangOpts<"CPlusPlus">;
defvar cpp11 = LangOpts<"CPlusPlus11">;
defvar cpp14 = LangOpts<"CPlusPlus14">;
defvar cpp17 = LangOpts<"CPlusPlus17">;
defvar cpp20 = LangOpts<"CPlusPlus20">;
defvar c99 = LangOpts<"C99">;
Expand Down Expand Up @@ -3370,10 +3371,9 @@ defm relaxed_template_template_args : BoolFOption<"relaxed-template-template-arg
"Enable C++17 relaxed template template argument matching">,
NegFlag<SetFalse>>;
defm sized_deallocation : BoolFOption<"sized-deallocation",
LangOpts<"SizedDeallocation">, DefaultFalse,
PosFlag<SetTrue, [], [ClangOption, CC1Option],
"Enable C++14 sized global deallocation functions">,
NegFlag<SetFalse>>;
LangOpts<"SizedDeallocation">, Default<cpp14.KeyPath>,
PosFlag<SetTrue, [], [], "Enable C++14 sized global deallocation functions">,
NegFlag<SetFalse>, BothFlags<[], [ClangOption, CC1Option]>>;
defm aligned_allocation : BoolFOption<"aligned-allocation",
LangOpts<"AlignedAllocation">, Default<cpp17.KeyPath>,
PosFlag<SetTrue, [], [ClangOption], "Enable C++17 aligned allocation functions">,
Expand Down
13 changes: 9 additions & 4 deletions clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7262,10 +7262,15 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
Args.addOptInFlag(CmdArgs, options::OPT_frelaxed_template_template_args,
options::OPT_fno_relaxed_template_template_args);

// -fsized-deallocation is off by default, as it is an ABI-breaking change for
// most platforms.
Args.addOptInFlag(CmdArgs, options::OPT_fsized_deallocation,
options::OPT_fno_sized_deallocation);
// -fsized-deallocation is on by default in C++14 onwards and otherwise off
// by default.
if (Arg *A = Args.getLastArg(options::OPT_fsized_deallocation,
options::OPT_fno_sized_deallocation)) {
if (A->getOption().matches(options::OPT_fno_sized_deallocation))
CmdArgs.push_back("-fno-sized-deallocation");
else
CmdArgs.push_back("-fsized-deallocation");
}

// -faligned-allocation is on by default in C++17 onwards and otherwise off
// by default.
Expand Down
58 changes: 55 additions & 3 deletions clang/lib/Driver/ToolChains/Darwin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2912,16 +2912,68 @@ static bool sdkSupportsBuiltinModules(const Darwin::DarwinPlatformKind &TargetPl
}
}

void Darwin::addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
Action::OffloadKind DeviceOffloadKind) const {
static inline llvm::VersionTuple
sizedDeallocMinVersion(llvm::Triple::OSType OS) {
switch (OS) {
default:
break;
case llvm::Triple::Darwin:
case llvm::Triple::MacOSX: // Earliest supporting version is 10.12.
return llvm::VersionTuple(10U, 12U);
case llvm::Triple::IOS:
case llvm::Triple::TvOS: // Earliest supporting version is 10.0.0.
return llvm::VersionTuple(10U);
case llvm::Triple::WatchOS: // Earliest supporting version is 3.0.0.
return llvm::VersionTuple(3U);
}

llvm_unreachable("Unexpected OS");
}

bool Darwin::isSizedDeallocationUnavailable() const {
llvm::Triple::OSType OS;

if (isTargetMacCatalyst())
return TargetVersion < sizedDeallocMinVersion(llvm::Triple::MacOSX);
switch (TargetPlatform) {
case MacOS: // Earlier than 10.12.
OS = llvm::Triple::MacOSX;
break;
case IPhoneOS:
OS = llvm::Triple::IOS;
break;
case TvOS: // Earlier than 10.0.
OS = llvm::Triple::TvOS;
break;
case WatchOS: // Earlier than 3.0.
OS = llvm::Triple::WatchOS;
break;
case DriverKit:
case XROS:
// Always available.
return false;
}

return TargetVersion < sizedDeallocMinVersion(OS);
}

void Darwin::addClangTargetOptions(
const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args,
Action::OffloadKind DeviceOffloadKind) const {
// Pass "-faligned-alloc-unavailable" only when the user hasn't manually
// enabled or disabled aligned allocations.
if (!DriverArgs.hasArgNoClaim(options::OPT_faligned_allocation,
options::OPT_fno_aligned_allocation) &&
isAlignedAllocationUnavailable())
CC1Args.push_back("-faligned-alloc-unavailable");

// Pass "-fno-sized-deallocation" only when the user hasn't manually enabled
// or disabled sized deallocations.
if (!DriverArgs.hasArgNoClaim(options::OPT_fsized_deallocation,
options::OPT_fno_sized_deallocation) &&
isSizedDeallocationUnavailable())
CC1Args.push_back("-fno-sized-deallocation");

addClangCC1ASTargetOptions(DriverArgs, CC1Args);

// Enable compatibility mode for NSItemProviderCompletionHandler in
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/Driver/ToolChains/Darwin.h
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,10 @@ class LLVM_LIBRARY_VISIBILITY Darwin : public MachO {
/// targeting.
bool isAlignedAllocationUnavailable() const;

/// Return true if c++14 sized deallocation functions are not implemented in
/// the c++ standard library of the deployment target we are targeting.
bool isSizedDeallocationUnavailable() const;

void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
Action::OffloadKind DeviceOffloadKind) const override;
Expand Down
6 changes: 6 additions & 0 deletions clang/lib/Driver/ToolChains/ZOS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ void ZOS::addClangTargetOptions(const ArgList &DriverArgs,
if (!DriverArgs.hasArgNoClaim(options::OPT_faligned_allocation,
options::OPT_fno_aligned_allocation))
CC1Args.push_back("-faligned-alloc-unavailable");

// Pass "-fno-sized-deallocation" only when the user hasn't manually enabled
// or disabled sized deallocations.
if (!DriverArgs.hasArgNoClaim(options::OPT_fsized_deallocation,
options::OPT_fno_sized_deallocation))
CC1Args.push_back("-fno-sized-deallocation");
}

void zos::Assembler::ConstructJob(Compilation &C, const JobAction &JA,
Expand Down
2 changes: 1 addition & 1 deletion clang/test/AST/ast-dump-expr-json.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2333,7 +2333,7 @@ void TestNonADLCall3() {
// CHECK-NEXT: "kind": "FunctionDecl",
// CHECK-NEXT: "name": "operator delete",
// CHECK-NEXT: "type": {
// CHECK-NEXT: "qualType": "void (void *) noexcept"
// CHECK-NEXT: "qualType": "void (void *, unsigned long) noexcept"
// CHECK-NEXT: }
// CHECK-NEXT: },
// CHECK-NEXT: "inner": [
Expand Down
2 changes: 1 addition & 1 deletion clang/test/AST/ast-dump-expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ void UnaryExpressions(int *p) {
// CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} <col:8> 'int *' lvalue ParmVar 0x{{[^ ]*}} 'p' 'int *'

::delete p;
// CHECK: CXXDeleteExpr 0x{{[^ ]*}} <line:[[@LINE-1]]:3, col:12> 'void' global Function 0x{{[^ ]*}} 'operator delete' 'void (void *) noexcept'
// CHECK: CXXDeleteExpr 0x{{[^ ]*}} <line:[[@LINE-1]]:3, col:12> 'void' global Function 0x{{[^ ]*}} 'operator delete' 'void (void *, unsigned long) noexcept'
// CHECK-NEXT: ImplicitCastExpr
// CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} <col:12> 'int *' lvalue ParmVar 0x{{[^ ]*}} 'p' 'int *'

Expand Down

0 comments on commit cc72243

Please sign in to comment.