From fce1f2c7fe341cee5198ea5bbd061df22ef66a87 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 11 Mar 2021 15:20:19 -0500 Subject: [PATCH 1/2] GSB: getConformanceAccessPath() doesn't need to use getMinimalConformanceSource() Previously we would look for a derived source before an explicit one, on account of the explicit one possibly being redundant. However, the presence of 'self-derived' sources meant that we had to call getMinimalConformanceSource() to ensure the derived sources were actually usable and would not produce an infinite conformance access path. I'd like to remove getMinimalConformanceSource() now that we have an alternate algorithm to identify redundant explicit requirements. Instead, we can handle the explicit case first, by checking for a conformance requirement in the generic signature -- its presence means it was not redundant, by construction. Then once we handle that case, we know we're going to use a derived source, and finding the shortest one seems to be good enough. This fixes the IRGen crash in https://bugs.swift.org/browse/SR-11153; the requirement signatures in that test still have unnecessary same-type requirements printed, so I added a separate RUN: line for those, and it's marked as known-failing with 'not %FileCheck'. --- lib/AST/GenericSignature.cpp | 68 ++++++++++++++---------------------- test/Generics/sr11153.swift | 22 ++++++++++++ 2 files changed, 49 insertions(+), 41 deletions(-) create mode 100644 test/Generics/sr11153.swift diff --git a/lib/AST/GenericSignature.cpp b/lib/AST/GenericSignature.cpp index 0fd81519ba62e..b532e6d7b57ef 100644 --- a/lib/AST/GenericSignature.cpp +++ b/lib/AST/GenericSignature.cpp @@ -776,9 +776,21 @@ GenericSignatureImpl::getConformanceAccessPath(Type type, auto conforms = equivClass->conformsTo.find(protocol); assert(conforms != equivClass->conformsTo.end()); - // Look at every requirement source for this conformance. If all sources are - // explicit, leave these three values empty. Otherwise, they are computed - // from the 'best' derived requirement source for this conformance. + auto rootType = equivClass->getAnchor(builder, { }); + if (hasConformanceInSignature(getRequirements(), rootType, protocol)) { + ConformanceAccessPath::Entry root(rootType, protocol); + ArrayRef path(root); + + ConformanceAccessPath result(builder.getASTContext().AllocateCopy(path)); + equivClass->conformanceAccessPathCache.insert({protocol, result}); + + return result; + } + + // This conformance comes from a derived source. + // + // To recover this the conformance, we recursively recover the conformance + // of the shortest parent type to the parent protocol first. Type shortestParentType; Type shortestSubjectType; ProtocolDecl *shortestParentProto = nullptr; @@ -823,28 +835,13 @@ GenericSignatureImpl::getConformanceAccessPath(Type type, case RequirementSource::ProtocolRequirement: case RequirementSource::InferredProtocolRequirement: { - if (source->parent->kind == RequirementSource::RequirementSignatureSelf) { - // This is a top-level requirement in the requirement signature that is - // currently being computed. This is not a derived source, so it - // contributes nothing to the "shortest parent type" computation. - break; - } - - auto constraintType = constraint.getSubjectDependentType({ }); - - // Skip self-recursive sources. - bool derivedViaConcrete = false; - if (source->getMinimalConformanceSource(builder, constraintType, protocol, - derivedViaConcrete) != source) - break; - + assert(source->parent->kind != RequirementSource::RequirementSignatureSelf); // If we have a derived conformance requirement like T[.P].X : Q, we can // recursively compute the conformance access path for T : P, and append // the path element (Self.X : Q). auto parentType = source->parent->getAffectedType()->getCanonicalType(); auto subjectType = source->getStoredType()->getCanonicalType(); - auto *parentProto = source->getProtocolDecl(); // We might have multiple candidate parent types and protocols for the @@ -861,33 +858,22 @@ GenericSignatureImpl::getConformanceAccessPath(Type type, } } + assert(shortestParentType); + SmallVector path; - if (!shortestParentType) { - // All requirement sources were explicit. This means we can recover the - // conformance directly from the generic signature; canonicalize the - // dependent type and add it as an initial path element. - auto rootType = equivClass->getAnchor(builder, { }); - assert(hasConformanceInSignature(getRequirements(), rootType, protocol)); - path.emplace_back(rootType, protocol); - } else { - // This conformance comes from a derived source. - // - // To recover this the conformance, we recursively recover the conformance - // of the parent type to the parent protocol first. - auto parentPath = getConformanceAccessPath( - shortestParentType, shortestParentProto); - for (auto entry : parentPath) - path.push_back(entry); - - // Then, we add the subject type from the parent protocol's requirement - // signature. - path.emplace_back(shortestSubjectType, protocol); - } + auto parentPath = getConformanceAccessPath( + shortestParentType, shortestParentProto); + for (auto entry : parentPath) + path.push_back(entry); - ConformanceAccessPath result(getASTContext().AllocateCopy(path)); + // Then, we add the subject type from the parent protocol's requirement + // signature. + path.emplace_back(shortestSubjectType, protocol); + ConformanceAccessPath result(builder.getASTContext().AllocateCopy(path)); equivClass->conformanceAccessPathCache.insert({protocol, result}); + return result; } diff --git a/test/Generics/sr11153.swift b/test/Generics/sr11153.swift new file mode 100644 index 0000000000000..bd8470b4e7255 --- /dev/null +++ b/test/Generics/sr11153.swift @@ -0,0 +1,22 @@ +// RUN: %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | not %FileCheck %s +// RUN: %target-swift-frontend -emit-ir %s + +// CHECK: Requirement signature: +public protocol VectorSpace { + associatedtype Field: FieldAlgebra +} + +// CHECK: Requirement signature: +public protocol FieldAlgebra: VectorSpace where Self.Field == Self { + associatedtype ComparableSubalgebra: ComparableFieldAlgebra + static var zero: Self { get } +} + +// CHECK: Requirement signature: +public protocol ComparableFieldAlgebra: FieldAlgebra where Self.ComparableSubalgebra == Self { +} + +// CHECK: Generic signature: +public func test(_ f: F) { + _ = F.ComparableSubalgebra.zero +} From d59f01de5dab0288dc6558969fd5b2a97dcd46eb Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 11 Mar 2021 15:32:35 -0500 Subject: [PATCH 2/2] Add regression test for a fixed crasher This test case was also posted by a developer in https://bugs.swift.org/browse/SR-11153, but it appears to be a different problem that was fixed in Swift 5.2. --- .../Inputs/sr11153_2_other.swift | 17 +++++++++++++++++ .../compiler_crashers_2_fixed/sr11153_2.swift | 15 +++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 validation-test/compiler_crashers_2_fixed/Inputs/sr11153_2_other.swift create mode 100644 validation-test/compiler_crashers_2_fixed/sr11153_2.swift diff --git a/validation-test/compiler_crashers_2_fixed/Inputs/sr11153_2_other.swift b/validation-test/compiler_crashers_2_fixed/Inputs/sr11153_2_other.swift new file mode 100644 index 0000000000000..2b25360b91fb7 --- /dev/null +++ b/validation-test/compiler_crashers_2_fixed/Inputs/sr11153_2_other.swift @@ -0,0 +1,17 @@ +protocol Snapshotting { + associatedtype NativeType: NativeInserting where NativeType.SnapshotType == Self + associatedtype ChangeType: SnapshotChange where ChangeType.SnapshotType == Self +} + +protocol NativeInserting { + associatedtype SnapshotType : Snapshotting where SnapshotType.NativeType == Self +} + +protocol SnapshotProperties : OptionSet where RawValue == Int { + static var all: Self { get } +} + +protocol SnapshotChange { + associatedtype SnapshotType : Snapshotting where SnapshotType.ChangeType == Self + associatedtype PropertiesType : SnapshotProperties +} diff --git a/validation-test/compiler_crashers_2_fixed/sr11153_2.swift b/validation-test/compiler_crashers_2_fixed/sr11153_2.swift new file mode 100644 index 0000000000000..614fbb26a42e3 --- /dev/null +++ b/validation-test/compiler_crashers_2_fixed/sr11153_2.swift @@ -0,0 +1,15 @@ +// RUN: %target-swift-frontend -emit-ir -primary-file %S/Inputs/sr11153_2_other.swift -primary-file %s +// RUN: %target-swift-frontend -emit-ir %S/Inputs/sr11153_2_other.swift %s + +class WatchRegistry { + func single(objectType: S.Type) throws -> Watch + { + return try Watch.singleObject(objectType: S.self, properties: S.ChangeType.PropertiesType.all) + } +} + +class Watch { + static func singleObject(objectType: SnapshotType.Type, properties: SnapshotType.ChangeType.PropertiesType) throws -> Watch { + fatalError() + } +}