Skip to content

Commit 39a22f3

Browse files
slavapestovslavapestov
committed
AST: Remove ParameterTypeFlags::Escaping
Escapingness is a property of the type of a value, not a property of a function parameter. Having it as a separate parameter flag just meant one more piece of state that could get out of sync and cause weird problems. Instead, always look at the noescape bit in a function type as the canonical source of truth. This does mean that '@escaping' is now printed in a few diagnostics where it was not printed before; we can investigate these as separate issues, but it is correct to print it there because the function types in question are, in fact, escaping. Fixes <https://bugs.swift.org/browse/SR-10256>, <rdar://problem/49522774>.
1 parent 91dffc9 commit 39a22f3

19 files changed

+72
-85
lines changed

‎include/swift/AST/Types.h

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,11 +1718,10 @@ class ParameterTypeFlags {
17181718
None = 0,
17191719
Variadic = 1 << 0,
17201720
AutoClosure = 1 << 1,
1721-
Escaping = 1 << 2,
1722-
OwnershipShift = 3,
1721+
OwnershipShift = 2,
17231722
Ownership = 7 << OwnershipShift,
17241723

1725-
NumBits = 6
1724+
NumBits = 5
17261725
};
17271726
OptionSet<ParameterFlags> value;
17281727
static_assert(NumBits < 8*sizeof(OptionSet<ParameterFlags>), "overflowed");
@@ -1735,10 +1734,9 @@ class ParameterTypeFlags {
17351734
return ParameterTypeFlags(OptionSet<ParameterFlags>(raw));
17361735
}
17371736

