Skip to content

Commit 0bb868a

Browse files
milsemanmilseman
committed
[ParameterTypeFlags] Incorporate review feedback
1 parent 8923a12 commit 0bb868a

File tree

10 files changed

+65
-53
lines changed

10 files changed

+65
-53
lines changed

‎include/swift/AST/Types.h

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,21 +1295,22 @@ class NameAliasType : public TypeBase {
12951295
//
12961296
/// Provide parameter type relevant flags, i.e. variadic, autoclosure, and
12971297
/// escaping.
1298-
struct ParameterTypeFlags {
1299-
uint8_t value;
1300-
enum : uint8_t {
1298+
class ParameterTypeFlags {
1299+
enum ParameterFlags : uint8_t {
13011300
None = 0,
13021301
Variadic = 1 << 0,
13031302
AutoClosure = 1 << 1,
13041303
Escaping = 1 << 2,
13051304

13061305
NumBits = 3
13071306
};
1307+
OptionSet<ParameterFlags> value;
13081308
static_assert(NumBits < 8*sizeof(value), "overflowed");
13091309

1310-
ParameterTypeFlags() : value(None) {}
1310+
ParameterTypeFlags(OptionSet<ParameterFlags, uint8_t> val) : value(val) {}
13111311

1312-
ParameterTypeFlags(uint8_t val) : value(val) {}
1312+
public:
1313+
ParameterTypeFlags() = default;
13131314

13141315
ParameterTypeFlags(bool variadic, bool autoclosure, bool escaping)
13151316
: value((variadic ? Variadic : 0) |
@@ -1320,13 +1321,21 @@ struct ParameterTypeFlags {
13201321
inline static ParameterTypeFlags fromParameterType(Type paramTy,
13211322
bool isVariadic);
13221323

1323-
bool isVariadic() const { return 0 != (value & Variadic); }
1324-
bool isAutoClosure() const { return 0 != (value & AutoClosure); }
1325-
bool isEscaping() const { return 0 != (value & Escaping); }
1324+
bool isNone() const { return !value; }
1325+
bool isVariadic() const { return value.contains(Variadic); }
1326+
bool isAutoClosure() const { return value.contains(AutoClosure); }
1327+
bool isEscaping() const { return value.contains(Escaping); }
1328+
1329+
ParameterTypeFlags withEscaping(bool escaping) const {
1330+
return ParameterTypeFlags(escaping ? value | ParameterTypeFlags::Escaping
1331+
: value - ParameterTypeFlags::Escaping);
1332+
}
13261333

1327-
bool operator==(const ParameterTypeFlags &other) {
1328-
return value == other.value;
1334+
bool operator ==(const ParameterTypeFlags &other) const {
1335+
return value.toRaw() == other.value.toRaw();
13291336
}
1337+
1338+
uint8_t toRaw() const { return value.toRaw(); }
13301339
};
13311340

13321341
/// ParenType - A paren type is a type that's been written in parentheses.
@@ -1411,21 +1420,16 @@ class TupleTypeElt {
14111420

14121421
/// Retrieve a copy of this tuple type element with the type replaced.
14131422
TupleTypeElt getWithType(Type T) const {
1414-
return TupleTypeElt(T, getName(), isVararg(), isAutoClosure(),
1415-
isEscaping());
1423+
return TupleTypeElt(T, getName(), getParameterFlags());
14161424
}
14171425

14181426
/// Retrieve a copy of this tuple type element with the name replaced.
1419-
TupleTypeElt getWithName(Identifier name = Identifier()) const {
1420-
return TupleTypeElt(getType(), name, isVararg(), isAutoClosure(),
1421-
isEscaping());
1427+
TupleTypeElt getWithName(Identifier name) const {
1428+
return TupleTypeElt(getType(), name, getParameterFlags());
14221429
}
14231430

1424-
/// Retrieve a copy of this tuple type element with the type and name
1425-
/// replaced.
1426-
TupleTypeElt getWithTypeAndName(Type T, Identifier name) const {
1427-
return TupleTypeElt(T, name, isVararg(), isAutoClosure(), isEscaping());
1428-
}
1431+
/// Retrieve a copy of this tuple type element with no name
1432+
TupleTypeElt getWithoutName() const { return getWithName(Identifier()); }
14291433
};
14301434

14311435
inline Type getTupleEltType(const TupleTypeElt &elt) {

‎include/swift/Serialization/ModuleFormat.h

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ const uint16_t VERSION_MAJOR = 0;
5454
/// in source control, you should also update the comment to briefly
5555
/// describe what change you made. The content of this comment isn't important;
5656
/// it just ensures a conflict if two people change the module format.
57-
const uint16_t VERSION_MINOR = 268; // Last change: parameter type flags
57+
const uint16_t VERSION_MINOR = 269; // Last change: parameter type flags
5858

5959
using DeclID = PointerEmbeddedInt<unsigned, 31>;
6060
using DeclIDField = BCFixed<31>;
@@ -571,8 +571,10 @@ namespace decls_block {
571571

572572
using ParenTypeLayout = BCRecordLayout<
573573
PAREN_TYPE,
574-
TypeIDField, // inner type
575-
BCFixed<ParameterTypeFlags::NumBits> // vararg?, autoclosure?, escaping?
574+
TypeIDField, // inner type
575+
BCFixed<1>, // vararg?
576+
BCFixed<1>, // autoclosure?
577+
BCFixed<1> // escaping?
576578
>;
577579

578580
using TupleTypeLayout = BCRecordLayout<
@@ -581,9 +583,11 @@ namespace decls_block {
581583

582584
using TupleTypeEltLayout = BCRecordLayout<
583585
TUPLE_TYPE_ELT,
584-
IdentifierIDField, // name
585-
TypeIDField, // type
586-
BCFixed<ParameterTypeFlags::NumBits> // vararg?, autoclosure?, escaping?
586+
IdentifierIDField, // name
587+
TypeIDField, // type
588+
BCFixed<1>, // vararg?
589+
BCFixed<1>, // autoclosure?
590+
BCFixed<1> // escaping?
587591
>;
588592

589593
using FunctionTypeLayout = BCRecordLayout<

‎lib/AST/ASTContext.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2602,7 +2602,7 @@ ParenType *ParenType::get(const ASTContext &C, Type underlying,
26022602
auto properties = underlying->getRecursiveProperties();
26032603
auto arena = getArena(properties);
26042604
ParenType *&Result =
2605-
C.Impl.getArena(arena).ParenTypes[{underlying, flags.value}];
2605+
C.Impl.getArena(arena).ParenTypes[{underlying, flags.toRaw()}];
26062606
if (Result == 0) {
26072607
Result = new (C, arena) ParenType(underlying, properties, flags);
26082608
}
@@ -2619,7 +2619,7 @@ void TupleType::Profile(llvm::FoldingSetNodeID &ID,
26192619
for (const TupleTypeElt &Elt : Fields) {
26202620
ID.AddPointer(Elt.Name.get());
26212621
ID.AddPointer(Elt.getType().getPointer());
2622-
ID.AddInteger(Elt.Flags.value);
2622+
ID.AddInteger(Elt.Flags.toRaw());
26232623
}
26242624
}
26252625

‎lib/AST/ASTPrinter.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2640,7 +2640,7 @@ void PrintAST::printOneParameter(const ParamDecl *param,
26402640
printParameterFlags(Printer, Options, paramFlags);
26412641

26422642
// Special case, if we're not going to use the type repr printing, peek
2643-
// through the paren types so that we don't print excessive @escapings
2643+
// through the paren types so that we don't print excessive @escapings.
26442644
unsigned numParens = 0;
26452645
if (!willUseTypeReprPrinting(TheTypeLoc, Options)) {
26462646
while (auto parenTy =
@@ -2727,7 +2727,7 @@ void PrintAST::printFunctionParameters(AbstractFunctionDecl *AFD) {
27272727
}
27282728

27292729
SmallVector<Type, 4> parameterListTypes;
2730-
for (auto i = 0; i < BodyParams.size(); ++i) {
2730+
for (unsigned i = 0; i < BodyParams.size(); ++i) {
27312731
if (curTy) {
27322732
if (auto funTy = curTy->getAs<AnyFunctionType>()) {
27332733
parameterListTypes.push_back(funTy->getInput());
@@ -2742,7 +2742,7 @@ void PrintAST::printFunctionParameters(AbstractFunctionDecl *AFD) {
27422742
for (unsigned CurrPattern = 0, NumPatterns = BodyParams.size();
27432743
CurrPattern != NumPatterns; ++CurrPattern) {
27442744
// Be extra careful in the event of printing mal-formed ASTs
2745-
auto paramListType = parameterListTypes.size() > CurrPattern
2745+
auto paramListType = CurrPattern < parameterListTypes.size()
27462746
? parameterListTypes[CurrPattern]
27472747
: nullptr;
27482748
printParameterList(BodyParams[CurrPattern], paramListType,
@@ -3985,7 +3985,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
39853985
SmallVector<TupleTypeElt, 4> elements;
39863986
elements.reserve(tupleTy->getNumElements());
39873987
for (const auto &elt : tupleTy->getElements())
3988-
elements.push_back(elt.getWithName());
3988+
elements.push_back(elt.getWithoutName());
39893989
inputType = TupleType::get(elements, inputType->getASTContext());
39903990
}
39913991

‎lib/AST/Decl.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,9 +1470,7 @@ static Type mapSignatureFunctionType(ASTContext &ctx, Type type,
14701470
// Remap our parameters, and make sure to strip off @escaping
14711471
for (const auto &elt : tupleTy->getElements()) {
14721472
auto newEltTy = mapSignatureParamType(ctx, elt.getType());
1473-
ParameterTypeFlags newParamFlags(
1474-
elt.getParameterFlags().value & ~ParameterTypeFlags::Escaping);
1475-
1473+
auto newParamFlags = elt.getParameterFlags().withEscaping(false);
14761474
bool exactlyTheSame = newParamFlags == elt.getParameterFlags() &&
14771475
newEltTy.getPointer() == elt.getType().getPointer();
14781476

@@ -1489,7 +1487,6 @@ static Type mapSignatureFunctionType(ASTContext &ctx, Type type,
14891487
anyChanged = true;
14901488
}
14911489

1492-
// TODO: drop the name when it's finally out of the type system
14931490
elements.emplace_back(newEltTy, elt.getName(), newParamFlags);
14941491
}
14951492
if (anyChanged) {

‎lib/AST/Type.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -761,7 +761,7 @@ static Type getStrippedType(const ASTContext &context, Type type,
761761
}
762762

763763
Identifier newName = stripLabels? Identifier() : elt.getName();
764-
elements.push_back(elt.getWithTypeAndName(eltTy, newName));
764+
elements.emplace_back(eltTy, newName, elt.getParameterFlags());
765765
}
766766
++idx;
767767
}
@@ -850,7 +850,7 @@ swift::decomposeArgType(Type type, ArrayRef<Identifier> argumentLabels) {
850850

851851
for (auto i : range(0, tupleTy->getNumElements())) {
852852
const auto &elt = tupleTy->getElement(i);
853-
assert(elt.getParameterFlags().value == 0 &&
853+
assert(elt.getParameterFlags().isNone() &&
854854
"Vararg, autoclosure, or escaping argument tuple"
855855
"doesn't make sense");
856856
CallArgParam argParam;

‎lib/Sema/ConstraintSystem.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,7 @@ static Type removeArgumentLabels(Type type, unsigned numArgumentLabels) {
623623
SmallVector<TupleTypeElt, 4> elements;
624624
elements.reserve(tupleTy->getNumElements());
625625
for (const auto &elt : tupleTy->getElements()) {
626-
elements.push_back(elt.getWithName());
626+
elements.push_back(elt.getWithoutName());
627627
}
628628
inputType = TupleType::get(elements, type->getASTContext());
629629
}

‎lib/Sema/TypeCheckType.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2577,7 +2577,7 @@ Type TypeResolver::resolveTupleType(TupleTypeRepr *repr,
25772577

25782578
auto paramFlags = isImmediateFunctionInput
25792579
? ParameterTypeFlags::fromParameterType(ty, variadic)
2580-
: ParameterTypeFlags(variadic, false, false);
2580+
: ParameterTypeFlags();
25812581
elements.emplace_back(ty, name, paramFlags);
25822582
}
25832583

‎lib/Serialization/Deserialization.cpp

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3446,10 +3446,12 @@ Type ModuleFile::getType(TypeID TID) {
34463446

34473447
case decls_block::PAREN_TYPE: {
34483448
TypeID underlyingID;
3449-
uint8_t flagsValue;
3450-
decls_block::ParenTypeLayout::readRecord(scratch, underlyingID, flagsValue);
3451-
typeOrOffset = ParenType::get(ctx, getType(underlyingID),
3452-
ParameterTypeFlags(flagsValue));
3449+
bool isVariadic, isAutoClosure, isEscaping;
3450+
decls_block::ParenTypeLayout::readRecord(scratch, underlyingID, isVariadic,
3451+
isAutoClosure, isEscaping);
3452+
typeOrOffset = ParenType::get(
3453+
ctx, getType(underlyingID),
3454+
ParameterTypeFlags(isVariadic, isAutoClosure, isEscaping));
34533455
break;
34543456
}
34553457

@@ -3469,12 +3471,13 @@ Type ModuleFile::getType(TypeID TID) {
34693471

34703472
IdentifierID nameID;
34713473
TypeID typeID;
3472-
uint8_t flagsValue;
3473-
decls_block::TupleTypeEltLayout::readRecord(scratch, nameID, typeID,
3474-
flagsValue);
3474+
bool isVariadic, isAutoClosure, isEscaping;
3475+
decls_block::TupleTypeEltLayout::readRecord(
3476+
scratch, nameID, typeID, isVariadic, isAutoClosure, isEscaping);
34753477

3476-
elements.emplace_back(getType(typeID), getIdentifier(nameID),
3477-
ParameterTypeFlags(flagsValue));
3478+
elements.emplace_back(
3479+
getType(typeID), getIdentifier(nameID),
3480+
ParameterTypeFlags(isVariadic, isAutoClosure, isEscaping));
34783481
}
34793482

34803483
typeOrOffset = TupleType::get(elements, ctx);

‎lib/Serialization/Serialization.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2823,11 +2823,13 @@ void Serializer::writeType(Type ty) {
28232823

28242824
case TypeKind::Paren: {
28252825
auto parenTy = cast<ParenType>(ty.getPointer());
2826+
auto paramFlags = parenTy->getParameterFlags();
28262827

28272828
unsigned abbrCode = DeclTypeAbbrCodes[ParenTypeLayout::Code];
2828-
ParenTypeLayout::emitRecord(Out, ScratchRecord, abbrCode,
2829-
addTypeRef(parenTy->getUnderlyingType()),
2830-
parenTy->getParameterFlags().value);
2829+
ParenTypeLayout::emitRecord(
2830+
Out, ScratchRecord, abbrCode, addTypeRef(parenTy->getUnderlyingType()),
2831+
paramFlags.isVariadic(), paramFlags.isAutoClosure(),
2832+
paramFlags.isEscaping());
28312833
break;
28322834
}
28332835

@@ -2839,9 +2841,11 @@ void Serializer::writeType(Type ty) {
28392841

28402842
abbrCode = DeclTypeAbbrCodes[TupleTypeEltLayout::Code];
28412843
for (auto &elt : tupleTy->getElements()) {
2844+
auto paramFlags = elt.getParameterFlags();
28422845
TupleTypeEltLayout::emitRecord(
28432846
Out, ScratchRecord, abbrCode, addIdentifierRef(elt.getName()),
2844-
addTypeRef(elt.getType()), elt.getParameterFlags().value);
2847+
addTypeRef(elt.getType()), paramFlags.isVariadic(),
2848+
paramFlags.isAutoClosure(), paramFlags.isEscaping());
28452849
}
28462850

28472851
break;

0 commit comments

Comments
 (0)