Skip to content

Commit c0ec6c3

Browse files
davezarzyckidavezarzycki
committed
[AST] NFC: Make ExtInfo param Optional<>
While it is very convenient to default the ExtInfo state when creating new function types, it also make the intent unclear to those looking to extend ExtInfo state. For example, did a given call site intend to have the default ExtInfo state or does it just happen to work? This matters a lot because function types are regularly unpacked and rebuilt and it's really easy to accidentally drop ExtInfo state. By changing the ExtInfo state to an optional, we can track when it is actually needed.
1 parent aa74b1b commit c0ec6c3

23 files changed

+392
-206
lines changed

‎include/swift/AST/Types.h

Lines changed: 47 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include "llvm/ADT/ArrayRef.h"
3838
#include "llvm/ADT/DenseMapInfo.h"
3939
#include "llvm/ADT/FoldingSet.h"
40+
#include "llvm/ADT/Optional.h"
4041
#include "llvm/ADT/PointerEmbeddedInt.h"
4142
#include "llvm/ADT/PointerUnion.h"
4243
#include "llvm/ADT/SmallBitVector.h"
@@ -347,10 +348,11 @@ class alignas(1 << TypeAlignInBits) TypeBase {
347348
Flags : NumFlagBits
348349
);
349350

350-
SWIFT_INLINE_BITFIELD_FULL(AnyFunctionType, TypeBase, NumAFTExtInfoBits+1+16,
351+
SWIFT_INLINE_BITFIELD_FULL(AnyFunctionType, TypeBase, NumAFTExtInfoBits+1+1+16,
351352
/// Extra information which affects how the function is called, like
352353
/// regparm and the calling convention.
353354
ExtInfoBits : NumAFTExtInfoBits,
355+
HasExtInfo : 1,
354356
HasClangTypeInfo : 1,
355357
: NumPadBits,
356358
NumParams : 16
@@ -2919,20 +2921,28 @@ class AnyFunctionType : public TypeBase {
29192921
///
29202922
/// Subclasses are responsible for storing and retrieving the
29212923
/// ClangTypeInfo value if one is present.
2922-
AnyFunctionType(TypeKind Kind, const ASTContext *CanTypeContext,
2923-
Type Output, RecursiveTypeProperties properties,
2924-
unsigned NumParams, ExtInfo Info)
2925-
: TypeBase(Kind, CanTypeContext, properties), Output(Output) {
2926-
Bits.AnyFunctionType.ExtInfoBits = Info.getBits();
2927-
Bits.AnyFunctionType.HasClangTypeInfo = !Info.getClangTypeInfo().empty();
2924+
AnyFunctionType(TypeKind Kind, const ASTContext *CanTypeContext, Type Output,
2925+
RecursiveTypeProperties properties, unsigned NumParams,
2926+
Optional<ExtInfo> Info)
2927+
: TypeBase(Kind, CanTypeContext, properties), Output(Output) {
2928+
if (Info.hasValue()) {
2929+
Bits.AnyFunctionType.HasExtInfo = true;
2930+
Bits.AnyFunctionType.ExtInfoBits = Info.getValue().getBits();
2931+
Bits.AnyFunctionType.HasClangTypeInfo =
2932+
!Info.getValue().getClangTypeInfo().empty();
2933+
// The use of both assert() and static_assert() is intentional.
2934+
assert(Bits.AnyFunctionType.ExtInfoBits == Info.getValue().getBits() &&
2935+
"Bits were dropped!");
2936+
static_assert(
2937+
ASTExtInfoBuilder::NumMaskBits == NumAFTExtInfoBits,
2938+
"ExtInfo and AnyFunctionTypeBitfields must agree on bit size");
2939+
} else {
2940+
Bits.AnyFunctionType.HasExtInfo = false;
2941+
Bits.AnyFunctionType.HasClangTypeInfo = false;
2942+
Bits.AnyFunctionType.ExtInfoBits = 0;
2943+
}
29282944
Bits.AnyFunctionType.NumParams = NumParams;
29292945
assert(Bits.AnyFunctionType.NumParams == NumParams && "Params dropped!");
2930-
// The use of both assert() and static_assert() is intentional.
2931-
assert(Bits.AnyFunctionType.ExtInfoBits == Info.getBits() &&
2932-
"Bits were dropped!");
2933-
static_assert(
2934-
ASTExtInfoBuilder::NumMaskBits == NumAFTExtInfoBits,
2935-
"ExtInfo and AnyFunctionTypeBitfields must agree on bit size");
29362946
}
29372947

29382948
public:
@@ -2993,7 +3003,10 @@ class AnyFunctionType : public TypeBase {
29933003
/// outer function type's mangling doesn't need to duplicate that information.
29943004
bool hasNonDerivableClangType();
29953005

3006+
bool hasExtInfo() const { return Bits.AnyFunctionType.HasExtInfo; }
3007+
29963008
ExtInfo getExtInfo() const {
3009+
assert(hasExtInfo());
29973010
return ExtInfo(Bits.AnyFunctionType.ExtInfoBits, getClangTypeInfo());
29983011
}
29993012

@@ -3002,6 +3015,7 @@ class AnyFunctionType : public TypeBase {
30023015
/// The parameter useClangFunctionType is present only for staging purposes.
30033016
/// In the future, we will always use the canonical clang function type.
30043017
ExtInfo getCanonicalExtInfo(bool useClangFunctionType) const {
3018+
assert(hasExtInfo());
30053019
return ExtInfo(Bits.AnyFunctionType.ExtInfoBits,
30063020
useClangFunctionType ? getCanonicalClangTypeInfo()
30073021
: ClangTypeInfo());
@@ -3193,7 +3207,7 @@ BEGIN_CAN_TYPE_WRAPPER(AnyFunctionType, Type)
31933207
static CanAnyFunctionType get(CanGenericSignature signature,
31943208
CanParamArrayRef params,
31953209
CanType result,
3196-
ExtInfo info = ExtInfo());
3210+
Optional<ExtInfo> info = None);
31973211

31983212
CanGenericSignature getOptGenericSignature() const;
31993213

@@ -3234,7 +3248,7 @@ class FunctionType final
32343248
public:
32353249
/// 'Constructor' Factory Function
32363250
static FunctionType *get(ArrayRef<Param> params, Type result,
3237-
ExtInfo info = ExtInfo());
3251+
Optional<ExtInfo> info = None);
32383252

32393253
// Retrieve the input parameters of this function type.
32403254
ArrayRef<Param> getParams() const {
@@ -3251,25 +3265,28 @@ class FunctionType final
32513265
}
32523266

32533267
void Profile(llvm::FoldingSetNodeID &ID) {
3254-
Profile(ID, getParams(), getResult(), getExtInfo());
3268+
Optional<ExtInfo> info = None;
3269+
if (hasExtInfo())
3270+
info = getExtInfo();
3271+
Profile(ID, getParams(), getResult(), info);
32553272
}
32563273
static void Profile(llvm::FoldingSetNodeID &ID,
32573274
ArrayRef<Param> params,
32583275
Type result,
3259-
ExtInfo info);
3276+
Optional<ExtInfo> info);
32603277

32613278
// Implement isa/cast/dyncast/etc.
32623279
static bool classof(const TypeBase *T) {
32633280
return T->getKind() == TypeKind::Function;
32643281
}
32653282

32663283
private:
3267-
FunctionType(ArrayRef<Param> params, Type result, ExtInfo info,
3284+
FunctionType(ArrayRef<Param> params, Type result, Optional<ExtInfo> info,
32683285
const ASTContext *ctx, RecursiveTypeProperties properties);
32693286
};
32703287
BEGIN_CAN_TYPE_WRAPPER(FunctionType, AnyFunctionType)
32713288
static CanFunctionType get(CanParamArrayRef params, CanType result,
3272-
ExtInfo info = ExtInfo()) {
3289+
Optional<ExtInfo> info = None) {
32733290
auto fnType = FunctionType::get(params.getOriginalArray(), result, info);
32743291
return cast<FunctionType>(fnType->getCanonicalType());
32753292
}
@@ -3334,7 +3351,7 @@ class GenericFunctionType final : public AnyFunctionType,
33343351
GenericFunctionType(GenericSignature sig,
33353352
ArrayRef<Param> params,
33363353
Type result,
3337-
ExtInfo info,
3354+
Optional<ExtInfo> info,
33383355
const ASTContext *ctx,
33393356
RecursiveTypeProperties properties);
33403357

@@ -3343,7 +3360,7 @@ class GenericFunctionType final : public AnyFunctionType,
33433360
static GenericFunctionType *get(GenericSignature sig,
33443361
ArrayRef<Param> params,
33453362
Type result,
3346-
ExtInfo info = ExtInfo());
3363+
Optional<ExtInfo> info = None);
33473364

33483365
// Retrieve the input parameters of this function type.
33493366
ArrayRef<Param> getParams() const {
@@ -3367,14 +3384,16 @@ class GenericFunctionType final : public AnyFunctionType,
33673384
FunctionType *substGenericArgs(llvm::function_ref<Type(Type)> substFn) const;
33683385

33693386
void Profile(llvm::FoldingSetNodeID &ID) {
3370-
Profile(ID, getGenericSignature(), getParams(), getResult(),
3371-
getExtInfo());
3387+
Optional<ExtInfo> info = None;
3388+
if (hasExtInfo())
3389+
info = getExtInfo();
3390+
Profile(ID, getGenericSignature(), getParams(), getResult(), info);
33723391
}
33733392
static void Profile(llvm::FoldingSetNodeID &ID,
33743393
GenericSignature sig,
33753394
ArrayRef<Param> params,
33763395
Type result,
3377-
ExtInfo info);
3396+
Optional<ExtInfo> info);
33783397

33793398
// Implement isa/cast/dyncast/etc.
33803399
static bool classof(const TypeBase *T) {
@@ -3387,11 +3406,11 @@ BEGIN_CAN_TYPE_WRAPPER(GenericFunctionType, AnyFunctionType)
33873406
static CanGenericFunctionType get(CanGenericSignature sig,
33883407
CanParamArrayRef params,
33893408
CanType result,
3390-
ExtInfo info = ExtInfo()) {
3409+
Optional<ExtInfo> info = None) {
33913410
// Knowing that the argument types are independently canonical is
33923411
// not sufficient to guarantee that the function type will be canonical.
3393-
auto fnType = GenericFunctionType::get(sig, params.getOriginalArray(),
3394-
result, info);
3412+
auto fnType =
3413+
GenericFunctionType::get(sig, params.getOriginalArray(), result, info);
33953414
return cast<GenericFunctionType>(fnType->getCanonicalType());
33963415
}
33973416

@@ -3413,7 +3432,7 @@ END_CAN_TYPE_WRAPPER(GenericFunctionType, AnyFunctionType)
34133432

34143433
inline CanAnyFunctionType
34153434
CanAnyFunctionType::get(CanGenericSignature signature, CanParamArrayRef params,
3416-
CanType result, ExtInfo extInfo) {
3435+
CanType result, Optional<ExtInfo> extInfo) {
34173436
if (signature) {
34183437
return CanGenericFunctionType::get(signature, params, result, extInfo);
34193438
} else {

‎lib/AST/ASTContext.cpp

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3245,18 +3245,19 @@ static void profileParams(llvm::FoldingSetNodeID &ID,
32453245
}
32463246

32473247
void FunctionType::Profile(llvm::FoldingSetNodeID &ID,
3248-
ArrayRef<AnyFunctionType::Param> params,
3249-
Type result,
3250-
ExtInfo info) {
3248+
ArrayRef<AnyFunctionType::Param> params, Type result,
3249+
Optional<ExtInfo> info) {
32513250
profileParams(ID, params);
32523251
ID.AddPointer(result.getPointer());
3253-
auto infoKey = info.getFuncAttrKey();
3254-
ID.AddInteger(infoKey.first);
3255-
ID.AddPointer(infoKey.second);
3252+
if (info.hasValue()) {
3253+
auto infoKey = info.getValue().getFuncAttrKey();
3254+
ID.AddInteger(infoKey.first);
3255+
ID.AddPointer(infoKey.second);
3256+
}
32563257
}
32573258

32583259
FunctionType *FunctionType::get(ArrayRef<AnyFunctionType::Param> params,
3259-
Type result, ExtInfo info) {
3260+
Type result, Optional<ExtInfo> info) {
32603261
auto properties = getFunctionRecursiveProperties(params, result);
32613262
auto arena = getArena(properties);
32623263

@@ -3272,10 +3273,15 @@ FunctionType *FunctionType::get(ArrayRef<AnyFunctionType::Param> params,
32723273
return funcTy;
32733274
}
32743275

3275-
auto clangTypeInfo = info.getClangTypeInfo();
3276+
ClangTypeInfo clangTypeInfo;
3277+
if (info.hasValue())
3278+
clangTypeInfo = info.getValue().getClangTypeInfo();
3279+
3280+
bool hasClangInfo =
3281+
info.hasValue() && !info.getValue().getClangTypeInfo().empty();
32763282

32773283
size_t allocSize = totalSizeToAlloc<AnyFunctionType::Param, ClangTypeInfo>(
3278-
params.size(), clangTypeInfo.empty() ? 0 : 1);
3284+
params.size(), hasClangInfo ? 1 : 0);
32793285
void *mem = ctx.Allocate(allocSize, alignof(FunctionType), arena);
32803286

32813287
bool isCanonical = isFunctionTypeCanonical(params, result);
@@ -3294,36 +3300,38 @@ FunctionType *FunctionType::get(ArrayRef<AnyFunctionType::Param> params,
32943300
}
32953301

32963302
// If the input and result types are canonical, then so is the result.
3297-
FunctionType::FunctionType(ArrayRef<AnyFunctionType::Param> params,
3298-
Type output, ExtInfo info,
3299-
const ASTContext *ctx,
3303+
FunctionType::FunctionType(ArrayRef<AnyFunctionType::Param> params, Type output,
3304+
Optional<ExtInfo> info, const ASTContext *ctx,
33003305
RecursiveTypeProperties properties)
3301-
: AnyFunctionType(TypeKind::Function, ctx,
3302-
output, properties, params.size(), info) {
3306+
: AnyFunctionType(TypeKind::Function, ctx, output, properties,
3307+
params.size(), info) {
33033308
std::uninitialized_copy(params.begin(), params.end(),
33043309
getTrailingObjects<AnyFunctionType::Param>());
3305-
auto clangTypeInfo = info.getClangTypeInfo();
3306-
if (!clangTypeInfo.empty())
3307-
*getTrailingObjects<ClangTypeInfo>() = clangTypeInfo;
3310+
if (info.hasValue()) {
3311+
auto clangTypeInfo = info.getValue().getClangTypeInfo();
3312+
if (!clangTypeInfo.empty())
3313+
*getTrailingObjects<ClangTypeInfo>() = clangTypeInfo;
3314+
}
33083315
}
33093316

33103317
void GenericFunctionType::Profile(llvm::FoldingSetNodeID &ID,
33113318
GenericSignature sig,
33123319
ArrayRef<AnyFunctionType::Param> params,
3313-
Type result,
3314-
ExtInfo info) {
3320+
Type result, Optional<ExtInfo> info) {
33153321
ID.AddPointer(sig.getPointer());
33163322
profileParams(ID, params);
33173323
ID.AddPointer(result.getPointer());
3318-
auto infoKey = info.getFuncAttrKey();
3319-
ID.AddInteger(infoKey.first);
3320-
ID.AddPointer(infoKey.second);
3324+
if (info.hasValue()) {
3325+
auto infoKey = info.getValue().getFuncAttrKey();
3326+
ID.AddInteger(infoKey.first);
3327+
ID.AddPointer(infoKey.second);
3328+
}
33213329
}
33223330

33233331
GenericFunctionType *GenericFunctionType::get(GenericSignature sig,
33243332
ArrayRef<Param> params,
33253333
Type result,
3326-
ExtInfo info) {
3334+
Optional<ExtInfo> info) {
33273335
assert(sig && "no generic signature for generic function type?!");
33283336
assert(!result->hasTypeVariable());
33293337
assert(!result->hasPlaceholder());
@@ -3346,7 +3354,7 @@ GenericFunctionType *GenericFunctionType::get(GenericSignature sig,
33463354
// point.
33473355
bool isCanonical = isGenericFunctionTypeCanonical(sig, params, result);
33483356

3349-
assert(info.getClangTypeInfo().empty() &&
3357+
assert((!info.hasValue() || info.getValue().getClangTypeInfo().empty()) &&
33503358
"Generic functions do not have Clang types at the moment.");
33513359

33523360
if (auto funcTy
@@ -3370,7 +3378,7 @@ GenericFunctionType::GenericFunctionType(
33703378
GenericSignature sig,
33713379
ArrayRef<AnyFunctionType::Param> params,
33723380
Type result,
3373-
ExtInfo info,
3381+
Optional<ExtInfo> info,
33743382
const ASTContext *ctx,
33753383
RecursiveTypeProperties properties)
33763384
: AnyFunctionType(TypeKind::GenericFunction, ctx, result,

‎lib/AST/ASTMangler.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2846,9 +2846,12 @@ CanType ASTMangler::getDeclTypeForMangling(
28462846

28472847
auto &C = decl->getASTContext();
28482848
if (decl->isInvalid()) {
2849-
if (isa<AbstractFunctionDecl>(decl))
2849+
if (isa<AbstractFunctionDecl>(decl)) {
2850+
// FIXME: Verify ExtInfo state is correct, not working by accident.
2851+
CanFunctionType::ExtInfo info;
28502852
return CanFunctionType::get({AnyFunctionType::Param(C.TheErrorType)},
2851-
C.TheErrorType);
2853+
C.TheErrorType, info);
2854+
}
28522855
return C.TheErrorType;
28532856
}
28542857

‎lib/AST/ASTPrinter.cpp

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4474,6 +4474,10 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
44744474
}
44754475

44764476
void printFunctionExtInfo(AnyFunctionType *fnType) {
4477+
if (!fnType->hasExtInfo()) {
4478+
Printer << "@_NO_EXTINFO ";
4479+
return;
4480+
}
44774481
auto &ctx = fnType->getASTContext();
44784482
auto info = fnType->getExtInfo();
44794483
if (Options.SkipAttributes)
@@ -4693,13 +4697,15 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
46934697
// If we're stripping argument labels from types, do it when printing.
46944698
visitAnyFunctionTypeParams(T->getParams(), /*printLabels*/false);
46954699

4696-
if (T->isAsync()) {
4697-
Printer << " ";
4698-
Printer.printKeyword("async", Options);
4699-
}
4700+
if (T->hasExtInfo()) {
4701+
if (T->isAsync()) {
4702+
Printer << " ";
4703+
Printer.printKeyword("async", Options);
4704+
}
47004705

4701-
if (T->isThrowing())
4702-
Printer << " " << tok::kw_throws;
4706+
if (T->isThrowing())
4707+
Printer << " " << tok::kw_throws;
4708+
}
47034709

47044710
Printer << " -> ";
47054711

@@ -4738,13 +4744,15 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
47384744

47394745
visitAnyFunctionTypeParams(T->getParams(), /*printLabels*/true);
47404746

4741-
if (T->isAsync()) {
4742-
Printer << " ";
4743-
Printer.printKeyword("async", Options);
4744-
}
4747+
if (T->hasExtInfo()) {
4748+
if (T->isAsync()) {
4749+
Printer << " ";
4750+
Printer.printKeyword("async", Options);
4751+
}
47454752

4746-
if (T->isThrowing())
4747-
Printer << " " << tok::kw_throws;
4753+
if (T->isThrowing())
4754+
Printer << " " << tok::kw_throws;
4755+
}
47484756

47494757
Printer << " -> ";
47504758
Printer.callPrintStructurePre(PrintStructureKind::FunctionReturnType);

0 commit comments

Comments
 (0)