Skip to content

Commit 3eb97e0

Browse files
committed
Remove pseudo coverage of allocation for coroutines
While coroutines perform a notionally usual lookup for new/delete on the promise type, they are using the result to allocate space for an anonymous type representing the frame. There's no C++ representation of this type (currently) so simply passing the promise type is erroneous, and there's no real mechanism vend a real type. In the long term this needs to be addressed by the coroutine specification.
1 parent 19f69d0 commit 3eb97e0

File tree

11 files changed

+254
-124
lines changed

11 files changed

+254
-124
lines changed

clang/include/clang/AST/ExprCXX.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2261,6 +2261,12 @@ inline SizedDeallocationMode sizedDeallocationModeFromBool(bool IsSized) {
22612261
}
22622262

22632263
struct ImplicitAllocationParameters {
2264+
ImplicitAllocationParameters(QualType Type,
2265+
TypeAwareAllocationMode PassTypeIdentity,
2266+
AlignedAllocationMode PassAlignment)
2267+
: Type(Type.isNull() ? Type : Type.getUnqualifiedType()),
2268+
PassTypeIdentity(PassTypeIdentity), PassAlignment(PassAlignment) {}
2269+
QualType Type;
22642270
TypeAwareAllocationMode PassTypeIdentity;
22652271
AlignedAllocationMode PassAlignment;
22662272
unsigned getNumImplicitArgs() const {
@@ -2274,6 +2280,14 @@ struct ImplicitAllocationParameters {
22742280
};
22752281

22762282
struct ImplicitDeallocationParameters {
2283+
ImplicitDeallocationParameters(QualType Type,
2284+
TypeAwareAllocationMode PassTypeIdentity,
2285+
AlignedAllocationMode PassAlignment,
2286+
SizedDeallocationMode PassSize)
2287+
: Type(Type.isNull() ? Type : Type.getUnqualifiedType()),
2288+
PassTypeIdentity(PassTypeIdentity), PassAlignment(PassAlignment),
2289+
PassSize(PassSize) {}
2290+
QualType Type;
22772291
TypeAwareAllocationMode PassTypeIdentity;
22782292
AlignedAllocationMode PassAlignment;
22792293
SizedDeallocationMode PassSize;
@@ -2502,6 +2516,7 @@ class CXXNewExpr final
25022516
/// parameters in this call
25032517
ImplicitAllocationParameters implicitAllocationParameters() const {
25042518
return ImplicitAllocationParameters{
2519+
getAllocatedType(),
25052520
typeAwareAllocationModeFromBool(CXXNewExprBits.ShouldPassTypeIdentity),
25062521
alignedAllocationModeFromBool(CXXNewExprBits.ShouldPassAlignment)};
25072522
}

clang/include/clang/Basic/DiagnosticGroups.td

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,11 @@ def AlwaysInlineCoroutine :
6464
DiagGroup<"always-inline-coroutine">;
6565
def CoroNonAlignedAllocationFunction :
6666
DiagGroup<"coro-non-aligned-allocation-function">;
67+
def CoroTypeAwareAllocationFunction :
68+
DiagGroup<"coro-type-aware-allocation-function">;
6769
def Coroutine : DiagGroup<"coroutine", [CoroutineMissingUnhandledException, DeprecatedCoroutine,
68-
AlwaysInlineCoroutine, CoroNonAlignedAllocationFunction]>;
70+
AlwaysInlineCoroutine, CoroNonAlignedAllocationFunction,
71+
CoroTypeAwareAllocationFunction]>;
6972
def ObjCBoolConstantConversion : DiagGroup<"objc-bool-constant-conversion">;
7073
def ConstantConversion : DiagGroup<"constant-conversion",
7174
[BitFieldConstantConversion,

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9733,7 +9733,7 @@ def err_unsupported_type_aware_allocator : Error<
97339733
def warn_type_aware_cleanup_deallocator_context_mismatch : Warning<
97349734
"type aware %0 requires matching %1 in %2">,
97359735
InGroup<StrictTypeAwareAllocators>, DefaultError;
9736-
def err_type_aware_operator_found : Note<
9736+
def note_type_aware_operator_found : Note<
97379737
"type aware %0 found in %1">;
97389738
def warn_mismatching_type_aware_cleanup_deallocator : Warning<
97399739
"mismatched type aware allocation operators for constructor cleanup">,
@@ -12044,6 +12044,9 @@ def warn_always_inline_coroutine : Warning<
1204412044
def err_coroutine_unusable_new : Error<
1204512045
"'operator new' provided by %0 is not usable with the function signature of %1"
1204612046
>;
12047+
def note_coroutine_unusable_type_aware_allocators : Note<
12048+
"type aware %0 will not be used for coroutine allocation"
12049+
>;
1204712050
def err_coroutine_unfound_nothrow_new : Error <
1204812051
"unable to find %select{'::operator new(size_t, nothrow_t)'|"
1204912052
"'::operator new(size_t, align_val_t, nothrow_t)'}1 for %0"
@@ -12063,6 +12066,9 @@ def err_coroutine_return_type : Error<
1206312066
"function returns a type %0 marked with [[clang::coro_return_type]] but is neither a coroutine nor a coroutine wrapper; "
1206412067
"non-coroutines should be marked with [[clang::coro_wrapper]] to allow returning coroutine return type"
1206512068
>;
12069+
def warn_coroutine_type_aware_allocator_ignored : Warning <
12070+
"type aware %0 will not be used for coroutine allocation">,
12071+
InGroup<CoroTypeAwareAllocationFunction>;
1206612072
} // end of coroutines issue category
1206712073

1206812074
let CategoryName = "Documentation Issue" in {

clang/lib/AST/DeclCXX.cpp

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2480,26 +2480,20 @@ bool CXXMethodDecl::isUsualDeallocationFunction(
24802480
bool IsTypeAware = isTypeAwareOperatorNewOrDelete();
24812481

24822482
// C++ [basic.stc.dynamic.deallocation]p2:
2483+
// Pre-type aware allocators:
24832484
// A template instance is never a usual deallocation function,
24842485
// regardless of its signature.
2486+
// Pending final C++26 text:
2487+
// A template instance is only a usual deallocation function if it
2488+
// is a type aware deallocation function, and only the type-identity
2489+
// parameter is dependent.
24852490
if (FunctionTemplateDecl *PrimaryTemplate = getPrimaryTemplate()) {
2486-
// Addendum: a template instance is a usual deallocation function if there
2487-
// is a single template parameter, that parameter is a type, only the first
2488-
// parameter is dependent, and that parameter is a specialization of
2489-
// std::type_identity.
24902491
if (!IsTypeAware) {
24912492
// Stop early on if the specialization is not explicitly type aware
24922493
return false;
24932494
}
24942495

24952496
FunctionDecl *SpecializedDecl = PrimaryTemplate->getTemplatedDecl();
2496-
if (!SpecializedDecl->isTypeAwareOperatorNewOrDelete()) {
2497-
// The underlying template decl must also be explicitly type aware
2498-
// e.g. template <typename T> void operator delete(T, ...)
2499-
// specialized on T=type_identity<X> is not usual
2500-
return false;
2501-
}
2502-
25032497
for (unsigned Idx = 1; Idx < NumParams; ++Idx) {
25042498
if (SpecializedDecl->getParamDecl(Idx)->getType()->isDependentType())
25052499
return false;
@@ -2512,9 +2506,10 @@ bool CXXMethodDecl::isUsualDeallocationFunction(
25122506

25132507
// C++ [basic.stc.dynamic.deallocation]p2:
25142508
// If a class T has a member deallocation function named operator delete
2515-
// with exactly one parameter, then that function is a usual (non-placement)
2509+
// with exactly one parameter or a type aware operator delete with two
2510+
// two arguments, then that function is a usual (non-placement)
25162511
// deallocation function. [...]
2517-
if (getNumParams() == 1)
2512+
if (getNumParams() == UsualParams)
25182513
return true;
25192514

25202515
// C++ P0722:

clang/lib/Sema/SemaCoroutine.cpp

Lines changed: 71 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,12 +1095,40 @@ static TypeSourceInfo *getTypeSourceInfoForStdAlignValT(Sema &S,
10951095
return S.Context.getTrivialTypeSourceInfo(StdAlignValDecl);
10961096
}
10971097

1098+
// When searching for custom allocators on the PromiseType we want to
1099+
// warn that we will ignore type aware allocators.
1100+
static bool DiagnoseTypeAwareAllocatorsIfNecessary(Sema &S, SourceLocation Loc,
1101+
unsigned DiagnosticID,
1102+
DeclarationName Name,
1103+
QualType PromiseType) {
1104+
if (!S.getLangOpts().TypeAwareAllocators)
1105+
return false;
1106+
if (!PromiseType->isRecordType())
1107+
return false;
1108+
LookupResult R(S, Name, Loc, Sema::LookupOrdinaryName);
1109+
S.LookupQualifiedName(R, PromiseType->getAsCXXRecordDecl());
1110+
bool HaveIssuedWarning = false;
1111+
for (auto Decl : R) {
1112+
if (S.isTypeAwareOperatorNewOrDelete(Decl)) {
1113+
if (!HaveIssuedWarning) {
1114+
S.Diag(Loc, DiagnosticID) << Name;
1115+
HaveIssuedWarning = true;
1116+
}
1117+
S.Diag(Decl->getLocation(), diag::note_type_aware_operator_declared)
1118+
<< 0 << Decl;
1119+
}
1120+
}
1121+
return HaveIssuedWarning;
1122+
}
1123+
10981124
// Find an appropriate delete for the promise.
10991125
static bool findDeleteForPromise(Sema &S, SourceLocation Loc, QualType PromiseType,
11001126
FunctionDecl *&OperatorDelete) {
11011127
DeclarationName DeleteName =
11021128
S.Context.DeclarationNames.getCXXOperatorName(OO_Delete);
1103-
1129+
DiagnoseTypeAwareAllocatorsIfNecessary(
1130+
S, Loc, diag::warn_coroutine_type_aware_allocator_ignored, DeleteName,
1131+
PromiseType);
11041132
auto *PointeeRD = PromiseType->getAsCXXRecordDecl();
11051133
assert(PointeeRD && "PromiseType must be a CxxRecordDecl type");
11061134

@@ -1111,7 +1139,7 @@ static bool findDeleteForPromise(Sema &S, SourceLocation Loc, QualType PromiseTy
11111139
// scope of the promise type. If nothing is found, a search is performed in
11121140
// the global scope.
11131141
ImplicitDeallocationParameters IDP = {
1114-
typeAwareAllocationModeFromBool(S.getLangOpts().TypeAwareAllocators),
1142+
S.Context.VoidTy, TypeAwareAllocationMode::No,
11151143
alignedAllocationModeFromBool(Overaligned), SizedDeallocationMode::Yes};
11161144
if (S.FindDeallocationFunction(Loc, PointeeRD, DeleteName, OperatorDelete,
11171145
PromiseType, IDP, /*Diagnose*/ true))
@@ -1407,10 +1435,10 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
14071435

14081436
FunctionDecl *OperatorNew = nullptr;
14091437
SmallVector<Expr *, 1> PlacementArgs;
1438+
DeclarationName NewName =
1439+
S.getASTContext().DeclarationNames.getCXXOperatorName(OO_New);
14101440

1411-
const bool PromiseContainsNew = [this, &PromiseType]() -> bool {
1412-
DeclarationName NewName =
1413-
S.getASTContext().DeclarationNames.getCXXOperatorName(OO_New);
1441+
const bool PromiseContainsNew = [this, &PromiseType, NewName]() -> bool {
14141442
LookupResult R(S, NewName, Loc, Sema::LookupOrdinaryName);
14151443

14161444
if (PromiseType->isRecordType())
@@ -1422,36 +1450,36 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
14221450
// Helper function to indicate whether the last lookup found the aligned
14231451
// allocation function.
14241452
ImplicitAllocationParameters IAP = {
1425-
typeAwareAllocationModeFromBool(S.getLangOpts().TypeAwareAllocators),
1453+
S.Context.VoidTy, TypeAwareAllocationMode::No,
14261454
alignedAllocationModeFromBool(S.getLangOpts().CoroAlignedAllocation)};
1427-
auto LookupAllocationFunction = [&](Sema::AllocationFunctionScope NewScope =
1428-
Sema::AFS_Both,
1429-
bool WithoutPlacementArgs = false,
1430-
bool ForceNonAligned = false) {
1431-
// [dcl.fct.def.coroutine]p9
1432-
// The allocation function's name is looked up by searching for it in
1433-
// the
1434-
// scope of the promise type.
1435-
// - If any declarations are found, ...
1436-
// - If no declarations are found in the scope of the promise type, a
1437-
// search is performed in the global scope.
1438-
if (NewScope == Sema::AFS_Both)
1439-
NewScope = PromiseContainsNew ? Sema::AFS_Class : Sema::AFS_Global;
1440-
1441-
bool ShouldUseAlignedAlloc =
1442-
!ForceNonAligned && S.getLangOpts().CoroAlignedAllocation;
1443-
IAP = {typeAwareAllocationModeFromBool(S.getLangOpts().TypeAwareAllocators),
1444-
alignedAllocationModeFromBool(ShouldUseAlignedAlloc)};
1445-
1446-
FunctionDecl *UnusedResult = nullptr;
1447-
1448-
S.FindAllocationFunctions(Loc, SourceRange(), NewScope,
1449-
/*DeleteScope*/ Sema::AFS_Both, PromiseType,
1450-
/*isArray*/ false, IAP,
1451-
WithoutPlacementArgs ? MultiExprArg{}
1452-
: PlacementArgs,
1453-
OperatorNew, UnusedResult, /*Diagnose*/ false);
1454-
};
1455+
auto LookupAllocationFunction =
1456+
[&](Sema::AllocationFunctionScope NewScope = Sema::AFS_Both,
1457+
bool WithoutPlacementArgs = false, bool ForceNonAligned = false) {
1458+
// [dcl.fct.def.coroutine]p9
1459+
// The allocation function's name is looked up by searching for it in
1460+
// the
1461+
// scope of the promise type.
1462+
// - If any declarations are found, ...
1463+
// - If no declarations are found in the scope of the promise type, a
1464+
// search is performed in the global scope.
1465+
if (NewScope == Sema::AFS_Both)
1466+
NewScope = PromiseContainsNew ? Sema::AFS_Class : Sema::AFS_Global;
1467+
1468+
bool ShouldUseAlignedAlloc =
1469+
!ForceNonAligned && S.getLangOpts().CoroAlignedAllocation;
1470+
IAP = {S.Context.VoidTy, TypeAwareAllocationMode::No,
1471+
alignedAllocationModeFromBool(ShouldUseAlignedAlloc)};
1472+
1473+
FunctionDecl *UnusedResult = nullptr;
1474+
1475+
S.FindAllocationFunctions(
1476+
Loc, SourceRange(), NewScope,
1477+
/*DeleteScope*/ Sema::AFS_Both, PromiseType,
1478+
/*isArray*/ false, IAP,
1479+
WithoutPlacementArgs ? MultiExprArg{} : PlacementArgs, OperatorNew,
1480+
UnusedResult, /*Diagnose*/ false);
1481+
assert(!OperatorNew || !OperatorNew->isTypeAwareOperatorNewOrDelete());
1482+
};
14551483

14561484
// We don't expect to call to global operator new with (size, p0, …, pn).
14571485
// So if we choose to lookup the allocation function in global scope, we
@@ -1537,15 +1565,22 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
15371565
}
15381566

15391567
if (!OperatorNew) {
1540-
if (PromiseContainsNew)
1568+
if (PromiseContainsNew) {
15411569
S.Diag(Loc, diag::err_coroutine_unusable_new) << PromiseType << &FD;
1542-
else if (RequiresNoThrowAlloc)
1570+
DiagnoseTypeAwareAllocatorsIfNecessary(
1571+
S, Loc, diag::note_coroutine_unusable_type_aware_allocators, NewName,
1572+
PromiseType);
1573+
} else if (RequiresNoThrowAlloc)
15431574
S.Diag(Loc, diag::err_coroutine_unfound_nothrow_new)
15441575
<< &FD << S.getLangOpts().CoroAlignedAllocation;
15451576

15461577
return false;
15471578
}
15481579

1580+
DiagnoseTypeAwareAllocatorsIfNecessary(
1581+
S, Loc, diag::warn_coroutine_type_aware_allocator_ignored, NewName,
1582+
PromiseType);
1583+
15491584
if (RequiresNoThrowAlloc) {
15501585
const auto *FT = OperatorNew->getType()->castAs<FunctionProtoType>();
15511586
if (!FT->isNothrow(/*ResultIfDependent*/ false)) {

clang/lib/Sema/SemaDeclCXX.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9716,6 +9716,7 @@ bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD,
97169716
DeclarationName Name =
97179717
Context.DeclarationNames.getCXXOperatorName(OO_Delete);
97189718
ImplicitDeallocationParameters IDP = {
9719+
DeallocType,
97199720
typeAwareAllocationModeFromBool(getLangOpts().TypeAwareAllocators),
97209721
AlignedAllocationMode::No, SizedDeallocationMode::No};
97219722
if (FindDeallocationFunction(MD->getLocation(), MD->getParent(), Name,

clang/lib/Sema/SemaExprCXX.cpp

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1775,12 +1775,15 @@ static bool isNonPlacementDeallocationFunction(Sema &S, FunctionDecl *FD) {
17751775

17761776
namespace {
17771777
struct UsualDeallocFnInfo {
1778-
UsualDeallocFnInfo() : Found(), FD(nullptr) {}
1778+
UsualDeallocFnInfo()
1779+
: Found(), FD(nullptr),
1780+
IDP(QualType(), TypeAwareAllocationMode::No,
1781+
AlignedAllocationMode::No, SizedDeallocationMode::No) {}
17791782
UsualDeallocFnInfo(Sema &S, DeclAccessPair Found, QualType AllocType)
17801783
: Found(Found), FD(dyn_cast<FunctionDecl>(Found->getUnderlyingDecl())),
17811784
Destroying(false),
1782-
IDP({TypeAwareAllocationMode::No, AlignedAllocationMode::No,
1783-
SizedDeallocationMode::No}),
1785+
IDP({AllocType, TypeAwareAllocationMode::No,
1786+
AlignedAllocationMode::No, SizedDeallocationMode::No}),
17841787
CUDAPref(SemaCUDA::CFP_Native) {
17851788
// A function template declaration is only a usual deallocation function
17861789
// if it is a typed delete.
@@ -2008,7 +2011,7 @@ static bool doesUsualArrayDeleteWantSize(Sema &S, SourceLocation loc,
20082011
// If the deallocation functions have class scope, the one without a
20092012
// parameter of type std::size_t is selected.
20102013
ImplicitDeallocationParameters IDP = {
2011-
PassType,
2014+
allocType, PassType,
20122015
alignedAllocationModeFromBool(hasNewExtendedAlignment(S, allocType)),
20132016
SizedDeallocationMode::No};
20142017
auto Best = resolveDeallocationOverload(S, ops, IDP, allocType);
@@ -2425,6 +2428,7 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
24252428
AllocType->isDependentType() ? 0 : Context.getTypeAlign(AllocType);
24262429
unsigned NewAlignment = Context.getTargetInfo().getNewAlign();
24272430
ImplicitAllocationParameters IAP = {
2431+
AllocType,
24282432
typeAwareAllocationModeFromBool(getLangOpts().TypeAwareAllocators),
24292433
alignedAllocationModeFromBool(getLangOpts().AlignedAllocation &&
24302434
Alignment > NewAlignment)};
@@ -3159,7 +3163,7 @@ bool Sema::FindAllocationFunctions(
31593163
// with a size_t where possible (which it always is in this case).
31603164
llvm::SmallVector<UsualDeallocFnInfo, 4> BestDeallocFns;
31613165
ImplicitDeallocationParameters IDP = {
3162-
OriginalTypeAwareState,
3166+
AllocElemType, OriginalTypeAwareState,
31633167
alignedAllocationModeFromBool(
31643168
hasNewExtendedAlignment(*this, AllocElemType)),
31653169
sizedDeallocationModeFromBool(FoundGlobalDelete)};
@@ -3197,9 +3201,9 @@ bool Sema::FindAllocationFunctions(
31973201
OperatorDelete->getDeclContext() != OperatorNew->getDeclContext()) {
31983202
Diag(StartLoc, diag::warn_type_aware_cleanup_deallocator_context_mismatch)
31993203
<< OperatorNew << DeleteName << OperatorNew->getDeclContext();
3200-
Diag(OperatorNew->getLocation(), diag::err_type_aware_operator_found)
3204+
Diag(OperatorNew->getLocation(), diag::note_type_aware_operator_found)
32013205
<< OperatorNew << OperatorNew->getDeclContext();
3202-
Diag(OperatorDelete->getLocation(), diag::err_type_aware_operator_found)
3206+
Diag(OperatorDelete->getLocation(), diag::note_type_aware_operator_found)
32033207
<< OperatorDelete << OperatorDelete->getDeclContext();
32043208
}
32053209

@@ -3221,7 +3225,7 @@ bool Sema::FindAllocationFunctions(
32213225
bool IsSizedDelete = isSizedDeallocation(Info.IDP.PassSize);
32223226
if (IsSizedDelete && !FoundGlobalDelete) {
32233227
ImplicitDeallocationParameters SizeTestingIDP = {
3224-
Info.IDP.PassTypeIdentity, Info.IDP.PassAlignment,
3228+
AllocElemType, Info.IDP.PassTypeIdentity, Info.IDP.PassAlignment,
32253229
SizedDeallocationMode::No};
32263230
auto NonSizedDelete = resolveDeallocationOverload(
32273231
*this, FoundDelete, SizeTestingIDP, AllocElemType);
@@ -3544,6 +3548,7 @@ FunctionDecl *Sema::FindDeallocationFunctionForDestructor(SourceLocation Loc,
35443548
FunctionDecl *OperatorDelete = nullptr;
35453549
QualType DeallocType = Context.getRecordType(RD);
35463550
ImplicitDeallocationParameters IDP = {
3551+
DeallocType,
35473552
typeAwareAllocationModeFromBool(getLangOpts().TypeAwareAllocators),
35483553
AlignedAllocationMode::No, SizedDeallocationMode::No};
35493554

@@ -4017,6 +4022,7 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
40174022

40184023
if (PointeeRD) {
40194024
ImplicitDeallocationParameters IDP = {
4025+
Pointee,
40204026
typeAwareAllocationModeFromBool(getLangOpts().TypeAwareAllocators),
40214027
AlignedAllocationMode::No, SizedDeallocationMode::No};
40224028
if (!UseGlobal &&
@@ -4070,6 +4076,7 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
40704076

40714077
// Look for a global declaration.
40724078
ImplicitDeallocationParameters IDP = {
4079+
Pointee,
40734080
typeAwareAllocationModeFromBool(getLangOpts().TypeAwareAllocators),
40744081
alignedAllocationModeFromBool(Overaligned),
40754082
sizedDeallocationModeFromBool(CanProvideSize)};

0 commit comments

Comments
 (0)