Skip to content

Commit e14fefb

Browse files
slavapestovslavapestov
authored
Merge pull request #60716 from slavapestov/ban-normal-conformances-of-protocols
Ban constructing normal conformances where the conforming type is a protocol
2 parents 36893aa + 50aaaa4 commit e14fefb

20 files changed

+192
-120
lines changed

‎lib/APIDigester/ModuleAnalyzerNodes.cpp

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ struct swift::ide::api::SDKNodeInitInfo {
5050
SDKNodeInitInfo(SDKContext &Ctx, ValueDecl *VD);
5151
SDKNodeInitInfo(SDKContext &Ctx, OperatorDecl *D);
5252
SDKNodeInitInfo(SDKContext &Ctx, ImportDecl *ID);
53-
SDKNodeInitInfo(SDKContext &Ctx, ProtocolConformance *Conform);
53+
SDKNodeInitInfo(SDKContext &Ctx, ProtocolConformanceRef Conform);
5454
SDKNodeInitInfo(SDKContext &Ctx, Type Ty, TypeInitInfo Info = TypeInitInfo());
5555
SDKNode* createSDKNode(SDKNodeKind Kind);
5656
};
@@ -1459,17 +1459,21 @@ SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, ImportDecl *ID):
14591459
Name = PrintedName = Ctx.buffer(content);
14601460
}
14611461

1462-
SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, ProtocolConformance *Conform):
1463-
SDKNodeInitInfo(Ctx, Conform->getProtocol()) {
1462+
SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, ProtocolConformanceRef Conform):
1463+
SDKNodeInitInfo(Ctx, Conform.getRequirement()) {
14641464
// The conformance can be conditional. The generic signature keeps track of
14651465
// the requirements.
1466-
GenericSig = printGenericSignature(Ctx, Conform, Ctx.checkingABI());
1467-
SugaredGenericSig = Ctx.checkingABI() ?
1468-
printGenericSignature(Ctx, Conform, false): StringRef();
1469-
// Whether this conformance is ABI placeholder depends on the decl context
1470-
// of this conformance.
1471-
IsABIPlaceholder = isABIPlaceholderRecursive(Conform->getDeclContext()->
1472-
getAsDecl());
1466+
if (Conform.isConcrete()) {
1467+
auto *Concrete = Conform.getConcrete();
1468+
1469+
GenericSig = printGenericSignature(Ctx, Concrete, Ctx.checkingABI());
1470+
SugaredGenericSig = Ctx.checkingABI() ?
1471+
printGenericSignature(Ctx, Concrete, false): StringRef();
1472+
// Whether this conformance is ABI placeholder depends on the decl context
1473+
// of this conformance.
1474+
IsABIPlaceholder = isABIPlaceholderRecursive(Concrete->getDeclContext()->
1475+
getAsDecl());
1476+
}
14731477
}
14741478

