Skip to content

Commit 89bc3a6

Browse files
committed
[GSB] Make conformance access paths robust against non-canonical signatures.
With specific type-checking interleavings in large projects, the generic signature builder for the generic signature of a protocol might not have been built from the requirement signature of the protocol, which would lead to malformed conformance access paths. Make this code path more robust by building a new generic signature builder when this happens. This is a stop-gap solution that is suitable for the Swift 4.1 branch; a better solution would be to re-canonicalize the generic signature builder itself somehow, but this carries more risk in the short term. Fixes rdar://problem/37335173.
1 parent fd5d3af commit 89bc3a6

File tree

1 file changed

+48
-14
lines changed

1 file changed

+48
-14
lines changed

lib/AST/GenericSignature.cpp

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -959,20 +959,32 @@ static bool hasNonCanonicalSelfProtocolRequirement(
959959

960960
/// Retrieve the best requirement source from the list
961961
static const RequirementSource *
962-
getBestCanonicalRequirementSource(
963-
ArrayRef<GSBConstraint<ProtocolDecl *>> constraints) {
962+
getBestRequirementSource(ArrayRef<GSBConstraint<ProtocolDecl *>> constraints) {
964963
const RequirementSource *bestSource = nullptr;
964+
bool bestIsNonCanonical = false;
965+
966+
auto isBetter = [&](const RequirementSource *source, bool isNonCanonical) {
967+
if (!bestSource) return true;
968+
969+
if (bestIsNonCanonical != isNonCanonical)
970+
return bestIsNonCanonical;
971+
972+
return bestSource->compare(source) > 0;
973+
};
974+
965975
for (const auto &constraint : constraints) {
966976
auto source = constraint.source;
967977

968978
// If there is a non-canonical protocol requirement next to the root,
969979
// skip this requirement source.
970-
if (hasNonCanonicalSelfProtocolRequirement(source, constraint.value))
971-
continue;
980+
bool isNonCanonical =
981+
hasNonCanonicalSelfProtocolRequirement(source, constraint.value);
972982

973-
// Check whether this is better than our best source.
974-
if (!bestSource || source->compare(bestSource) < 0)
983+
if (isBetter(source, isNonCanonical)) {
975984
bestSource = source;
985+
bestIsNonCanonical = isNonCanonical;
986+
continue;
987+
}
976988
}
977989

978990
return bestSource;
@@ -1008,13 +1020,32 @@ ConformanceAccessPath GenericSignature::getConformanceAccessPath(
10081020
ProtocolDecl *requirementSignatureProto) {
10091021
// Each protocol requirement is a step along the path.
10101022
if (source->isProtocolRequirement()) {
1011-
// If we're expanding for a protocol that has no requirement signature
1012-
// (yet) and have hit the penultimate step, this is the last step
1023+
// If we're expanding for a protocol that had no requirement signature
1024+
// and have hit the penultimate step, this is the last step
10131025
// that would occur in the requirement signature.
1026+
Optional<GenericSignatureBuilder> replacementBuilder;
10141027
if (!source->parent->parent && requirementSignatureProto) {
1015-
Type subjectType = source->getStoredType()->getCanonicalType();
1016-
path.path.push_back({subjectType, conformingProto});
1017-
return;
1028+
// If we have a requirement signature now, we're done.
1029+
if (source->usesRequirementSignature || true) {
1030+
Type subjectType = source->getStoredType()->getCanonicalType();
1031+
path.path.push_back({subjectType, conformingProto});
1032+
return;
1033+
}
1034+
1035+
// The generic signature builder we're using for this protocol
1036+
// wasn't built from its own requirement signature, so we can't
1037+
// trust it. Make sure we have a requirement signature, then build
1038+
// a new generic signature builder.
1039+
// FIXME: It would be better if we could replace the canonical generic
1040+
// signature builder with the rebuilt one.
1041+
if (!requirementSignatureProto->isRequirementSignatureComputed())
1042+
requirementSignatureProto->computeRequirementSignature();
1043+
assert(requirementSignatureProto->isRequirementSignatureComputed());
1044+
1045+
replacementBuilder.emplace(getASTContext());
1046+
replacementBuilder->addGenericSignature(
1047+
requirementSignatureProto->getGenericSignature());
1048+
replacementBuilder->processDelayedRequirements();
10181049
}
10191050

10201051
// Follow the rest of the path to derive the conformance into which
@@ -1047,9 +1078,12 @@ ConformanceAccessPath GenericSignature::getConformanceAccessPath(
10471078
return;
10481079
}
10491080

1081+
// Get the generic signature builder for the protocol.
10501082
// Get a generic signature for the protocol's signature.
10511083
auto inProtoSig = inProtocol->getGenericSignature();
1052-
auto &inProtoSigBuilder = *inProtoSig->getGenericSignatureBuilder();
1084+
auto &inProtoSigBuilder =
1085+
replacementBuilder ? *replacementBuilder
1086+
: *inProtoSig->getGenericSignatureBuilder();
10531087

10541088
// Retrieve the stored type, but erase all of the specific associated
10551089
// type declarations; we don't want any details of the enclosing context
@@ -1068,7 +1102,7 @@ ConformanceAccessPath GenericSignature::getConformanceAccessPath(
10681102
assert(conforms != equivClass->conformsTo.end());
10691103

10701104
// Compute the root type, canonicalizing it w.r.t. the protocol context.
1071-
auto conformsSource = getBestCanonicalRequirementSource(conforms->second);
1105+
auto conformsSource = getBestRequirementSource(conforms->second);
10721106
assert(conformsSource != source || !requirementSignatureProto);
10731107
Type localRootType = conformsSource->getRootType();
10741108
localRootType = inProtoSig->getCanonicalTypeInContext(localRootType);
@@ -1116,7 +1150,7 @@ ConformanceAccessPath GenericSignature::getConformanceAccessPath(
11161150
};
11171151

11181152
// Canonicalize the root type.
1119-
auto source = getBestCanonicalRequirementSource(conforms->second);
1153+
auto source = getBestRequirementSource(conforms->second);
11201154
Type rootType = source->getRootType()->getCanonicalType(this);
11211155

11221156
// Build the path.

0 commit comments

Comments
 (0)