Skip to content

Commit e7fccde

Browse files
committed
Directly detect the cyclic case while computing conditional requirement
The general class of cycle here is when validation asks for the generic signature which triggers requirement checking which forces us to ask for the generic signature of the extension again. We should look into a more principled solution. See rdar://55263708
1 parent 321d9b7 commit e7fccde

File tree

3 files changed

+20
-20
lines changed

3 files changed

+20
-20
lines changed

include/swift/AST/Decl.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1536,6 +1536,7 @@ class GenericContext : private _GenericContext, public DeclContext {
15361536
/// \endcode
15371537
bool isGeneric() const { return getGenericParams() != nullptr; }
15381538
bool hasComputedGenericSignature() const;
1539+
bool isComputingGenericSignature() const;
15391540

15401541
/// Retrieve the trailing where clause for this extension, if any.
15411542
TrailingWhereClause *getTrailingWhereClause() const {

lib/AST/Decl.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,11 @@ GenericParamList *GenericContext::getGenericParams() const {
859859
bool GenericContext::hasComputedGenericSignature() const {
860860
return GenericSigAndBit.getInt();
861861
}
862+
863+
bool GenericContext::isComputingGenericSignature() const {
864+
return getASTContext().evaluator.hasActiveRequest(
865+
GenericSignatureRequest{const_cast<GenericContext*>(this)});
866+
}
862867

863868
GenericSignature *GenericContext::getGenericSignature() const {
864869
return evaluateOrDefault(

lib/AST/ProtocolConformance.cpp

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ void NormalProtocolConformance::differenceAndStoreConditionalRequirements()
531531
assert(CRState == ConditionalRequirementsState::Computing);
532532
CRState = ConditionalRequirementsState::Uncomputed;
533533
};
534-
534+
535535
auto &ctxt = getProtocol()->getASTContext();
536536
auto DC = getDeclContext();
537537
// A non-extension conformance won't have conditional requirements.
@@ -550,30 +550,24 @@ void NormalProtocolConformance::differenceAndStoreConditionalRequirements()
550550
return;
551551
}
552552

553-
// The type is generic, but the extension doesn't have a signature yet, so
554-
// we might be in a recursive validation situation.
555-
if (!ext->hasComputedGenericSignature()) {
556-
// If the extension is invalid, it won't ever get a signature, so we
557-
// "succeed" with an empty result instead.
558-
if (ext->isInvalid()) {
559-
success({});
560-
return;
561-
}
553+
// If the extension is invalid, it won't ever get a signature, so we
554+
// "succeed" with an empty result instead.
555+
if (ext->isInvalid()) {
556+
success({});
557+
return;
558+
}
562559

563-
// Otherwise we'll try again later.
560+
// Recursively validating the signature comes up frequently as expanding
561+
// conformance requirements might re-enter this method. We can at least catch
562+
// this and come back to these requirements later.
563+
//
564+
// FIXME: In the long run, break this cycle in a more principled way.
565+
if (ext->isComputingGenericSignature()) {
564566
failure();
565567
return;
566568
}
567-
568-
// FIXME: All of this will be removed when validateExtension goes away.
569-
auto extensionSig = ext->getGenericSignature();
570-
if (!extensionSig) {
571-
if (auto lazyResolver = ctxt.getLazyResolver()) {
572-
lazyResolver->resolveExtension(ext);
573-
extensionSig = ext->getGenericSignature();
574-
}
575-
}
576569

570+
auto extensionSig = ext->getGenericSignature();
577571
auto canExtensionSig = extensionSig->getCanonicalSignature();
578572
auto canTypeSig = typeSig->getCanonicalSignature();
579573
if (canTypeSig == canExtensionSig) {

0 commit comments

Comments
 (0)