14751479
static bool isProtocolRequirement(ValueDecl *VD) {
@@ -1910,7 +1914,7 @@ SwiftDeclCollector::constructConformanceNode(ProtocolConformance *Conform) {
19101914
if (Ctx.checkingABI())
19111915
Conform = Conform->getCanonicalConformance();
19121916
auto ConfNode = cast<SDKNodeConformance>(SDKNodeInitInfo(Ctx,
1913-
Conform).createSDKNode(SDKNodeKind::Conformance));
1917+
ProtocolConformanceRef(Conform)).createSDKNode(SDKNodeKind::Conformance));
19141918
Conform->forEachTypeWitness(
19151919
[&](AssociatedTypeDecl *assoc, Type ty, TypeDecl *typeDecl) -> bool {
19161920
ConfNode->addChild(constructTypeWitnessNode(assoc, ty));
@@ -1922,12 +1926,25 @@ SwiftDeclCollector::constructConformanceNode(ProtocolConformance *Conform) {
19221926
void swift::ide::api::
19231927
SwiftDeclCollector::addConformancesToTypeDecl(SDKNodeDeclType *Root,
19241928
NominalTypeDecl *NTD) {
1925-
// Avoid adding the same conformance twice.
1926-
SmallPtrSet<ProtocolConformance*, 4> Seen;
1927-
for (auto &Conf: NTD->getAllConformances()) {
1928-
if (!Ctx.shouldIgnore(Conf->getProtocol()) && !Seen.count(Conf))
1929-
Root->addConformance(constructConformanceNode(Conf));
1930-
Seen.insert(Conf);
1929+
if (auto *PD = dyn_cast<ProtocolDecl>(NTD)) {
1930+
PD->walkInheritedProtocols([&](ProtocolDecl *inherited) {
1931+
if (PD != inherited && !Ctx.shouldIgnore(inherited)) {
1932+
ProtocolConformanceRef Conf(inherited);
1933+
auto ConfNode = SDKNodeInitInfo(Ctx, Conf)
1934+
.createSDKNode(SDKNodeKind::Conformance);
1935+
Root->addConformance(ConfNode);
1936+
}
1937+
1938+
return TypeWalker::Action::Continue;
1939+
});
1940+
} else {
1941+
// Avoid adding the same conformance twice.
1942+
SmallPtrSet<ProtocolConformance*, 4> Seen;
1943+
for (auto &Conf: NTD->getAllConformances()) {
1944+
if (!Ctx.shouldIgnore(Conf->getProtocol()) && !Seen.count(Conf))
1945+
Root->addConformance(constructConformanceNode(Conf));
1946+
Seen.insert(Conf);
1947+
}
19311948
}
19321949
}
19331950

‎lib/AST/ASTPrinter.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2314,10 +2314,12 @@ void PrintAST::printMembersOfDecl(Decl *D, bool needComma,
23142314
AddMembers(Ext);
23152315
}
23162316
if (Options.PrintExtensionFromConformingProtocols) {
2317-
for (auto Conf : NTD->getAllConformances()) {
2318-
for (auto Ext : Conf->getProtocol()->getExtensions()) {
2319-
if (Options.printExtensionContentAsMembers(Ext))
2320-
AddMembers(Ext);
2317+
if (!isa<ProtocolDecl>(NTD)) {
2318+
for (auto Conf : NTD->getAllConformances()) {
2319+
for (auto Ext : Conf->getProtocol()->getExtensions()) {
2320+
if (Options.printExtensionContentAsMembers(Ext))
2321+
AddMembers(Ext);
2322+
}
23212323
}
23222324
}
23232325
}

‎lib/AST/ConformanceLookupTable.cpp

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -620,20 +620,14 @@ ConformanceLookupTable::Ordering ConformanceLookupTable::compareConformances(
620620
// If the explicit protocol for the left-hand side is implied by
621621
// the explicit protocol for the right-hand side, the left-hand
622622
// side supersedes the right-hand side.
623-
for (auto rhsProtocol : rhsExplicitProtocol->getAllProtocols()) {
624-
if (rhsProtocol == lhsExplicitProtocol) {
625-
return Ordering::Before;
626-
}
627-
}
623+
if (rhsExplicitProtocol->inheritsFrom(lhsExplicitProtocol))
624+
return Ordering::Before;
628625

629626
// If the explicit protocol for the right-hand side is implied by
630627
// the explicit protocol for the left-hand side, the right-hand
631628
// side supersedes the left-hand side.
632-
for (auto lhsProtocol : lhsExplicitProtocol->getAllProtocols()) {
633-
if (lhsProtocol == rhsExplicitProtocol) {
634-
return Ordering::After;
635-
}
636-
}
629+
if (lhsExplicitProtocol->inheritsFrom(rhsExplicitProtocol))
630+
return Ordering::After;
637631
}
638632

639633
// Prefer the least conditional implier, which we approximate by seeing if one
@@ -880,13 +874,18 @@ ConformanceLookupTable::getConformance(NominalTypeDecl *nominal,
880874
entry->Conformance =
881875
ctx.getInheritedConformance(type, inheritedConformance.getConcrete());
882876
} else {
883-
// Create or find the normal conformance.
884-
Type conformingType = conformingDC->getDeclaredInterfaceType();
877+
// Protocols don't have conformance lookup tables. Self-conformance is
878+
// handled directly in lookupConformance().
879+
assert(!isa<ProtocolDecl>(conformingNominal));
880+
assert(!isa<ProtocolDecl>(conformingDC->getSelfNominalTypeDecl()));
881+
Type conformingType = conformingDC->getSelfInterfaceType();
882+
885883
SourceLoc conformanceLoc
886884
= conformingNominal == conformingDC
887885
? conformingNominal->getLoc()
888886
: cast<ExtensionDecl>(conformingDC)->getLoc();
889887

888+
// Create or find the normal conformance.
890889
auto normalConf =
891890
ctx.getConformance(conformingType, protocol, conformanceLoc,
892891
conformingDC, ProtocolConformanceState::Incomplete,

‎lib/AST/NameLookup.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3121,10 +3121,17 @@ static bool checkForDynamicAttribute(Evaluator &eval, NominalTypeDecl *decl) {
31213121
return evaluateOrDefault(eval, Req{decl}, false);
31223122
};
31233123

3124-
// Check the protocols the type conforms to.
3125-
for (auto *proto : decl->getAllProtocols()) {
3126-
if (hasAttribute(proto))
3127-
return true;
3124+
if (auto *proto = dyn_cast<ProtocolDecl>(decl)) {
3125+
// Check inherited protocols of a protocol.
3126+
for (auto *otherProto : proto->getInheritedProtocols())
3127+
if (hasAttribute(otherProto))
3128+
return true;
3129+
} else {
3130+
// Check the protocols the type conforms to.
3131+
for (auto *otherProto : decl->getAllProtocols()) {
3132+
if (hasAttribute(otherProto))
3133+
return true;
3134+
}
31283135
}
31293136

31303137
// Check the superclass if present.

‎lib/AST/ProtocolConformance.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,6 +1224,9 @@ ProtocolConformance::getInheritedConformance(ProtocolDecl *protocol) const {
12241224

12251225
#pragma mark Protocol conformance lookup
12261226
void NominalTypeDecl::prepareConformanceTable() const {
1227+
assert(!isa<ProtocolDecl>(this) &&
1228+
"Protocols don't have a conformance table");
1229+
12271230
if (ConformanceTable)
12281231
return;
12291232

@@ -1292,6 +1295,10 @@ void NominalTypeDecl::prepareConformanceTable() const {
12921295
bool NominalTypeDecl::lookupConformance(
12931296
ProtocolDecl *protocol,
12941297
SmallVectorImpl<ProtocolConformance *> &conformances) const {
1298+
assert(!isa<ProtocolDecl>(this) &&
1299+
"Self-conformances are only found by the higher-level "
1300+
"ModuleDecl::lookupConformance() entry point");
1301+
12951302
prepareConformanceTable();
12961303
return ConformanceTable->lookupConformance(
12971304
const_cast<NominalTypeDecl *>(this),
@@ -1301,6 +1308,10 @@ bool NominalTypeDecl::lookupConformance(
13011308

13021309
SmallVector<ProtocolDecl *, 2>
13031310
NominalTypeDecl::getAllProtocols(bool sorted) const {
1311+
assert(!isa<ProtocolDecl>(this) &&
1312+
"For inherited protocols, use ProtocolDecl::inheritsFrom() or "
1313+
"ProtocolDecl::getInheritedProtocols()");
1314+
13041315
prepareConformanceTable();
13051316
SmallVector<ProtocolDecl *, 2> result;
13061317
ConformanceTable->getAllProtocols(const_cast<NominalTypeDecl *>(this), result,

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6598,8 +6598,8 @@ void SwiftDeclConverter::importObjCProtocols(
65986598
SmallVectorImpl<InheritedEntry> &inheritedTypes) {
65996599
SmallVector<ProtocolDecl *, 4> protocols;
66006600
llvm::SmallPtrSet<ProtocolDecl *, 4> knownProtocols;
6601-
if (auto nominal = dyn_cast<NominalTypeDecl>(decl)) {
6602-
nominal->getImplicitProtocols(protocols);
6601+
if (auto classDecl = dyn_cast<ClassDecl>(decl)) {
6602+
classDecl->getImplicitProtocols(protocols);
66036603
knownProtocols.insert(protocols.begin(), protocols.end());
66046604
}
66056605

‎lib/ConstExtract/ConstExtract.cpp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,24 +26,27 @@
2626
#include <sstream>
2727
#include <string>
2828

29+
using namespace swift;
30+
2931
namespace {
3032
/// A helper class to collect all nominal type declarations that conform to
3133
/// specific protocols provided as input.
32-
class NominalTypeConformanceCollector : public swift::ASTWalker {
34+
class NominalTypeConformanceCollector : public ASTWalker {
3335
const std::unordered_set<std::string> &Protocols;
34-
std::vector<swift::NominalTypeDecl *> &ConformanceTypeDecls;
36+
std::vector<NominalTypeDecl *> &ConformanceTypeDecls;
3537

3638
public:
3739
NominalTypeConformanceCollector(
3840
const std::unordered_set<std::string> &Protocols,
39-
std::vector<swift::NominalTypeDecl *> &ConformanceDecls)
41+
std::vector<NominalTypeDecl *> &ConformanceDecls)
4042
: Protocols(Protocols), ConformanceTypeDecls(ConformanceDecls) {}
4143

42-
bool walkToDeclPre(swift::Decl *D) override {
43-
if (auto *NTD = llvm::dyn_cast<swift::NominalTypeDecl>(D))
44-
for (auto &Protocol : NTD->getAllProtocols())
45-
if (Protocols.count(Protocol->getName().str().str()) != 0)
46-
ConformanceTypeDecls.push_back(NTD);
44+
bool walkToDeclPre(Decl *D) override {
45+
if (auto *NTD = llvm::dyn_cast<NominalTypeDecl>(D))
46+
if (!isa<ProtocolDecl>(NTD))
47+
for (auto &Protocol : NTD->getAllProtocols())
48+
if (Protocols.count(Protocol->getName().str().str()) != 0)
49+
ConformanceTypeDecls.push_back(NTD);
4750
return true;
4851
}
4952
};

‎lib/IDE/CodeCompletionResultType.cpp

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -208,32 +208,45 @@ const USRBasedType *USRBasedType::fromType(Type Ty, USRBasedTypeArena &Arena) {
208208
}
209209

210210
SmallVector<const USRBasedType *, 2> Supertypes;
211+
;
211212
if (auto Nominal = Ty->getAnyNominal()) {
212-
auto Conformances = Nominal->getAllConformances();
213-
Supertypes.reserve(Conformances.size());
214-
for (auto Conformance : Conformances) {
215-
if (Conformance->getDeclContext()->getParentModule() !=
216-
Nominal->getModuleContext()) {
217-
// Only include conformances that are declared within the module of the
218-
// type to avoid caching retroactive conformances which might not
219-
// exist when using the code completion cache from a different module.
220-
continue;
221-
}
222-
if (Conformance->getProtocol()->isSpecificProtocol(KnownProtocolKind::Sendable)) {
223-
// FIXME: Sendable conformances are lazily synthesized as they are
224-
// needed by the compiler. Depending on whether we checked whether a
225-
// type conforms to Sendable before constructing the USRBasedType, we
226-
// get different results for its conformance. For now, always drop the
227-
// Sendable conformance.
228-
continue;
213+
if (auto *Proto = dyn_cast<ProtocolDecl>(Nominal)) {
214+
Proto->walkInheritedProtocols([&](ProtocolDecl *inherited) {
215+
if (Proto != inherited &&
216+
!inherited->isSpecificProtocol(KnownProtocolKind::Sendable)) {
217+
Supertypes.push_back(USRBasedType::fromType(
218+
inherited->getDeclaredInterfaceType(), Arena));
219+
}
220+
221+
return TypeWalker::Action::Continue;
222+
});
223+
} else {
224+
auto Conformances = Nominal->getAllConformances();
225+
Supertypes.reserve(Conformances.size());
226+
for (auto Conformance : Conformances) {
227+
if (Conformance->getDeclContext()->getParentModule() !=
228+
Nominal->getModuleContext()) {
229+
// Only include conformances that are declared within the module of the
230+
// type to avoid caching retroactive conformances which might not
231+
// exist when using the code completion cache from a different module.
232+
continue;
233+
}
234+
if (Conformance->getProtocol()->isSpecificProtocol(KnownProtocolKind::Sendable)) {
235+
// FIXME: Sendable conformances are lazily synthesized as they are
236+
// needed by the compiler. Depending on whether we checked whether a
237+
// type conforms to Sendable before constructing the USRBasedType, we
238+
// get different results for its conformance. For now, always drop the
239+
// Sendable conformance.
240+
continue;
241+
}
242+
Supertypes.push_back(USRBasedType::fromType(
243+
Conformance->getProtocol()->getDeclaredInterfaceType(), Arena));
229244
}
230-
Supertypes.push_back(USRBasedType::fromType(
231-
Conformance->getProtocol()->getDeclaredInterfaceType(), Arena));
232245
}
233246
}
234247

235248
// You would think that superclass + conformances form a DAG. You are wrong!
236-
// We can achieve a circular supertype hierarcy with
249+
// We can achieve a circular supertype hierarchy with
237250
//
238251
// protocol Proto : Class {}
239252
// class Class : Proto {}

‎lib/IDE/CompletionLookup.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2483,7 +2483,7 @@ void CompletionLookup::addTypeRelationFromProtocol(
24832483

24842484
// The literal can produce any type that conforms to its ExpressibleBy
24852485
// protocol. Figure out as which type we want to show it in code completion.
2486-
auto *P = Ctx.getProtocol(protocolForLiteralKind(kind));
2486+
auto *PD = Ctx.getProtocol(protocolForLiteralKind(kind));
24872487
for (auto T : expectedTypeContext.getPossibleTypes()) {
24882488
if (!T)
24892489
continue;
@@ -2497,9 +2497,8 @@ void CompletionLookup::addTypeRelationFromProtocol(
24972497
}
24982498

24992499
// Check for conformance to the literal protocol.
2500-
if (auto *NTD = T->getAnyNominal()) {
2501-
SmallVector<ProtocolConformance *, 2> conformances;
2502-
if (NTD->lookupConformance(P, conformances)) {
2500+
if (T->getAnyNominal()) {
2501+
if (CurrModule->lookupConformance(T, PD)) {
25032502
literalType = T;
25042503
break;
25052504
}

‎lib/IDE/CompletionOverrideLookup.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,9 @@ void CompletionOverrideLookup::addAssociatedTypes(NominalTypeDecl *NTD) {
411411
hasOverride || hasOverridabilityModifier || hasStaticOrClass))
412412
return;
413413

414+
if (isa<ProtocolDecl>(NTD))
415+
return;
416+
414417
for (auto Conformance : NTD->getAllConformances()) {
415418
auto Proto = Conformance->getProtocol();
416419
if (!Proto->isAccessibleFrom(CurrDeclContext))

‎lib/PrintAsClang/DeclAndTypePrinter.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1742,6 +1742,9 @@ class DeclAndTypePrinter::Implementation
17421742
if (nominal->hasClangNode())
17431743
return nullptr;
17441744

1745+
if (isa<ProtocolDecl>(nominal))
1746+
return nullptr;
1747+
17451748
auto &ctx = nominal->getASTContext();
17461749

17471750
// Dig out the ObjectiveCBridgeable protocol.

‎lib/SILOptimizer/Analysis/ProtocolConformanceAnalysis.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,12 @@ class NominalTypeWalker : public ASTWalker {
3737
bool walkToDeclPre(Decl *D) override {
3838
/// (1) Walk over all NominalTypeDecls to determine conformances.
3939
if (auto *NTD = dyn_cast<NominalTypeDecl>(D)) {
40-
auto Protocols = NTD->getAllProtocols();
41-
for (auto &Protocol : Protocols) {
42-
if (Protocol->getEffectiveAccess() <= AccessLevel::Internal) {
43-
ProtocolConformanceCache[Protocol].push_back(NTD);
40+
if (!isa<ProtocolDecl>(NTD)) {
41+
auto Protocols = NTD->getAllProtocols();
42+
for (auto &Protocol : Protocols) {
43+
if (Protocol->getEffectiveAccess() <= AccessLevel::Internal) {
44+
ProtocolConformanceCache[Protocol].push_back(NTD);
45+
}
4446
}
4547
}
4648
}

‎lib/Sema/CSSimplify.cpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8093,16 +8093,12 @@ allFromConditionalConformances(DeclContext *DC, Type baseTy,
80938093
}
80948094

80958095
if (auto *protocol = candidateDC->getSelfProtocolDecl()) {
8096-
SmallVector<ProtocolConformance *, 4> conformances;
8097-
if (!NTD->lookupConformance(protocol, conformances))
8096+
auto conformance = DC->getParentModule()->lookupConformance(
8097+
baseTy, protocol);
8098+
if (!conformance.isConcrete())
80988099
return false;
80998100

8100-
// This is opportunistic, there should be a way to narrow the
8101-
// list down to a particular declaration member comes from.
8102-
return llvm::any_of(
8103-
conformances, [](const ProtocolConformance *conformance) {
8104-
return !conformance->getConditionalRequirements().empty();
8105-
});
8101+
return !conformance.getConcrete()->getConditionalRequirements().empty();
81068102
}
81078103

81088104
return false;

0 commit comments

Comments
 (0)