Skip to content

Commit f3f9555

Browse files
committed
GSB: New implementation of getConformanceAccessPath()
A new implementation from "first principles". The idea is that for a given conformance, we either have an explicit source which forms the root of the requirement path, or a derived source, which we 'factor' into a parent type/parent protocol pair, and a requirement signature requirement. We recursively compute the conformance access path of the parent type and parent protocol, and append the path element for the requirement. This fixes a long-standing crasher, and eliminates two hacks, the 'usesRequirementSource' flag in RequirementSource, and the 'HadAnyRedundantConstraints' flag in GenericSignatureBuilder. Fixes https://bugs.swift.org/browse/SR-7371 / rdar://problem/39239511
1 parent d492a53 commit f3f9555

File tree

3 files changed

+111
-235
lines changed

3 files changed

+111
-235
lines changed

include/swift/AST/GenericSignature.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -234,13 +234,6 @@ class alignas(1 << TypeAlignInBits) GenericSignatureImpl final
234234
mutable llvm::PointerUnion<const GenericSignatureImpl *, ASTContext *>
235235
CanonicalSignatureOrASTContext;
236236

237-
void buildConformanceAccessPath(
238-
SmallVectorImpl<ConformanceAccessPath::Entry> &path,
239-
ArrayRef<Requirement> reqs,
240-
const void /*GenericSignatureBuilder::RequirementSource*/ *source,
241-
ProtocolDecl *conformingProto, Type rootType,
242-
ProtocolDecl *requirementSignatureProto) const;
243-
244237
friend class ArchetypeType;
245238

246239
public:

lib/AST/GenericSignature.cpp

Lines changed: 110 additions & 225 deletions
Original file line numberDiff line numberDiff line change
@@ -729,16 +729,6 @@ CanGenericSignature::getGenericParams() const{
729729
return {base, params.size()};
730730
}
731731

