Skip to content

Commit 737dcb9

Browse files
committed
NCGenerics: break cycle with SuperclassTypeRequest
With NoncopyableGenerics, we get a cycle involving `SuperclassTypeRequest` with this program: ``` public struct RawMarkupHeader {} final class RawMarkup: ManagedBuffer<RawMarkupHeader, RawMarkup> { } ``` Because we generally don't support the following kind of relationship: ``` class Base<T: P>: P {} class Derived: Base<Derived> {} ``` This commit works around the root-cause, which is that Derived's synthesized conformance to Copyable gets superceded by the inherited one from Base. Instead of recording conformances in the ConformanceLookup table at all, create builtin conformances, since classes cannot be conditionally Copyable.
1 parent ee5df55 commit 737dcb9

File tree

5 files changed

+56
-22
lines changed

5 files changed

+56
-22
lines changed

lib/AST/ConformanceLookup.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,20 @@ LookupConformanceInModuleRequest::evaluate(
586586
if (!nominal || isa<ProtocolDecl>(nominal))
587587
return ProtocolConformanceRef::forMissingOrInvalid(type, protocol);
588588

589+
// We specially avoid recording conformances to invertible protocols in a
590+
// class's conformance table. This prevents an evaulator cycle.
591+
if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics) &&
592+
isa<ClassDecl>(nominal)) {
593+
if (auto ip = protocol->getInvertibleProtocolKind()) {
594+
ImplicitKnownProtocolConformanceRequest req{nominal,
595+
getKnownProtocolKind(*ip)};
596+
if (auto conformance = evaluateOrDefault(ctx.evaluator, req, nullptr))
597+
return ProtocolConformanceRef(conformance);
598+
else
599+
return ProtocolConformanceRef::forInvalid();
600+
}
601+
}
602+
589603
// Expand conformances added by extension macros.
590604
//
591605
// FIXME: This expansion should only be done if the

lib/AST/GenericEnvironment.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,9 @@ Type GenericEnvironment::mapTypeIntoContext(GenericEnvironment *env,
417417
}
418418

419419
Type MapTypeOutOfContext::operator()(SubstitutableType *type) const {
420+
if (auto genericParam = dyn_cast<GenericTypeParamType>(type))
421+
return genericParam;
422+
420423
auto archetype = cast<ArchetypeType>(type);
421424
if (isa<OpaqueTypeArchetypeType>(archetype->getRoot()))
422425
return Type();

lib/AST/ProtocolConformance.cpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,19 +1090,21 @@ void NominalTypeDecl::prepareConformanceTable() const {
10901090
// Synthesize the unconditional conformances to invertible protocols.
10911091
// For conditional ones, see findSynthesizedConformances .
10921092
if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics)) {
1093-
bool missingOne = false;
1094-
for (auto ip : InvertibleProtocolSet::full()) {
1095-
auto invertible = getMarking(ip);
1096-
if (!invertible.getInverse() || bool(invertible.getPositive()))
1097-
addSynthesized(ctx.getProtocol(getKnownProtocolKind(ip)));
1098-
else
1099-
missingOne = true;
1100-
}
1101-
1102-
// FIXME: rdar://122289155 (NCGenerics: convert Equatable, Hashable, and RawRepresentable to ~Copyable.)
1103-
if (missingOne)
1104-
return;
1093+
// Classes get their conformances during ModuleDecl::lookupConformance.
1094+
if (!isa<ClassDecl>(this)) {
1095+
bool missingOne = false;
1096+
for (auto ip : InvertibleProtocolSet::full()) {
1097+
auto invertible = getMarking(ip);
1098+
if (!invertible.getInverse() || bool(invertible.getPositive()))
1099+
addSynthesized(ctx.getProtocol(getKnownProtocolKind(ip)));
1100+
else
1101+
missingOne = true;
1102+
}
11051103

1104+
// FIXME: rdar://122289155 (NCGenerics: convert Equatable, Hashable, and RawRepresentable to ~Copyable.)
1105+
if (missingOne)
1106+
return;
1107+
}
11061108
} else if (!canBeCopyable()) {
11071109
return; // No synthesized conformances for move-only nominals.
11081110
}

lib/Sema/TypeCheckInvertible.cpp

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -349,16 +349,26 @@ ProtocolConformance *deriveConformanceForInvertible(Evaluator &evaluator,
349349
// The conformanceDC specifies THE decl context to use for the conformance.
350350
auto generateConformance =
351351
[&](DeclContext *conformanceDC) -> ProtocolConformance * {
352-
// Form a conformance.
353-
auto conformance = ctx.getNormalConformance(
354-
nominal->getDeclaredInterfaceType(), proto, nominal->getLoc(),
355-
conformanceDC, ProtocolConformanceState::Complete,
356-
/*isUnchecked=*/false, /*isPreconcurrency=*/false);
357-
conformance->setSourceKindAndImplyingConformance(
358-
ConformanceEntryKind::Synthesized, nullptr);
359-
360-
nominal->registerProtocolConformance(conformance, /*synthesized=*/true);
361-
return conformance;
352+
if (!isa<ClassDecl>(nominal)) {
353+
// Form a conformance.
354+
auto conformance = ctx.getNormalConformance(
355+
nominal->getDeclaredInterfaceType(), proto, nominal->getLoc(),
356+
conformanceDC, ProtocolConformanceState::Complete,
357+
/*isUnchecked=*/false, /*isPreconcurrency=*/false);
358+
conformance->setSourceKindAndImplyingConformance(
359+
ConformanceEntryKind::Synthesized, nullptr);
360+
361+
nominal->registerProtocolConformance(conformance, /*synthesized=*/true);
362+
return conformance;
363+
} else {
364+
// Classes need to return a built-in conformance to avoid cycles with
365+
// superclasses, such as:
366+
// class Base<T>: IP {}
367+
// class Derived: Base<Derived> {}
368+
return ctx.getBuiltinConformance(nominal->getDeclaredInterfaceType(),
369+
proto,
370+
BuiltinConformanceKind::Synthesized);
371+
}
362372
};
363373

364374
auto generateConditionalConformance = [&]() -> ProtocolConformance * {

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4864,6 +4864,11 @@ static void ensureRequirementsAreSatisfied(ASTContext &ctx,
48644864
auto assocConf = conformance->getAssociatedConformance(depTy, proto);
48654865
if (assocConf.isConcrete()) {
48664866
auto *concrete = assocConf.getConcrete();
4867+
4868+
// Builtin conformances are always available
4869+
if (isa<BuiltinProtocolConformance>(concrete))
4870+
return false;
4871+
48674872
auto replacementTy = dc->mapTypeIntoContext(concrete->getType());
48684873
diagnoseConformanceAvailability(conformance->getLoc(),
48694874
assocConf, where,

0 commit comments

Comments
 (0)