diff --git a/include/swift/AST/GenericSignature.h b/include/swift/AST/GenericSignature.h index fa1db48079e14..400060bdbe7b6 100644 --- a/include/swift/AST/GenericSignature.h +++ b/include/swift/AST/GenericSignature.h @@ -175,7 +175,7 @@ class CanGenericSignature : public GenericSignature { /// requirements, first canonicalizing the types. static CanGenericSignature getCanonical(TypeArrayView params, - ArrayRef requirements, bool skipValidation = false); + ArrayRef requirements); public: CanGenericSignature(std::nullptr_t) : GenericSignature(nullptr) {} @@ -342,6 +342,8 @@ class alignas(1 << TypeAlignInBits) GenericSignatureImpl final /// /// The type parameters must be known to not be concrete within the context. bool areSameTypeParameterInContext(Type type1, Type type2) const; + bool areSameTypeParameterInContext(Type type1, Type type2, + GenericSignatureBuilder &builder) const; /// Determine if \c sig can prove \c requirement, meaning that it can deduce /// T: Foo or T == U (etc.) with the information it knows. This includes @@ -360,12 +362,10 @@ class alignas(1 << TypeAlignInBits) GenericSignatureImpl final /// Return the canonical version of the given type under this generic /// signature. CanType getCanonicalTypeInContext(Type type) const; - bool isCanonicalTypeInContext(Type type) const; - - /// Return the canonical version of the given type under this generic - /// signature. CanType getCanonicalTypeInContext(Type type, GenericSignatureBuilder &builder) const; + + bool isCanonicalTypeInContext(Type type) const; bool isCanonicalTypeInContext(Type type, GenericSignatureBuilder &builder) const; diff --git a/lib/AST/GenericSignature.cpp b/lib/AST/GenericSignature.cpp index 0fd81519ba62e..9209ce1d437ed 100644 --- a/lib/AST/GenericSignature.cpp +++ b/lib/AST/GenericSignature.cpp @@ -176,23 +176,9 @@ bool GenericSignatureImpl::isCanonical() const { return getCanonicalSignature().getPointer() == this; } -#ifndef NDEBUG -/// Determine the canonical ordering of requirements. -static unsigned getRequirementKindOrder(RequirementKind kind) { - switch (kind) { - case RequirementKind::Conformance: return 2; - case RequirementKind::Superclass: return 0; - case RequirementKind::SameType: return 3; - case RequirementKind::Layout: return 1; - } - llvm_unreachable("unhandled kind"); -} -#endif - CanGenericSignature CanGenericSignature::getCanonical(TypeArrayView params, - ArrayRef requirements, - bool skipValidation) { + ArrayRef requirements) { // Canonicalize the parameters and requirements. SmallVector canonicalParams; canonicalParams.reserve(params.size()); @@ -205,116 +191,9 @@ CanGenericSignature::getCanonical(TypeArrayView params, for (auto &reqt : requirements) canonicalRequirements.push_back(reqt.getCanonical()); - (void)skipValidation; auto canSig = get(canonicalParams, canonicalRequirements, /*isKnownCanonical=*/true); -#ifndef NDEBUG - if (skipValidation) - return CanGenericSignature(canSig); - - PrettyStackTraceGenericSignature debugStack("canonicalizing", canSig); - - // Check that the signature is canonical. - for (unsigned idx : indices(canonicalRequirements)) { - debugStack.setRequirement(idx); - - const auto &reqt = canonicalRequirements[idx]; - - // Left-hand side must be canonical in its context. - // Check canonicalization of requirement itself. - switch (reqt.getKind()) { - case RequirementKind::Superclass: - assert(canSig->isCanonicalTypeInContext(reqt.getFirstType()) && - "Left-hand side is not canonical"); - assert(canSig->isCanonicalTypeInContext(reqt.getSecondType()) && - "Superclass type isn't canonical in its own context"); - break; - - case RequirementKind::Layout: - assert(canSig->isCanonicalTypeInContext(reqt.getFirstType()) && - "Left-hand side is not canonical"); - break; - - case RequirementKind::SameType: { - auto isCanonicalAnchor = [&](Type type) { - if (auto *dmt = type->getAs()) - return canSig->isCanonicalTypeInContext(dmt->getBase()); - return type->is(); - }; - - auto firstType = reqt.getFirstType(); - auto secondType = reqt.getSecondType(); - assert(isCanonicalAnchor(firstType)); - - if (reqt.getSecondType()->isTypeParameter()) { - assert(isCanonicalAnchor(secondType)); - assert(compareDependentTypes(firstType, secondType) < 0 && - "Out-of-order type parameters in same-type constraint"); - } else { - assert(canSig->isCanonicalTypeInContext(secondType) && - "Concrete same-type isn't canonical in its own context"); - } - break; - } - - case RequirementKind::Conformance: - assert(reqt.getFirstType()->isTypeParameter() && - "Left-hand side must be a type parameter"); - assert(isa(reqt.getSecondType().getPointer()) && - "Right-hand side of conformance isn't a protocol type"); - break; - } - - // From here on, we're only interested in requirements beyond the first. - if (idx == 0) continue; - - // Make sure that the left-hand sides are in nondecreasing order. - const auto &prevReqt = canonicalRequirements[idx-1]; - int compareLHS = - compareDependentTypes(prevReqt.getFirstType(), reqt.getFirstType()); - assert(compareLHS <= 0 && "Out-of-order left-hand sides"); - - // If we have two same-type requirements where the left-hand sides differ - // but fall into the same equivalence class, we can check the form. - if (compareLHS < 0 && reqt.getKind() == RequirementKind::SameType && - prevReqt.getKind() == RequirementKind::SameType && - canSig->areSameTypeParameterInContext(prevReqt.getFirstType(), - reqt.getFirstType())) { - // If it's a it's a type parameter, make sure the equivalence class is - // wired together sanely. - if (prevReqt.getSecondType()->isTypeParameter()) { - assert(prevReqt.getSecondType()->isEqual(reqt.getFirstType()) && - "same-type constraints within an equiv. class are out-of-order"); - } else { - // Otherwise, the concrete types must match up. - assert(prevReqt.getSecondType()->isEqual(reqt.getSecondType()) && - "inconsistent concrete same-type constraints in equiv. class"); - } - } - - // From here on, we only care about cases where the previous and current - // requirements have the same left-hand side. - if (compareLHS != 0) continue; - - // Check ordering of requirement kinds. - assert((getRequirementKindOrder(prevReqt.getKind()) <= - getRequirementKindOrder(reqt.getKind())) && - "Requirements for a given kind are out-of-order"); - - // From here on, we only care about the same requirement kind. - if (prevReqt.getKind() != reqt.getKind()) continue; - - assert(reqt.getKind() == RequirementKind::Conformance && - "Only conformance requirements can have multiples"); - - auto prevProto = prevReqt.getProtocolDecl(); - auto proto = reqt.getProtocolDecl(); - assert(TypeDecl::compare(prevProto, proto) < 0 && - "Out-of-order conformance requirements"); - } -#endif - return CanGenericSignature(canSig); } @@ -519,7 +398,19 @@ bool GenericSignatureImpl::areSameTypeParameterInContext(Type type1, if (type1.getPointer() == type2.getPointer()) return true; - auto &builder = *getGenericSignatureBuilder(); + return areSameTypeParameterInContext(type1, type2, + *getGenericSignatureBuilder()); +} + +bool GenericSignatureImpl::areSameTypeParameterInContext(Type type1, + Type type2, + GenericSignatureBuilder &builder) const { + assert(type1->isTypeParameter()); + assert(type2->isTypeParameter()); + + if (type1.getPointer() == type2.getPointer()) + return true; + auto equivClass1 = builder.resolveEquivalenceClass( type1, diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp index 58e89fb78d26e..99b62d27bac9b 100644 --- a/lib/AST/GenericSignatureBuilder.cpp +++ b/lib/AST/GenericSignatureBuilder.cpp @@ -7948,6 +7948,129 @@ void GenericSignatureBuilder::addGenericSignature(GenericSignature sig) { addRequirement(reqt, FloatingRequirementSource::forAbstract(), nullptr); } +#ifndef NDEBUG + +/// Determine the canonical ordering of requirements. +static unsigned getRequirementKindOrder(RequirementKind kind) { + switch (kind) { + case RequirementKind::Conformance: return 2; + case RequirementKind::Superclass: return 0; + case RequirementKind::SameType: return 3; + case RequirementKind::Layout: return 1; + } + llvm_unreachable("unhandled kind"); +} + +static void checkGenericSignature(CanGenericSignature canSig, + GenericSignatureBuilder &builder) { + PrettyStackTraceGenericSignature debugStack("checking", canSig); + + auto canonicalRequirements = canSig->getRequirements(); + + // Check that the signature is canonical. + for (unsigned idx : indices(canonicalRequirements)) { + debugStack.setRequirement(idx); + + const auto &reqt = canonicalRequirements[idx]; + + // Left-hand side must be canonical in its context. + // Check canonicalization of requirement itself. + switch (reqt.getKind()) { + case RequirementKind::Superclass: + assert(canSig->isCanonicalTypeInContext(reqt.getFirstType(), builder) && + "Left-hand side is not canonical"); + assert(canSig->isCanonicalTypeInContext(reqt.getSecondType(), builder) && + "Superclass type isn't canonical in its own context"); + break; + + case RequirementKind::Layout: + assert(canSig->isCanonicalTypeInContext(reqt.getFirstType(), builder) && + "Left-hand side is not canonical"); + break; + + case RequirementKind::SameType: { + auto isCanonicalAnchor = [&](Type type) { + if (auto *dmt = type->getAs()) + return canSig->isCanonicalTypeInContext(dmt->getBase(), builder); + return type->is(); + }; + + auto firstType = reqt.getFirstType(); + auto secondType = reqt.getSecondType(); + assert(isCanonicalAnchor(firstType)); + + if (reqt.getSecondType()->isTypeParameter()) { + assert(isCanonicalAnchor(secondType)); + assert(compareDependentTypes(firstType, secondType) < 0 && + "Out-of-order type parameters in same-type constraint"); + } else { + assert(canSig->isCanonicalTypeInContext(secondType) && + "Concrete same-type isn't canonical in its own context"); + } + break; + } + + case RequirementKind::Conformance: + assert(canSig->isCanonicalTypeInContext(reqt.getFirstType(), builder) && + "Left-hand side is not canonical"); + assert(reqt.getFirstType()->isTypeParameter() && + "Left-hand side must be a type parameter"); + assert(isa(reqt.getSecondType().getPointer()) && + "Right-hand side of conformance isn't a protocol type"); + break; + } + + // From here on, we're only interested in requirements beyond the first. + if (idx == 0) continue; + + // Make sure that the left-hand sides are in nondecreasing order. + const auto &prevReqt = canonicalRequirements[idx-1]; + int compareLHS = + compareDependentTypes(prevReqt.getFirstType(), reqt.getFirstType()); + assert(compareLHS <= 0 && "Out-of-order left-hand sides"); + + // If we have two same-type requirements where the left-hand sides differ + // but fall into the same equivalence class, we can check the form. + if (compareLHS < 0 && reqt.getKind() == RequirementKind::SameType && + prevReqt.getKind() == RequirementKind::SameType && + canSig->areSameTypeParameterInContext(prevReqt.getFirstType(), + reqt.getFirstType(), + builder)) { + // If it's a it's a type parameter, make sure the equivalence class is + // wired together sanely. + if (prevReqt.getSecondType()->isTypeParameter()) { + assert(prevReqt.getSecondType()->isEqual(reqt.getFirstType()) && + "same-type constraints within an equiv. class are out-of-order"); + } else { + // Otherwise, the concrete types must match up. + assert(prevReqt.getSecondType()->isEqual(reqt.getSecondType()) && + "inconsistent concrete same-type constraints in equiv. class"); + } + } + + // From here on, we only care about cases where the previous and current + // requirements have the same left-hand side. + if (compareLHS != 0) continue; + + // Check ordering of requirement kinds. + assert((getRequirementKindOrder(prevReqt.getKind()) <= + getRequirementKindOrder(reqt.getKind())) && + "Requirements for a given kind are out-of-order"); + + // From here on, we only care about the same requirement kind. + if (prevReqt.getKind() != reqt.getKind()) continue; + + assert(reqt.getKind() == RequirementKind::Conformance && + "Only conformance requirements can have multiples"); + + auto prevProto = prevReqt.getProtocolDecl(); + auto proto = reqt.getProtocolDecl(); + assert(TypeDecl::compare(prevProto, proto) < 0 && + "Out-of-order conformance requirements"); + } +} +#endif + GenericSignature GenericSignatureBuilder::computeGenericSignature( bool allowConcreteGenericParams, bool allowBuilderToMove) && { @@ -7961,6 +8084,12 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature( // Form the generic signature. auto sig = GenericSignature::get(getGenericParams(), requirements); +#ifndef NDEBUG + if (!Impl->HadAnyError) { + checkGenericSignature(sig.getCanonicalSignature(), *this); + } +#endif + // When we can, move this generic signature builder to make it the canonical // builder, rather than constructing a new generic signature builder that // will produce the same thing. diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index c22425d25417e..5f9c4911fb802 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -2452,13 +2452,10 @@ class DeclChecker : public DeclVisitor { requirementsSig->print(llvm::errs()); llvm::errs() << "\n"; - // Note: One cannot canonicalize a requirement signature, because - // requirement signatures are necessarily missing requirements. llvm::errs() << "Canonical requirement signature: "; auto canRequirementSig = CanGenericSignature::getCanonical(requirementsSig->getGenericParams(), - requirementsSig->getRequirements(), - /*skipValidation=*/true); + requirementsSig->getRequirements()); canRequirementSig->print(llvm::errs()); llvm::errs() << "\n"; } diff --git a/validation-test/compiler_crashers_2/sr8968.swift b/validation-test/compiler_crashers_2/sr8968.swift new file mode 100644 index 0000000000000..b42be9911ef5e --- /dev/null +++ b/validation-test/compiler_crashers_2/sr8968.swift @@ -0,0 +1,48 @@ +// RUN: not --crash %target-swift-frontend -emit-ir %s + +// REQUIRES: asserts + +public class OFMAttachment : NativeInserting { + public typealias SnapshotType = Message.Attachment +} + +public protocol Snapshotting { + associatedtype NativeType: NativeInserting where NativeType.SnapshotType == Self + associatedtype RecordType: SnapshotRecord where RecordType.SnapshotType == Self + associatedtype ChangeType: SnapshotChange where ChangeType.SnapshotType == Self + + static var baseMessageName: String { get } +} + +public protocol NativeInserting { + associatedtype SnapshotType : Snapshotting where SnapshotType.NativeType == Self +} + +public protocol SnapshotRecord { + associatedtype SnapshotType : Snapshotting where SnapshotType.RecordType == Self + +} + +public protocol SnapshotChange { + associatedtype SnapshotType : Snapshotting where SnapshotType.ChangeType == Self +} + +public struct Message { + public enum Attachment : Snapshotting { + + public static var baseMessageName: String = "attachment" + + public typealias NativeType = OFMAttachment + public typealias RecordType = Record + public typealias ChangeType = Change + public struct Record : SnapshotRecord { + public typealias SnapshotType = Message.Attachment + } + + public struct Change : SnapshotChange { + public typealias SnapshotType = Message.Attachment + } + + } +} + diff --git a/validation-test/compiler_crashers_2_fixed/sr8968.swift b/validation-test/compiler_crashers_2_fixed/sr8968.swift deleted file mode 100644 index 3206e0dac9558..0000000000000 --- a/validation-test/compiler_crashers_2_fixed/sr8968.swift +++ /dev/null @@ -1,46 +0,0 @@ -// RUN: %target-swift-frontend -typecheck %s - -class OFMAttachment : NativeInserting { - typealias SnapshotType = Message.Attachment -} - -protocol Snapshotting { - associatedtype NativeType: NativeInserting where NativeType.SnapshotType == Self - associatedtype RecordType: SnapshotRecord where RecordType.SnapshotType == Self - associatedtype ChangeType: SnapshotChange where ChangeType.SnapshotType == Self - - static var baseMessageName: String { get } -} - -protocol NativeInserting { - associatedtype SnapshotType : Snapshotting where SnapshotType.NativeType == Self -} - -protocol SnapshotRecord { - associatedtype SnapshotType : Snapshotting where SnapshotType.RecordType == Self - -} - -protocol SnapshotChange { - associatedtype SnapshotType : Snapshotting where SnapshotType.ChangeType == Self -} - -struct Message { - enum Attachment : Snapshotting { - - static var baseMessageName: String = "attachment" - - typealias NativeType = OFMAttachment - typealias RecordType = Record - typealias ChangeType = Change - struct Record : SnapshotRecord { - typealias SnapshotType = Message.Attachment - } - - struct Change : SnapshotChange { - typealias SnapshotType = Message.Attachment - } - - } -} -