732-
/// Remove all of the associated type declarations from the given type
733-
/// parameter, producing \c DependentMemberTypes with names alone.
734-
static Type eraseAssociatedTypes(Type type) {
735-
if (auto depMemTy = type->getAs<DependentMemberType>())
736-
return DependentMemberType::get(eraseAssociatedTypes(depMemTy->getBase()),
737-
depMemTy->getName());
738-
739-
return type;
740-
}
741-
742732
namespace {
743733
typedef GenericSignatureBuilder::RequirementSource RequirementSource;
744734

@@ -765,246 +755,141 @@ static bool hasConformanceInSignature(ArrayRef<Requirement> requirements,
765755
return false;
766756
}
767757

768-
/// Check whether the given requirement source has any non-canonical protocol
769-
/// requirements in it.
770-
static bool hasNonCanonicalSelfProtocolRequirement(
771-
const RequirementSource *source,
772-
ProtocolDecl *conformingProto) {
773-
for (; source; source = source->parent) {
774-
// Only look at protocol requirements.
775-
if (!source->isProtocolRequirement())
776-
continue;
777-
778-
// If we don't already have a requirement signature for this protocol,
779-
// build one now.
780-
auto inProto = source->getProtocolDecl();
781-
782-
// Check whether the given requirement is in the requirement signature.
783-
if (!source->usesRequirementSignature &&
784-
!hasConformanceInSignature(inProto->getRequirementSignature(),
785-
source->getStoredType(), conformingProto))
786-
return true;
787-
788-
// Update the conforming protocol for the rest of the search.
789-
conformingProto = inProto;
790-
}
791-
792-
return false;
793-
}
794-
795-
/// Retrieve the best requirement source from the list
796-
static const RequirementSource *
797-
getBestRequirementSource(GenericSignatureBuilder &builder,
798-
ArrayRef<GSBConstraint<ProtocolDecl *>> constraints) {
799-
const RequirementSource *bestSource = nullptr;
800-
bool bestIsNonCanonical = false;
758+
ConformanceAccessPath
759+
GenericSignatureImpl::getConformanceAccessPath(Type type,
760+
ProtocolDecl *protocol) const {
761+
assert(type->isTypeParameter() && "not a type parameter");
801762

802-
auto isBetter = [&](const RequirementSource *source, bool isNonCanonical) {
803-
if (!bestSource) return true;
763+
// Look up the equivalence class for this type.
764+
auto &builder = *getGenericSignatureBuilder();
765+
auto equivClass =
766+
builder.resolveEquivalenceClass(
767+
type,
768+
ArchetypeResolutionKind::CompleteWellFormed);
804769

805-
if (bestIsNonCanonical != isNonCanonical)
806-
return bestIsNonCanonical;
770+
assert(!equivClass->concreteType &&
771+
"Concrete types don't have conformance access paths");
807772

808-
return bestSource->compare(source) > 0;
809-
};
773+
auto cached = equivClass->conformanceAccessPathCache.find(protocol);
774+
if (cached != equivClass->conformanceAccessPathCache.end())
775+
return cached->second;
810776

811-
for (const auto &constraint : constraints) {
812-
auto source = constraint.source;
813-
814-
// Skip self-recursive sources.
815-
bool derivedViaConcrete = false;
816-
if (source->getMinimalConformanceSource(
817-
builder,
818-
constraint.getSubjectDependentType({ }),
819-
constraint.value,
820-
derivedViaConcrete)
821-
!= source)
822-
continue;
823-
824-
// If there is a non-canonical protocol requirement next to the root,
825-
// skip this requirement source.
826-
bool isNonCanonical =
827-
hasNonCanonicalSelfProtocolRequirement(source, constraint.value);
828-
829-
if (isBetter(source, isNonCanonical)) {
830-
bestSource = source;
831-
bestIsNonCanonical = isNonCanonical;
832-
continue;
833-
}
834-
}
777+
// Dig out the conformance of this type to the given protocol, because we
778+
// want its requirement source.
779+
auto conforms = equivClass->conformsTo.find(protocol);
780+
assert(conforms != equivClass->conformsTo.end());
835781

836-
assert(bestSource && "All sources were self-recursive?");
837-
return bestSource;
838-
}
782+
// Look at every requirement source for this conformance. If all sources are
783+
// explicit, leave these three values empty. Otherwise, they are computed
784+
// from the 'best' derived requirement source for this conformance.
785+
Type shortestParentType;
786+
Type shortestSubjectType;
787+
ProtocolDecl *shortestParentProto = nullptr;
788+
789+
auto isShortestPath = [&](Type parentType,
790+
Type subjectType,
791+
ProtocolDecl *parentProto) -> bool {
792+
if (!shortestParentType)
793+
return true;
839794

840-
void GenericSignatureImpl::buildConformanceAccessPath(
841-
SmallVectorImpl<ConformanceAccessPath::Entry> &path,
842-
ArrayRef<Requirement> reqs, const void *opaqueSource,
843-
ProtocolDecl *conformingProto, Type rootType,
844-
ProtocolDecl *requirementSignatureProto) const {
845-
auto *source = reinterpret_cast<const RequirementSource *>(opaqueSource);
846-
// Each protocol requirement is a step along the path.
847-
if (source->isProtocolRequirement()) {
848-
// If we're expanding for a protocol that had no requirement signature
849-
// and have hit the penultimate step, this is the last step
850-
// that would occur in the requirement signature.
851-
Optional<GenericSignatureBuilder> replacementBuilder;
852-
if (!source->parent->parent && requirementSignatureProto) {
853-
// If we have a requirement signature now, we're done.
854-
if (source->usesRequirementSignature) {
855-
Type subjectType = source->getStoredType()->getCanonicalType();
856-
path.push_back({subjectType, conformingProto});
857-
return;
858-
}
795+
int cmpParentTypes = compareDependentTypes(parentType, shortestParentType);
796+
if (cmpParentTypes != 0)
797+
return cmpParentTypes < 0;
859798

860-
// The generic signature builder we're using for this protocol
861-
// wasn't built from its own requirement signature, so we can't
862-
// trust it, build a new generic signature builder.
863-
// FIXME: It would be better if we could replace the canonical generic
864-
// signature builder with the rebuilt one.
799+
int cmpSubjectTypes = compareDependentTypes(subjectType, shortestSubjectType);
800+
if (cmpSubjectTypes != 0)
801+
return cmpSubjectTypes < 0;
865802

866-
replacementBuilder.emplace(getASTContext());
867-
replacementBuilder->addGenericSignature(
868-
requirementSignatureProto->getGenericSignature());
869-
replacementBuilder->processDelayedRequirements();
870-
}
803+
int cmpParentProtos = TypeDecl::compare(parentProto, shortestParentProto);
804+
return cmpParentProtos < 0;
805+
};
871806

872-
// Follow the rest of the path to derive the conformance into which
873-
// this particular protocol requirement step would look.
874-
auto inProtocol = source->getProtocolDecl();
875-
buildConformanceAccessPath(path, reqs, source->parent, inProtocol, rootType,
876-
requirementSignatureProto);
877-
assert(path.back().second == inProtocol &&
878-
"path produces incorrect conformance");
879-
880-
// If this step was computed via the requirement signature, add it
881-
// directly.
882-
if (source->usesRequirementSignature) {
883-
// Add this step along the path, which involves looking for the
884-
// conformance we want (\c conformingProto) within the protocol
885-
// described by this source.
886-
887-
// Canonicalize the subject type within the protocol's generic
888-
// signature.
889-
Type subjectType = source->getStoredType();
890-
subjectType = inProtocol->getGenericSignature()
891-
->getCanonicalTypeInContext(subjectType);
892-
893-
assert(hasConformanceInSignature(inProtocol->getRequirementSignature(),
894-
subjectType, conformingProto) &&
895-
"missing explicit conformance in requirement signature");
896-
897-
// Record this step.
898-
path.push_back({subjectType, conformingProto});
899-
return;
807+
auto recordShortestParentType = [&](Type parentType,
808+
Type subjectType,
809+
ProtocolDecl *parentProto) {
810+
if (isShortestPath(parentType, subjectType, parentProto)) {
811+
shortestParentType = parentType;
812+
shortestSubjectType = subjectType;
813+
shortestParentProto = parentProto;
900814
}
815+
};
901816

902-
// Get the generic signature builder for the protocol.
903-
// Get a generic signature for the protocol's signature.
904-
auto inProtoSig = inProtocol->getGenericSignature();
905-
auto &inProtoSigBuilder =
906-
replacementBuilder ? *replacementBuilder
907-
: *inProtoSig->getGenericSignatureBuilder();
908-
909-
// Retrieve the stored type, but erase all of the specific associated
910-
// type declarations; we don't want any details of the enclosing context
911-
// to sneak in here.
912-
Type storedType = eraseAssociatedTypes(source->getStoredType());
913-
914-
// Dig out the potential archetype for this stored type.
915-
auto equivClass =
916-
inProtoSigBuilder.resolveEquivalenceClass(
917-
storedType,
918-
ArchetypeResolutionKind::CompleteWellFormed);
817+
for (auto constraint : conforms->second) {
818+
auto *source = constraint.source;
919819

920-
// Find the conformance of this potential archetype to the protocol in
921-
// question.
922-
auto conforms = equivClass->conformsTo.find(conformingProto);
923-
assert(conforms != equivClass->conformsTo.end());
924-
925-
// Compute the root type, canonicalizing it w.r.t. the protocol context.
926-
auto conformsSource = getBestRequirementSource(inProtoSigBuilder,
927-
conforms->second);
928-
assert(conformsSource != source || !requirementSignatureProto);
929-
Type localRootType = conformsSource->getRootType();
930-
localRootType = inProtoSig->getCanonicalTypeInContext(localRootType);
931-
932-
// Build the path according to the requirement signature.
933-
buildConformanceAccessPath(path, inProtocol->getRequirementSignature(),
934-
conformsSource, conformingProto, localRootType,
935-
inProtocol);
936-
937-
// We're done.
938-
return;
939-
}
820+
switch (source->kind) {
821+
case RequirementSource::Explicit:
822+
case RequirementSource::Inferred:
823+
// This is not a derived source, so it contributes nothing to the
824+
// "shortest parent type" computation.
825+
break;
940826

941-
// If we have a superclass or concrete requirement, the conformance
942-
// we need is stored in it.
943-
if (source->kind == RequirementSource::Superclass ||
944-
source->kind == RequirementSource::Concrete) {
945-
auto conformance = source->getProtocolConformance();
946-
(void)conformance;
947-
assert(conformance.getRequirement() == conformingProto);
948-
path.push_back({source->getAffectedType(), conformingProto});
949-
return;
950-
}
827+
case RequirementSource::ProtocolRequirement:
828+
case RequirementSource::InferredProtocolRequirement: {
829+
if (source->parent->kind == RequirementSource::RequirementSignatureSelf) {
830+
// This is a top-level requirement in the requirement signature that is
831+
// currently being computed. This is not a derived source, so it
832+
// contributes nothing to the "shortest parent type" computation.
833+
break;
834+
}
951835

952-
// If we still have a parent, keep going.
953-
if (source->parent) {
954-
buildConformanceAccessPath(path, reqs, source->parent, conformingProto,
955-
rootType, requirementSignatureProto);
956-
return;
957-
}
836+
auto constraintType = constraint.getSubjectDependentType({ });
958837

959-
// We are at an explicit or inferred requirement.
960-
assert(source->kind == RequirementSource::Explicit ||
961-
source->kind == RequirementSource::Inferred);
838+
// Skip self-recursive sources.
839+
bool derivedViaConcrete = false;
840+
if (source->getMinimalConformanceSource(builder, constraintType, protocol,
841+
derivedViaConcrete) != source)
842+
break;
962843

963-
// Skip trivial path elements. These occur when querying a requirement
964-
// signature.
965-
if (!path.empty() && conformingProto == path.back().second &&
966-
rootType->isEqual(conformingProto->getSelfInterfaceType()))
967-
return;
968844

969-
assert(hasConformanceInSignature(reqs, rootType, conformingProto) &&
970-
"missing explicit conformance in signature");
845+
// If we have a derived conformance requirement like T[.P].X : Q, we can
846+
// recursively compute the conformance access path for T : P, and append
847+
// the path element (Self.X : Q).
848+
auto parentType = source->parent->getAffectedType()->getCanonicalType();
849+
auto subjectType = source->getStoredType()->getCanonicalType();
971850

972-
// Add the root of the path, which starts at this explicit requirement.
973-
path.push_back({rootType, conformingProto});
974-
}
851+
auto *parentProto = source->getProtocolDecl();
975852

976-
ConformanceAccessPath
977-
GenericSignatureImpl::getConformanceAccessPath(Type type,
978-
ProtocolDecl *protocol) const {
979-
assert(type->isTypeParameter() && "not a type parameter");
853+
// We might have multiple candidate parent types and protocols for the
854+
// recursive step, so pick the shortest one.
855+
recordShortestParentType(parentType, subjectType, parentProto);
980856

981-
// Resolve this type to a potential archetype.
982-
auto &builder = *getGenericSignatureBuilder();
983-
auto equivClass =
984-
builder.resolveEquivalenceClass(
985-
type,
986-
ArchetypeResolutionKind::CompleteWellFormed);
987-
988-
auto cached = equivClass->conformanceAccessPathCache.find(protocol);
989-
if (cached != equivClass->conformanceAccessPathCache.end())
990-
return cached->second;
991-
992-
// Dig out the conformance of this type to the given protocol, because we
993-
// want its requirement source.
994-
auto conforms = equivClass->conformsTo.find(protocol);
995-
assert(conforms != equivClass->conformsTo.end());
857+
break;
858+
}
996859

997-
// Canonicalize the root type.
998-
auto source = getBestRequirementSource(builder, conforms->second);
999-
Type rootType = source->getRootType()->getCanonicalType(this);
860+
default:
861+
// There should be no other way of testifying to a conformance on a
862+
// dependent type.
863+
llvm_unreachable("Bad requirement source for conformance on dependent type");
864+
}
865+
}
1000866

1001-
// Build the path.
1002867
SmallVector<ConformanceAccessPath::Entry, 2> path;
1003-
buildConformanceAccessPath(path, getRequirements(), source, protocol,
1004-
rootType, nullptr);
1005868

1006-
// Return the path; we're done!
869+
if (!shortestParentType) {
870+
// All requirement sources were explicit. This means we can recover the
871+
// conformance directly from the generic signature; canonicalize the
872+
// dependent type and add it as an initial path element.
873+
auto rootType = equivClass->getAnchor(builder, { });
874+
assert(hasConformanceInSignature(getRequirements(), rootType, protocol));
875+
path.emplace_back(rootType, protocol);
876+
} else {
877+
// This conformance comes from a derived source.
878+
//
879+
// To recover this the conformance, we recursively recover the conformance
880+
// of the parent type to the parent protocol first.
881+
auto parentPath = getConformanceAccessPath(
882+
shortestParentType, shortestParentProto);
883+
for (auto entry : parentPath)
884+
path.push_back(entry);
885+
886+
// Then, we add the subject type from the parent protocol's requirement
887+
// signature.
888+
path.emplace_back(shortestSubjectType, protocol);
889+
}
890+
1007891
ConformanceAccessPath result(getASTContext().AllocateCopy(path));
892+
1008893
equivClass->conformanceAccessPathCache.insert({protocol, result});
1009894
return result;
1010895
}

validation-test/compiler_crashers_2/0207-sr7371.swift renamed to validation-test/compiler_crashers_2_fixed/0207-sr7371.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
// RUN: not --crash %target-swift-frontend -emit-ir %s
2-
// rdar://problem/65571199
3-
// UNSUPPORTED: asan
1+
// RUN: %target-swift-frontend -emit-ir %s
42

53
public protocol TypedParserResultTransferType {
64
// Remove type constraint

0 commit comments

Comments
 (0)