Skip to content

Commit 79a71c1

Browse files
xedinxedin
authored
Merge pull request #71971 from xedin/ncgenerics-swiftpm-circularity-issue
[AST/Serialization] Fix circularity issue with deserialized protocols and composition types
2 parents 87781ad + 3ec584e commit 79a71c1

File tree

9 files changed

+183
-81
lines changed

9 files changed

+183
-81
lines changed

‎lib/AST/NameLookup.cpp

Lines changed: 32 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -3257,55 +3257,42 @@ InheritedProtocolsRequest::evaluate(Evaluator &evaluator,
32573257

32583258
llvm::SmallSetVector<ProtocolDecl *, 2> inherited;
32593259

3260-
if (PD->wasDeserialized()) {
3261-
auto protoSelfTy = PD->getSelfInterfaceType();
3262-
for (auto req : PD->getRequirementSignature().getRequirements()) {
3263-
// Dig out a conformance requirement...
3264-
if (req.getKind() != RequirementKind::Conformance)
3265-
continue;
3266-
3267-
// constraining Self.
3268-
if (!req.getFirstType()->isEqual(protoSelfTy))
3269-
continue;
3260+
assert(!PD->wasDeserialized());
32703261

3271-
inherited.insert(req.getProtocolDecl());
3272-
}
3273-
} else {
3274-
InvertibleProtocolSet inverses;
3275-
bool anyObject = false;
3276-
for (const auto &found : getDirectlyInheritedNominalTypeDecls(
3277-
PD, inverses, anyObject)) {
3278-
auto proto = dyn_cast<ProtocolDecl>(found.Item);
3279-
if (proto && proto != PD)
3280-
inherited.insert(proto);
3281-
}
3282-
3283-
// Apply inverses.
3284-
if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics)) {
3285-
bool skipInverses = false;
3286-
3287-
// ... except for these protocols, so that Copyable does not have to
3288-
// inherit ~Copyable, etc.
3289-
if (auto kp = PD->getKnownProtocolKind()) {
3290-
switch (*kp) {
3291-
case KnownProtocolKind::Sendable:
3292-
case KnownProtocolKind::Copyable:
3293-
case KnownProtocolKind::Escapable:
3294-
skipInverses = true;
3295-
break;
3262+
InvertibleProtocolSet inverses;
3263+
bool anyObject = false;
3264+
for (const auto &found :
3265+
getDirectlyInheritedNominalTypeDecls(PD, inverses, anyObject)) {
3266+
auto proto = dyn_cast<ProtocolDecl>(found.Item);
3267+
if (proto && proto != PD)
3268+
inherited.insert(proto);
3269+
}
3270+
3271+
// Apply inverses.
3272+
if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics)) {
3273+
bool skipInverses = false;
3274+
3275+
// ... except for these protocols, so that Copyable does not have to
3276+
// inherit ~Copyable, etc.
3277+
if (auto kp = PD->getKnownProtocolKind()) {
3278+
switch (*kp) {
3279+
case KnownProtocolKind::Sendable:
3280+
case KnownProtocolKind::Copyable:
3281+
case KnownProtocolKind::Escapable:
3282+
skipInverses = true;
3283+
break;
32963284

3297-
default:
3298-
break;
3299-
}
3285+
default:
3286+
break;
33003287
}
3288+
}
33013289

3302-
if (!skipInverses) {
3303-
for (auto ip : InvertibleProtocolSet::full()) {
3304-
// Unless the user wrote ~P in the syntactic inheritance clause, the
3305-
// semantic inherited list includes P.
3306-
if (!inverses.contains(ip))
3307-
inherited.insert(ctx.getProtocol(getKnownProtocolKind(ip)));
3308-
}
3290+
if (!skipInverses) {
3291+
for (auto ip : InvertibleProtocolSet::full()) {
3292+
// Unless the user wrote ~P in the syntactic inheritance clause, the
3293+
// semantic inherited list includes P.
3294+
if (!inverses.contains(ip))
3295+
inherited.insert(ctx.getProtocol(getKnownProtocolKind(ip)));
33093296
}
33103297
}
33113298
}

‎lib/Serialization/DeclTypeRecordNodes.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,8 @@ TRAILING_INFO(CONDITIONAL_SUBSTITUTION_COND)
214214

215215
OTHER(LIFETIME_DEPENDENCE, 158)
216216

217+
TRAILING_INFO(INHERITED_PROTOCOLS)
218+
217219
#ifndef DECL_ATTR
218220
#define DECL_ATTR(NAME, CLASS, OPTIONS, CODE) RECORD_VAL(CLASS##_DECL_ATTR, 180+CODE)
219221
#endif

‎lib/Serialization/Deserialization.cpp

Lines changed: 58 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1719,6 +1719,35 @@ ModuleFile::getSubstitutionMapChecked(serialization::SubstitutionMapID id) {
17191719
return substitutions;
17201720
}
17211721

1722+
bool ModuleFile::readInheritedProtocols(
1723+
SmallVectorImpl<ProtocolDecl *> &inherited) {
1724+
using namespace decls_block;
1725+
1726+
BCOffsetRAII lastRecordOffset(DeclTypeCursor);
1727+
1728+
llvm::BitstreamEntry entry =
1729+
fatalIfUnexpected(DeclTypeCursor.advance(AF_DontPopBlockAtEnd));
1730+
if (entry.Kind != llvm::BitstreamEntry::Record)
1731+
return false;
1732+
1733+
SmallVector<uint64_t, 8> scratch;
1734+
unsigned recordID =
1735+
fatalIfUnexpected(DeclTypeCursor.readRecord(entry.ID, scratch));
1736+
if (recordID != INHERITED_PROTOCOLS)
1737+
return false;
1738+
1739+
lastRecordOffset.reset();
1740+
1741+
ArrayRef<uint64_t> inheritedIDs;
1742+
InheritedProtocolsLayout::readRecord(scratch, inheritedIDs);
1743+
1744+
llvm::transform(inheritedIDs, std::back_inserter(inherited),
1745+
[&](uint64_t protocolID) {
1746+
return cast<ProtocolDecl>(getDecl(protocolID));
1747+
});
1748+
return true;
1749+
}
1750+
17221751
bool ModuleFile::readDefaultWitnessTable(ProtocolDecl *proto) {
17231752
using namespace decls_block;
17241753

@@ -3187,6 +3216,21 @@ class DeclDeserializer {
31873216
decl.get<ExtensionDecl *>()->setInherited(inherited);
31883217
}
31893218

3219+
void handleInherited(ProtocolDecl *P,
3220+
ArrayRef<ProtocolDecl *> inherited) {
3221+
SmallVector<InheritedEntry, 2> inheritedTypes;
3222+
llvm::transform(inherited, std::back_inserter(inheritedTypes), [](auto *I) {
3223+
return InheritedEntry(TypeLoc::withoutLoc(I->getDeclaredInterfaceType()),
3224+
/*isUnchecked=*/false,
3225+
/*isRetroactive=*/false,
3226+
/*isPreconcurrency=*/false);
3227+
});
3228+
3229+
P->setInherited(ctx.AllocateCopy(inheritedTypes));
3230+
ctx.evaluator.cacheOutput(InheritedProtocolsRequest{P},
3231+
ctx.AllocateCopy(inherited));
3232+
}
3233+
31903234
public:
31913235
DeclDeserializer(ModuleFile &MF, Serialized<Decl *> &declOrOffset)
31923236
: MF(MF), ctx(MF.getContext()), declOrOffset(declOrOffset) {}
@@ -4425,21 +4469,21 @@ class DeclDeserializer {
44254469
IdentifierID nameID;
44264470
DeclContextID contextID;
44274471
bool isImplicit, isClassBounded, isObjC, hasSelfOrAssocTypeRequirements;
4472+
TypeID superclassID;
44284473
uint8_t rawAccessLevel;
4429-
unsigned numInheritedTypes;
4430-
ArrayRef<uint64_t> rawInheritedAndDependencyIDs;
4474+
ArrayRef<uint64_t> dependencyIDs;
44314475

44324476
decls_block::ProtocolLayout::readRecord(scratch, nameID, contextID,
44334477
isImplicit, isClassBounded, isObjC,
44344478
hasSelfOrAssocTypeRequirements,
4435-
rawAccessLevel, numInheritedTypes,
4436-
rawInheritedAndDependencyIDs);
4479+
superclassID,
4480+
rawAccessLevel,
4481+
dependencyIDs);
44374482

44384483
Identifier name = MF.getIdentifier(nameID);
44394484
PrettySupplementalDeclNameTrace trace(name);
44404485

4441-
for (TypeID dependencyID :
4442-
rawInheritedAndDependencyIDs.slice(numInheritedTypes)) {
4486+
for (TypeID dependencyID : dependencyIDs) {
44434487
auto dependency = MF.getTypeChecked(dependencyID);
44444488
if (!dependency) {
44454489
return llvm::make_error<TypeError>(
@@ -4457,6 +4501,8 @@ class DeclDeserializer {
44574501
/*TrailingWhere=*/nullptr);
44584502
declOrOffset = proto;
44594503

4504+
proto->setSuperclass(MF.getType(superclassID));
4505+
44604506
ctx.evaluator.cacheOutput(ProtocolRequiresClassRequest{proto},
44614507
std::move(isClassBounded));
44624508
ctx.evaluator.cacheOutput(HasSelfOrAssociatedTypeRequirementsRequest{proto},
@@ -4467,14 +4513,17 @@ class DeclDeserializer {
44674513
else
44684514
return MF.diagnoseFatal();
44694515

4516+
SmallVector<ProtocolDecl *, 2> inherited;
4517+
if (!MF.readInheritedProtocols(inherited))
4518+
return MF.diagnoseFatal();
4519+
4520+
handleInherited(proto, inherited);
4521+
44704522
auto genericParams = MF.maybeReadGenericParams(DC);
44714523
assert(genericParams && "protocol with no generic parameters?");
44724524
ctx.evaluator.cacheOutput(GenericParamListRequest{proto},
44734525
std::move(genericParams));
44744526

4475-
handleInherited(proto,
4476-
rawInheritedAndDependencyIDs.slice(0, numInheritedTypes));
4477-
44784527
if (isImplicit)
44794528
proto->setImplicit();
44804529
proto->setIsObjC(isObjC);

‎lib/Serialization/ModuleFile.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,9 @@ class ModuleFile
538538
SmallVectorImpl<AssociatedTypeDecl *> &assocTypes,
539539
llvm::BitstreamCursor &Cursor);
540540

541+
/// Read a list of the protocol declarations inherited by another protocol.
542+
bool readInheritedProtocols(SmallVectorImpl<ProtocolDecl *> &inherited);
543+
541544
/// Populates the protocol's default witness table.
542545
///
543546
/// Returns true if there is an error.

‎lib/Serialization/ModuleFormat.h

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
5858
/// describe what change you made. The content of this comment isn't important;
5959
/// it just ensures a conflict if two people change the module format.
6060
/// Don't worry about adhering to the 80-column limit for this line.
61-
const uint16_t SWIFTMODULE_VERSION_MINOR = 855; // ref_adhoc_requirement_witness attributes are back
61+
const uint16_t SWIFTMODULE_VERSION_MINOR = 858; // Replace inherited types with protocols in protocol layout
6262

6363
/// A standard hash seed used for all string hashes in a serialized module.
6464
///
@@ -1527,11 +1527,18 @@ namespace decls_block {
15271527
BCFixed<1>, // class-bounded?
15281528
BCFixed<1>, // objc?
15291529
BCFixed<1>, // existential-type-supported?
1530+
TypeIDField, // superclass
15301531
AccessLevelField, // access level
1531-
BCVBR<4>, // number of inherited types
1532-
BCArray<TypeIDField> // inherited types, followed by dependency types
1533-
// Trailed by the generic parameters (if any), the members record, and
1534-
// the default witness table record
1532+
BCArray<TypeIDField> // dependency types
1533+
// Trailed by the inherited protocols, the generic parameters (if any),
1534+
// the generic signature, the members record, and the default witness table record
1535+
>;
1536+
1537+
/// A default witness table for a protocol.
1538+
using InheritedProtocolsLayout = BCRecordLayout<
1539+
INHERITED_PROTOCOLS,
1540+
BCArray<DeclIDField>
1541+
// An array of inherited protocol declarations
15351542
>;
15361543

15371544
/// A default witness table for a protocol.

‎lib/Serialization/Serialization.cpp

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3712,6 +3712,18 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
37123712
}
37133713
}
37143714

3715+
void writeInheritedProtocols(ArrayRef<ProtocolDecl *> inherited) {
3716+
using namespace decls_block;
3717+
3718+
SmallVector<DeclID, 4> inheritedIDs;
3719+
llvm::transform(inherited, std::back_inserter(inheritedIDs),
3720+
[&](const ProtocolDecl *P) { return S.addDeclRef(P); });
3721+
3722+
unsigned abbrCode = S.DeclTypeAbbrCodes[InheritedProtocolsLayout::Code];
3723+
InheritedProtocolsLayout::emitRecord(S.Out, S.ScratchRecord, abbrCode,
3724+
inheritedIDs);
3725+
}
3726+
37153727
void writeDefaultWitnessTable(const ProtocolDecl *proto) {
37163728
using namespace decls_block;
37173729

@@ -4280,12 +4292,8 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
42804292

42814293
auto contextID = S.addDeclContextRef(proto->getDeclContext());
42824294

4283-
SmallVector<TypeID, 4> inheritedAndDependencyTypes;
42844295
swift::SmallSetVector<Type, 4> dependencyTypes;
42854296

4286-
unsigned numInherited = addInherited(
4287-
proto->getInherited(), inheritedAndDependencyTypes);
4288-
42894297
// Separately collect inherited protocol types as dependencies.
42904298
for (auto element : proto->getInherited().getEntries()) {
42914299
auto elementType = element.getType();
@@ -4311,8 +4319,9 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
43114319
/*excluding*/S.M);
43124320
}
43134321

4322+
SmallVector<TypeID, 4> dependencyTypeIDs;
43144323
for (Type ty : dependencyTypes)
4315-
inheritedAndDependencyTypes.push_back(S.addTypeRef(ty));
4324+
dependencyTypeIDs.push_back(S.addTypeRef(ty));
43164325

43174326
uint8_t rawAccessLevel = getRawStableAccessLevel(proto->getFormalAccess());
43184327

@@ -4325,9 +4334,11 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
43254334
->requiresClass(),
43264335
proto->isObjC(),
43274336
proto->hasSelfOrAssociatedTypeRequirements(),
4328-
rawAccessLevel, numInherited,
4329-
inheritedAndDependencyTypes);
4337+
S.addTypeRef(proto->getSuperclass()),
4338+
rawAccessLevel,
4339+
dependencyTypeIDs);
43304340

4341+
writeInheritedProtocols(proto->getInheritedProtocols());
43314342
writeGenericParams(proto->getGenericParams());
43324343
S.writeRequirementSignature(proto->getRequirementSignature());
43334344
S.writeAssociatedTypes(proto->getAssociatedTypeMembers());
@@ -5904,7 +5915,7 @@ bool Serializer::writeASTBlockEntitiesIfNeeded(
59045915
}
59055916

59065917
void Serializer::writeAllDeclsAndTypes() {
5907-
BCBlockRAII restoreBlock(Out, DECLS_AND_TYPES_BLOCK_ID, 8);
5918+
BCBlockRAII restoreBlock(Out, DECLS_AND_TYPES_BLOCK_ID, 9);
59085919
using namespace decls_block;
59095920
registerDeclTypeAbbr<BuiltinAliasTypeLayout>();
59105921
registerDeclTypeAbbr<TypeAliasTypeLayout>();
@@ -6031,6 +6042,8 @@ void Serializer::writeAllDeclsAndTypes() {
60316042
registerDeclTypeAbbr<ConditionalSubstitutionLayout>();
60326043
registerDeclTypeAbbr<ConditionalSubstitutionConditionLayout>();
60336044

6045+
registerDeclTypeAbbr<InheritedProtocolsLayout>();
6046+
60346047
#define DECL_ATTR(X, NAME, ...) \
60356048
registerDeclTypeAbbr<NAME##DeclAttrLayout>();
60366049
#include "swift/AST/DeclAttr.def"

‎test/Serialization/test_recursive_protocol_deserialization.swift

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// RUN: %empty-directory(%t/src)
2+
// RUN: split-file %s %t/src
3+
4+
/// Build the library A
5+
// RUN: %target-swift-frontend -emit-module %t/src/API.swift \
6+
// RUN: -module-name API -swift-version 5 \
7+
// RUN: -emit-module-path %t/API.swiftmodule
8+
9+
// Build client with module
10+
// RUN: %target-swift-emit-silgen \
11+
// RUN: -I %t \
12+
// RUN: -module-name Client \
13+
// RUN: %t/src/Client.swift -verify
14+
15+
//--- API.swift
16+
17+
public class C {}
18+
19+
public protocol P1 {
20+
typealias A = (C & P1)
21+
}
22+
23+
public protocol P2 {
24+
typealias F = (P2) -> Void
25+
}
26+
27+
public protocol P3 where Self : C {
28+
}
29+
30+
//--- Client.swift
31+
import API
32+
33+
func test(_: any P1) {} // Ok
34+
35+
_ = P2.self // Ok
36+
37+
class B : P3 {
38+
// expected-error@-1 {{type 'B' does not conform to protocol 'P3'}}
39+
// expected-error@-2 {{'P3' requires that 'B' inherit from 'C'}}
40+
// expected-note@-3 {{requirement specified as 'Self' : 'C' [with Self = B]}}
41+
}

‎test/api-digester/Outputs/cake-abi.json

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,19 +107,19 @@
107107
"genericSig": "<τ_0_0 : cake.P1, τ_0_0 : cake.P2>",
108108
"sugared_genericSig": "<Self : cake.P1, Self : cake.P2>",
109109
"conformances": [
110-
{
111-
"kind": "Conformance",
112-
"name": "P2",
113-
"printedName": "P2",
114-
"usr": "s:4cake2P2P",
115-
"mangledName": "$s4cake2P2P"
116-
},
117110
{
118111
"kind": "Conformance",
119112
"name": "P1",
120113
"printedName": "P1",
121114
"usr": "s:4cake2P1P",
122115
"mangledName": "$s4cake2P1P"
116+
},
117+
{
118+
"kind": "Conformance",
119+
"name": "P2",
120+
"printedName": "P2",
121+
"usr": "s:4cake2P2P",
122+
"mangledName": "$s4cake2P2P"
123123
}
124124
]
125125
},

0 commit comments

Comments
 (0)