Skip to content

Commit b9b1ea6

Browse files
milsemanmilseman
committed
[ParameterTypeFlags] Incorporate review feedback
1 parent 4e462d0 commit b9b1ea6

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
@@ -1283,21 +1283,22 @@ class NameAliasType : public TypeBase {
12831283
//
12841284
/// Provide parameter type relevant flags, i.e. variadic, autoclosure, and
12851285
/// escaping.
1286-
struct ParameterTypeFlags {
1287-
uint8_t value;
1288-
enum : uint8_t {
1286+
class ParameterTypeFlags {
1287+
enum ParameterFlags : uint8_t {
12891288
None = 0,
12901289
Variadic = 1 << 0,
12911290
AutoClosure = 1 << 1,
12921291
Escaping = 1 << 2,
12931292

12941293
NumBits = 3
12951294
};
1295+
OptionSet<ParameterFlags> value;
12961296
static_assert(NumBits < 8*sizeof(value), "overflowed");
12971297

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

1300-
ParameterTypeFlags(uint8_t val) : value(val) {}
1300+
public:
1301+
ParameterTypeFlags() = default;
13011302

13021303
ParameterTypeFlags(bool variadic, bool autoclosure, bool escaping)
13031304
: value((variadic ? Variadic : 0) |
@@ -1308,13 +1309,21 @@ struct ParameterTypeFlags {
13081309
inline static ParameterTypeFlags fromParameterType(Type paramTy,
13091310
bool isVariadic);
13101311

1311-
bool isVariadic() const { return 0 != (value & Variadic); }
1312-
bool isAutoClosure() const { return 0 != (value & AutoClosure); }
1313-
bool isEscaping() const { return 0 != (value & Escaping); }
1312+
bool isNone() const { return !value; }
1313+
bool isVariadic() const { return value.contains(Variadic); }
1314+
bool isAutoClosure() const { return value.contains(AutoClosure); }
1315+
bool isEscaping() const { return value.contains(Escaping); }
1316+
1317+
ParameterTypeFlags withEscaping(bool escaping) const {
1318+
return ParameterTypeFlags(escaping ? value | ParameterTypeFlags::Escaping
1319+
: value - ParameterTypeFlags::Escaping);
1320+
}
13141321

1315-
bool operator==(const ParameterTypeFlags &other) {
1316-
return value == other.value;
1322+
bool operator ==(const ParameterTypeFlags &other) const {
1323+
return value.toRaw() == other.value.toRaw();
13171324
}
1325+
1326+
uint8_t toRaw() const { return value.toRaw(); }
13181327
};
13191328

13201329
/// ParenType - A paren type is a type that's been written in parentheses.
@@ -1399,21 +1408,16 @@ class TupleTypeElt {
13991408

14001409
/// Retrieve a copy of this tuple type element with the type replaced.
14011410
TupleTypeElt getWithType(Type T) const {
1402-
return TupleTypeElt(T, getName(), isVararg(), isAutoClosure(),
1403-
isEscaping());
1411+
return TupleTypeElt(T, getName(), getParameterFlags());
14041412
}
14051413

14061414
/// Retrieve a copy of this tuple type element with the name replaced.
1407-
TupleTypeElt getWithName(Identifier name = Identifier()) const {
1408-
return TupleTypeElt(getType(), name, isVararg(), isAutoClosure(),
1409-
isEscaping());
1415+
TupleTypeElt getWithName(Identifier name) const {
1416+
return TupleTypeElt(getType(), name, getParameterFlags());
14101417
}
14111418

1412-
/// Retrieve a copy of this tuple type element with the type and name
1413-
/// replaced.
1414-
TupleTypeElt getWithTypeAndName(Type T, Identifier name) const {
1415-
return TupleTypeElt(T, name, isVararg(), isAutoClosure(), isEscaping());
1416-
}
1419+
/// Retrieve a copy of this tuple type element with no name
1420+
TupleTypeElt getWithoutName() const { return getWithName(Identifier()); }
14171421
};
14181422

14191423
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
@@ -53,7 +53,7 @@ const uint16_t VERSION_MAJOR = 0;
5353
/// in source control, you should also update the comment to briefly
5454
/// describe what change you made. The content of this comment isn't important;
5555
/// it just ensures a conflict if two people change the module format.
56-
const uint16_t VERSION_MINOR = 268; // Last change: parameter type flags
56+
const uint16_t VERSION_MINOR = 269; // Last change: parameter type flags
5757

5858
using DeclID = PointerEmbeddedInt<unsigned, 31>;
5959
using DeclIDField = BCFixed<31>;
@@ -570,8 +570,10 @@ namespace decls_block {
570570

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

577579
using TupleTypeLayout = BCRecordLayout<
@@ -580,9 +582,11 @@ namespace decls_block {
580582

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

588592
using FunctionTypeLayout = BCRecordLayout<

‎lib/AST/ASTContext.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2642,7 +2642,7 @@ ParenType *ParenType::get(const ASTContext &C, Type underlying,
26422642
auto properties = underlying->getRecursiveProperties();
26432643
auto arena = getArena(properties);
26442644
ParenType *&Result =
2645-
C.Impl.getArena(arena).ParenTypes[{underlying, flags.value}];
2645+
C.Impl.getArena(arena).ParenTypes[{underlying, flags.toRaw()}];
26462646
if (Result == 0) {
26472647
Result = new (C, arena) ParenType(underlying, properties, flags);
26482648
}
@@ -2659,7 +2659,7 @@ void TupleType::Profile(llvm::FoldingSetNodeID &ID,
26592659
for (const TupleTypeElt &Elt : Fields) {
26602660
ID.AddPointer(Elt.Name.get());
26612661
ID.AddPointer(Elt.getType().getPointer());
2662-
ID.AddInteger(Elt.Flags.value);
2662+
ID.AddInteger(Elt.Flags.toRaw());
26632663
}
26642664
}
26652665

‎lib/AST/ASTPrinter.cpp‎

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

26222622
// Special case, if we're not going to use the type repr printing, peek
2623-
// through the paren types so that we don't print excessive @escapings
2623+
// through the paren types so that we don't print excessive @escapings.
26242624
unsigned numParens = 0;
26252625
if (!willUseTypeReprPrinting(TheTypeLoc, Options)) {
26262626
while (auto parenTy =
@@ -2707,7 +2707,7 @@ void PrintAST::printFunctionParameters(AbstractFunctionDecl *AFD) {
27072707
}
27082708

27092709
SmallVector<Type, 4> parameterListTypes;
2710-
for (auto i = 0; i < BodyParams.size(); ++i) {
2710+
for (unsigned i = 0; i < BodyParams.size(); ++i) {
27112711
if (curTy) {
27122712
if (auto funTy = curTy->getAs<AnyFunctionType>()) {
27132713
parameterListTypes.push_back(funTy->getInput());
@@ -2722,7 +2722,7 @@ void PrintAST::printFunctionParameters(AbstractFunctionDecl *AFD) {
27222722
for (unsigned CurrPattern = 0, NumPatterns = BodyParams.size();
27232723
CurrPattern != NumPatterns; ++CurrPattern) {
27242724
// Be extra careful in the event of printing mal-formed ASTs
2725-
auto paramListType = parameterListTypes.size() > CurrPattern
2725+
auto paramListType = CurrPattern < parameterListTypes.size()
27262726
? parameterListTypes[CurrPattern]
27272727
: nullptr;
27282728
printParameterList(BodyParams[CurrPattern], paramListType,
@@ -3930,7 +3930,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
39303930
SmallVector<TupleTypeElt, 4> elements;
39313931
elements.reserve(tupleTy->getNumElements());
39323932
for (const auto &elt : tupleTy->getElements())
3933-
elements.push_back(elt.getWithName());
3933+
elements.push_back(elt.getWithoutName());
39343934
inputType = TupleType::get(elements, inputType->getASTContext());
39353935
}
39363936

‎lib/AST/Decl.cpp‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1482,9 +1482,7 @@ static Type mapSignatureFunctionType(ASTContext &ctx, Type type,
14821482
// Remap our parameters, and make sure to strip off @escaping
14831483
for (const auto &elt : tupleTy->getElements()) {
14841484
auto newEltTy = mapSignatureParamType(ctx, elt.getType());
1485-
ParameterTypeFlags newParamFlags(
1486-
elt.getParameterFlags().value & ~ParameterTypeFlags::Escaping);
1487-
1485+
auto newParamFlags = elt.getParameterFlags().withEscaping(false);
14881486
bool exactlyTheSame = newParamFlags == elt.getParameterFlags() &&
14891487
newEltTy.getPointer() == elt.getType().getPointer();
14901488

@@ -1501,7 +1499,6 @@ static Type mapSignatureFunctionType(ASTContext &ctx, Type type,
15011499
anyChanged = true;
15021500
}
15031501

1504-
// TODO: drop the name when it's finally out of the type system
15051502
elements.emplace_back(newEltTy, elt.getName(), newParamFlags);
15061503
}
15071504
if (anyChanged) {

‎lib/AST/Type.cpp‎

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

745745
Identifier newName = stripLabels? Identifier() : elt.getName();
746-
elements.push_back(elt.getWithTypeAndName(eltTy, newName));
746+
elements.emplace_back(eltTy, newName, elt.getParameterFlags());
747747
}
748748
++idx;
749749
}
@@ -832,7 +832,7 @@ swift::decomposeArgType(Type type, ArrayRef<Identifier> argumentLabels) {
832832

833833
for (auto i : range(0, tupleTy->getNumElements())) {
834834
const auto &elt = tupleTy->getElement(i);
835-
assert(elt.getParameterFlags().value == 0 &&
835+
assert(elt.getParameterFlags().isNone() &&
836836
"Vararg, autoclosure, or escaping argument tuple"
837837
"doesn't make sense");
838838
CallArgParam argParam;

‎lib/Sema/ConstraintSystem.cpp‎

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

‎lib/Sema/TypeCheckType.cpp‎

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

25682568
auto paramFlags = isImmediateFunctionInput
25692569
? ParameterTypeFlags::fromParameterType(ty, variadic)
2570-
: ParameterTypeFlags(variadic, false, false);
2570+
: ParameterTypeFlags();
25712571
elements.emplace_back(ty, name, paramFlags);
25722572
}
25732573

‎lib/Serialization/Deserialization.cpp‎

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

34653465
case decls_block::PAREN_TYPE: {
34663466
TypeID underlyingID;
3467-
uint8_t flagsValue;
3468-
decls_block::ParenTypeLayout::readRecord(scratch, underlyingID, flagsValue);
3469-
typeOrOffset = ParenType::get(ctx, getType(underlyingID),
3470-
ParameterTypeFlags(flagsValue));
3467+
bool isVariadic, isAutoClosure, isEscaping;
3468+
decls_block::ParenTypeLayout::readRecord(scratch, underlyingID, isVariadic,
3469+
isAutoClosure, isEscaping);
3470+
typeOrOffset = ParenType::get(
3471+
ctx, getType(underlyingID),
3472+
ParameterTypeFlags(isVariadic, isAutoClosure, isEscaping));
34713473
break;
34723474
}
34733475

@@ -3487,12 +3489,13 @@ Type ModuleFile::getType(TypeID TID) {
34873489

34883490
IdentifierID nameID;
34893491
TypeID typeID;
3490-
uint8_t flagsValue;
3491-
decls_block::TupleTypeEltLayout::readRecord(scratch, nameID, typeID,
3492-
flagsValue);
3492+
bool isVariadic, isAutoClosure, isEscaping;
3493+
decls_block::TupleTypeEltLayout::readRecord(
3494+
scratch, nameID, typeID, isVariadic, isAutoClosure, isEscaping);
34933495

3494-
elements.emplace_back(getType(typeID), getIdentifier(nameID),
3495-
ParameterTypeFlags(flagsValue));
3496+
elements.emplace_back(
3497+
getType(typeID), getIdentifier(nameID),
3498+
ParameterTypeFlags(isVariadic, isAutoClosure, isEscaping));
34963499
}
34973500

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

‎lib/Serialization/Serialization.cpp‎

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

27832783
case TypeKind::Paren: {
27842784
auto parenTy = cast<ParenType>(ty.getPointer());
2785+
auto paramFlags = parenTy->getParameterFlags();
27852786

27862787
unsigned abbrCode = DeclTypeAbbrCodes[ParenTypeLayout::Code];
2787-
ParenTypeLayout::emitRecord(Out, ScratchRecord, abbrCode,
2788-
addTypeRef(parenTy->getUnderlyingType()),
2789-
parenTy->getParameterFlags().value);
2788+
ParenTypeLayout::emitRecord(
2789+
Out, ScratchRecord, abbrCode, addTypeRef(parenTy->getUnderlyingType()),
2790+
paramFlags.isVariadic(), paramFlags.isAutoClosure(),
2791+
paramFlags.isEscaping());
27902792
break;
27912793
}
27922794

@@ -2798,9 +2800,11 @@ void Serializer::writeType(Type ty) {
27982800

27992801
abbrCode = DeclTypeAbbrCodes[TupleTypeEltLayout::Code];
28002802
for (auto &elt : tupleTy->getElements()) {
2803+
auto paramFlags = elt.getParameterFlags();
28012804
TupleTypeEltLayout::emitRecord(
28022805
Out, ScratchRecord, abbrCode, addIdentifierRef(elt.getName()),
2803-
addTypeRef(elt.getType()), elt.getParameterFlags().value);
2806+
addTypeRef(elt.getType()), paramFlags.isVariadic(),
2807+
paramFlags.isAutoClosure(), paramFlags.isEscaping());
28042808
}
28052809

28062810
break;

0 commit comments

Comments
 (0)