1738-
ParameterTypeFlags(bool variadic, bool autoclosure, bool escaping,
1737+
ParameterTypeFlags(bool variadic, bool autoclosure,
17391738
ValueOwnership ownership)
17401739
: value((variadic ? Variadic : 0) | (autoclosure ? AutoClosure : 0) |
1741-
(escaping ? Escaping : 0) |
17421740
uint8_t(ownership) << OwnershipShift) {}
17431741

17441742
/// Create one from what's present in the parameter type
@@ -1749,7 +1747,6 @@ class ParameterTypeFlags {
17491747
bool isNone() const { return !value; }
17501748
bool isVariadic() const { return value.contains(Variadic); }
17511749
bool isAutoClosure() const { return value.contains(AutoClosure); }
1752-
bool isEscaping() const { return value.contains(Escaping); }
17531750
bool isInOut() const { return getValueOwnership() == ValueOwnership::InOut; }
17541751
bool isShared() const { return getValueOwnership() == ValueOwnership::Shared;}
17551752
bool isOwned() const { return getValueOwnership() == ValueOwnership::Owned; }
@@ -1763,11 +1760,6 @@ class ParameterTypeFlags {
17631760
: value - ParameterTypeFlags::Variadic);
17641761
}
17651762

1766-
ParameterTypeFlags withEscaping(bool escaping) const {
1767-
return ParameterTypeFlags(escaping ? value | ParameterTypeFlags::Escaping
1768-
: value - ParameterTypeFlags::Escaping);
1769-
}
1770-
17711763
ParameterTypeFlags withInOut(bool isInout) const {
17721764
return withValueOwnership(isInout ? ValueOwnership::InOut
17731765
: ValueOwnership::Default);
@@ -1860,7 +1852,6 @@ class YieldTypeFlags {
18601852
ParameterTypeFlags asParamFlags() const {
18611853
return ParameterTypeFlags(/*variadic*/ false,
18621854
/*autoclosure*/ false,
1863-
/*escaping*/ false,
18641855
getValueOwnership());
18651856
}
18661857

@@ -1931,9 +1922,6 @@ class TupleTypeElt {
19311922
/// Determine whether this field is an autoclosure parameter closure.
19321923
bool isAutoClosure() const { return Flags.isAutoClosure(); }
19331924

1934-
/// Determine whether this field is an escaping parameter closure.
1935-
bool isEscaping() const { return Flags.isEscaping(); }
1936-
19371925
/// Determine whether this field is marked 'inout'.
19381926
bool isInOut() const { return Flags.isInOut(); }
19391927

@@ -2724,9 +2712,6 @@ class AnyFunctionType : public TypeBase {
27242712
/// Whether the parameter is marked '@autoclosure'
27252713
bool isAutoClosure() const { return Flags.isAutoClosure(); }
27262714

2727-
/// Whether the parameter is marked '@escaping'
2728-
bool isEscaping() const { return Flags.isEscaping(); }
2729-
27302715
/// Whether the parameter is marked 'inout'
27312716
bool isInOut() const { return Flags.isInOut(); }
27322717

@@ -5374,8 +5359,6 @@ inline ParameterTypeFlags
53745359
ParameterTypeFlags::fromParameterType(Type paramTy, bool isVariadic,
53755360
bool isAutoClosure,
53765361
ValueOwnership ownership) {
5377-
bool escaping = paramTy->is<AnyFunctionType>() &&
5378-
!paramTy->castTo<AnyFunctionType>()->isNoEscape();
53795362
// FIXME(Remove InOut): The last caller that needs this is argument
53805363
// decomposition. Start by enabling the assertion there and fixing up those
53815364
// callers, then remove this, then remove
@@ -5385,7 +5368,7 @@ ParameterTypeFlags::fromParameterType(Type paramTy, bool isVariadic,
53855368
ownership == ValueOwnership::InOut);
53865369
ownership = ValueOwnership::InOut;
53875370
}
5388-
return {isVariadic, isAutoClosure, escaping, ownership};
5371+
return {isVariadic, isAutoClosure, ownership};
53895372
}
53905373

53915374
inline const Type *BoundGenericType::getTrailingObjectsPointer() const {

‎include/swift/Serialization/ModuleFormat.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
5252
/// describe what change you made. The content of this comment isn't important;
5353
/// it just ensures a conflict if two people change the module format.
5454
/// Don't worry about adhering to the 80-column limit for this line.
55-
const uint16_t SWIFTMODULE_VERSION_MINOR = 484; // SDK-relative dependencies flag
55+
const uint16_t SWIFTMODULE_VERSION_MINOR = 485; // remove @escaping parameter flag
5656

5757
using DeclIDField = BCFixed<31>;
5858

@@ -765,7 +765,6 @@ namespace decls_block {
765765
TypeIDField, // type
766766
BCFixed<1>, // vararg?
767767
BCFixed<1>, // autoclosure?
768-
BCFixed<1>, // escaping?
769768
ValueOwnershipField // inout, shared or owned?
770769
>;
771770

‎lib/AST/ASTDemangler.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -361,10 +361,6 @@ Type ASTBuilder::createFunctionType(
361361
.withVariadic(flags.isVariadic())
362362
.withAutoClosure(flags.isAutoClosure());
363363

364-
if (auto *fnType = type->getAs<FunctionType>())
365-
if (!fnType->isNoEscape())
366-
parameterFlags = parameterFlags.withEscaping(true);
367-
368364
funcParams.push_back(AnyFunctionType::Param(type, label, parameterFlags));
369365
}
370366

‎lib/AST/ASTDumper.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3204,7 +3204,6 @@ namespace {
32043204
void dumpParameterFlags(ParameterTypeFlags paramFlags) {
32053205
printFlag(paramFlags.isVariadic(), "vararg");
32063206
printFlag(paramFlags.isAutoClosure(), "autoclosure");
3207-
printFlag(paramFlags.isEscaping(), "escaping");
32083207
switch (paramFlags.getValueOwnership()) {
32093208
case ValueOwnership::Default: break;
32103209
case ValueOwnership::Owned: printFlag("owned"); break;

‎lib/AST/ASTMangler.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1905,11 +1905,9 @@ void ASTMangler::appendTypeList(Type listTy) {
19051905
return appendOperator("y");
19061906
bool firstField = true;
19071907
for (auto &field : tuple->getElements()) {
1908-
// FIXME: We shouldn't put @escaping in non-parameter list tuples
1909-
auto flags = field.getParameterFlags().withEscaping(false);
1910-
1911-
assert(flags.isNone());
1912-
appendTypeListElement(field.getName(), field.getRawType(), flags);
1908+
assert(field.getParameterFlags().isNone());
1909+
appendTypeListElement(field.getName(), field.getRawType(),
1910+
ParameterTypeFlags());
19131911
appendListSeparator(firstField);
19141912
}
19151913
} else {

‎lib/AST/ASTPrinter.cpp

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2424,11 +2424,26 @@ static bool isStructOrClassContext(DeclContext *dc) {
24242424
return isa<ClassDecl>(nominal) || isa<StructDecl>(nominal);
24252425
}
24262426

2427+
static bool isEscaping(Type type) {
2428+
if (auto *funcType = type->getAs<AnyFunctionType>()) {
2429+
if (funcType->getExtInfo().getRepresentation() ==
2430+
FunctionTypeRepresentation::CFunctionPointer)
2431+
return false;
2432+
2433+
if (funcType->getExtInfo().isNoEscape())
2434+
return false;
2435+
2436+
return true;
2437+
}
2438+
2439+
return false;
2440+
}
2441+
24272442
static void printParameterFlags(ASTPrinter &printer, PrintOptions options,
2428-
ParameterTypeFlags flags) {
2443+
ParameterTypeFlags flags, bool escaping) {
24292444
if (!options.excludeAttrKind(TAK_autoclosure) && flags.isAutoClosure())
24302445
printer << "@autoclosure ";
2431-
if (!options.excludeAttrKind(TAK_escaping) && flags.isEscaping())
2446+
if (!options.excludeAttrKind(TAK_escaping) && escaping)
24322447
printer << "@escaping ";
24332448

24342449
switch (flags.getValueOwnership()) {
@@ -2561,29 +2576,17 @@ void PrintAST::printOneParameter(const ParamDecl *param,
25612576
TheTypeLoc.setType(BGT->getGenericArgs()[0]);
25622577
}
25632578

2564-
// Special case, if we're not going to use the type repr printing, peek
2565-
// through the paren types so that we don't print excessive @escapings.
2566-
unsigned numParens = 0;
2567-
if (!willUseTypeReprPrinting(TheTypeLoc, CurrentType, Options)) {
2579+
if (!param->isVariadic() &&
2580+
!willUseTypeReprPrinting(TheTypeLoc, CurrentType, Options)) {
25682581
auto type = TheTypeLoc.getType();
2569-
2570-
printParameterFlags(Printer, Options, paramFlags);
2571-
while (auto parenTy = dyn_cast<ParenType>(type.getPointer())) {
2572-
++numParens;
2573-
type = parenTy->getUnderlyingType();
2574-
}
2575-
2576-
TheTypeLoc = TypeLoc::withoutLoc(type);
2582+
printParameterFlags(Printer, Options, paramFlags,
2583+
isEscaping(type));
25772584
}
25782585

2579-
for (unsigned i = 0; i < numParens; ++i)
2580-
Printer << "(";
25812586
if (param->getAttrs().hasAttribute<ImplicitlyUnwrappedOptionalAttr>())
25822587
printTypeLocForImplicitlyUnwrappedOptional(TheTypeLoc);
25832588
else
25842589
printTypeLoc(TheTypeLoc);
2585-
for (unsigned i = 0; i < numParens; ++i)
2586-
Printer << ")";
25872590

25882591
if (param->isVariadic())
25892592
Printer << "...";
@@ -3553,7 +3556,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
35533556

35543557
void visitParenType(ParenType *T) {
35553558
Printer << "(";
3556-
printParameterFlags(Printer, Options, T->getParameterFlags());
3559+
printParameterFlags(Printer, Options, T->getParameterFlags(), false);
35573560
visit(T->getUnderlyingType()->getInOutObjectType());
35583561
Printer << ")";
35593562
}
@@ -3584,7 +3587,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
35843587
visit(TD.getVarargBaseTy());
35853588
Printer << "...";
35863589
} else {
3587-
printParameterFlags(Printer, Options, TD.getParameterFlags());
3590+
printParameterFlags(Printer, Options, TD.getParameterFlags(), false);
35883591
visit(EltType);
35893592
}
35903593
}
@@ -3841,10 +3844,15 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
38413844
Printer << ": ";
38423845
}
38433846

3844-
printParameterFlags(Printer, Options, Param.getParameterFlags());
3845-
visit(Param.getPlainType());
3846-
if (Param.isVariadic())
3847+
auto type = Param.getPlainType();
3848+
if (Param.isVariadic()) {
3849+
visit(type);
38473850
Printer << "...";
3851+
} else {
3852+
printParameterFlags(Printer, Options, Param.getParameterFlags(),
3853+
isEscaping(type));
3854+
visit(type);
3855+
}
38483856
}
38493857

38503858
Printer << ")";

‎lib/AST/Decl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2247,7 +2247,7 @@ static Type mapSignatureFunctionType(ASTContext &ctx, Type type,
22472247
SmallVector<AnyFunctionType::Param, 4> newParams;
22482248
for (const auto &param : funcTy->getParams()) {
22492249
auto newParamType = mapSignatureParamType(ctx, param.getPlainType());
2250-
ParameterTypeFlags newFlags = param.getParameterFlags().withEscaping(false);
2250+
ParameterTypeFlags newFlags = param.getParameterFlags();
22512251

22522252
// For the 'self' of a method, strip off 'inout'.
22532253
if (isMethod) {

‎lib/SIL/SILFunctionType.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2155,9 +2155,8 @@ static CanType copyOptionalityFromDerivedToBase(TypeConverter &tc,
21552155
auto baseParams = baseFunc.getParams();
21562156
assert(derivedParams.size() == baseParams.size());
21572157
for (unsigned i = 0, e = derivedParams.size(); i < e; i++) {
2158-
// FIXME: Why are 'escaping' flags set inconsistently?
2159-
assert(derivedParams[i].getParameterFlags().withEscaping(false) ==
2160-
baseParams[i].getParameterFlags().withEscaping(false));
2158+
assert(derivedParams[i].getParameterFlags() ==
2159+
baseParams[i].getParameterFlags());
21612160

21622161
params.emplace_back(
21632162
copyOptionalityFromDerivedToBase(

‎lib/Serialization/Deserialization.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4514,11 +4514,11 @@ class swift::TypeDeserializer {
45144514

45154515
IdentifierID labelID;
45164516
TypeID typeID;
4517-
bool isVariadic, isAutoClosure, isEscaping;
4517+
bool isVariadic, isAutoClosure;
45184518
unsigned rawOwnership;
45194519
decls_block::FunctionParamLayout::readRecord(scratch, labelID, typeID,
45204520
isVariadic, isAutoClosure,
4521-
isEscaping, rawOwnership);
4521+
rawOwnership);
45224522

45234523
auto ownership =
45244524
getActualValueOwnership((serialization::ValueOwnership)rawOwnership);
@@ -4534,7 +4534,7 @@ class swift::TypeDeserializer {
45344534
params.emplace_back(paramTy.get(),
45354535
MF.getIdentifier(labelID),
45364536
ParameterTypeFlags(isVariadic, isAutoClosure,
4537-
isEscaping, *ownership));
4537+
*ownership));
45384538
}
45394539

45404540
if (!isGeneric) {

‎lib/Serialization/Serialization.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3953,7 +3953,7 @@ void Serializer::writeType(Type ty) {
39533953
FunctionParamLayout::emitRecord(
39543954
Out, ScratchRecord, abbrCode, addDeclBaseNameRef(param.getLabel()),
39553955
addTypeRef(param.getPlainType()), paramFlags.isVariadic(),
3956-
paramFlags.isAutoClosure(), paramFlags.isEscaping(), rawOwnership);
3956+
paramFlags.isAutoClosure(), rawOwnership);
39573957
}
39583958

39593959
break;

0 commit comments

Comments
 (0)