From d017fe3ab4d4e5c274410c86e392cd0ecf1da387 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 17 Feb 2017 17:18:17 -0800 Subject: [PATCH] [GenericSigBuilder] Clean up handling of "inheritance" clauses. Unify the handling of the "inheritance" clauses of a generic type parameter, associated type, or protocol, walking them in a TypeRepr-preserving manner and adding requirements as they are discovered. This sets the stage for providing better source-location information [*]. This eliminates a redundant-but-different code path for protocol "inheritance" clauses, which was using ProtocolDecl::getInheritedProtocols() rather than looking at the actual TypeReprs, and unifies the logic with that of associated types and type parameters. This eliminates a near-DRY violation, sets us up for simplifying the "inherited protocols" part of ProtocolDecl, and sets us up better for the soon-to-be-proposed class C { } protocol P: C { } [*] We still drop it, but now we have a FIXME! --- include/swift/AST/DiagnosticsSema.def | 4 +- include/swift/AST/GenericSignatureBuilder.h | 27 +-- lib/AST/GenericSignatureBuilder.cpp | 213 ++++++++++++-------- test/Generics/superclass_constraint.swift | 16 ++ test/IDE/print_ast_tc_decls_errors.swift | 2 +- 5 files changed, 153 insertions(+), 109 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 6002f7ac70f04..4425303448c30 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -1563,8 +1563,8 @@ ERROR(requires_generic_param_made_equal_to_concrete,none, "same-type requirement makes generic parameter %0 non-generic", (Type)) ERROR(requires_superclass_conflict,none, - "generic parameter %0 cannot be a subclass of both %1 and %2", - (Type, Type, Type)) + "%select{generic parameter |protocol |}0%1 cannot be a subclass of both " + "%2 and %3", (unsigned, Type, Type, Type)) ERROR(recursive_type_reference,none, "%0 %1 references itself", (DescriptiveDeclKind, Identifier)) ERROR(recursive_requirement_reference,none, diff --git a/include/swift/AST/GenericSignatureBuilder.h b/include/swift/AST/GenericSignatureBuilder.h index d7c457292d478..65d48ac80c786 100644 --- a/include/swift/AST/GenericSignatureBuilder.h +++ b/include/swift/AST/GenericSignatureBuilder.h @@ -139,7 +139,7 @@ class RequirementSource : public llvm::FoldingSetNode { /// The actual storage, described by \c storageKind. union { /// The type representation descibing where the requirement came from. - TypeRepr *typeRepr; + const TypeRepr *typeRepr; /// Where a requirement came from. const RequirementRepr *requirementRepr; @@ -194,7 +194,7 @@ class RequirementSource : public llvm::FoldingSetNode { } RequirementSource(Kind kind, const RequirementSource *parent, - TypeRepr *typeRepr) + const TypeRepr *typeRepr) : kind(kind), storageKind(StorageKind::TypeRepr), parent(parent) { assert((static_cast(parent) != isRootKind(kind)) && "Root RequirementSource should not have parent (or vice versa)"); @@ -245,7 +245,7 @@ class RequirementSource : public llvm::FoldingSetNode { /// Retrieve a requirement source representing an explicit requirement /// stated in an 'inheritance' clause. static const RequirementSource *forExplicit(GenericSignatureBuilder &builder, - TypeRepr *typeRepr); + const TypeRepr *typeRepr); /// Retrieve a requirement source representing an explicit requirement /// stated in an 'where' clause. @@ -256,7 +256,7 @@ class RequirementSource : public llvm::FoldingSetNode { /// inferred from some part of a generic declaration's signature, e.g., the /// parameter or result type of a generic function. static const RequirementSource *forInferred(GenericSignatureBuilder &builder, - TypeRepr *typeRepr); + const TypeRepr *typeRepr); /// Retrieve a requirement source representing the requirement signature /// computation for a protocol. @@ -307,7 +307,7 @@ class RequirementSource : public llvm::FoldingSetNode { int compare(const RequirementSource *other) const; /// Retrieve the type representation for this requirement, if there is one. - TypeRepr *getTypeRepr() const { + const TypeRepr *getTypeRepr() const { if (storageKind != StorageKind::TypeRepr) return nullptr; return storage.typeRepr; } @@ -447,20 +447,11 @@ class GenericSignatureBuilder { Type T1, Type T2, const RequirementSource *Source, llvm::function_ref diagnoseMismatch); - /// Add the requirements placed on the given abstract type parameter + /// Add the requirements placed on the given type parameter /// to the given potential archetype. - bool addAbstractTypeParamRequirements( - AbstractTypeParamDecl *decl, - PotentialArchetype *pa, - const RequirementSource *source, - llvm::SmallPtrSetImpl &visited); - - /// Visit all of the types that show up in the list of inherited - /// types. - /// - /// \returns true if any of the invocations of \c visitor returned true. - bool visitInherited(ArrayRef inheritedTypes, - llvm::function_ref visitor); + bool addInheritedRequirements(TypeDecl *decl, PotentialArchetype *pa, + const RequirementSource *parentSource, + llvm::SmallPtrSetImpl &visited); /// Visit all of the potential archetypes. template diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp index a0f10dfd6f448..741c165e7de66 100644 --- a/lib/AST/GenericSignatureBuilder.cpp +++ b/lib/AST/GenericSignatureBuilder.cpp @@ -211,7 +211,7 @@ const RequirementSource *RequirementSource::forAbstract( const RequirementSource *RequirementSource::forExplicit( GenericSignatureBuilder &builder, - TypeRepr *typeRepr) { + const TypeRepr *typeRepr) { REQUIREMENT_SOURCE_FACTORY_BODY(Explicit, nullptr, typeRepr); } @@ -223,7 +223,7 @@ const RequirementSource *RequirementSource::forExplicit( const RequirementSource *RequirementSource::forInferred( GenericSignatureBuilder &builder, - TypeRepr *typeRepr) { + const TypeRepr *typeRepr) { REQUIREMENT_SOURCE_FACTORY_BODY(Inferred, nullptr, typeRepr); } @@ -310,7 +310,7 @@ int RequirementSource::compare(const RequirementSource *other) const { if (thisIsDerived != otherIsDerived) return thisIsDerived ? -1 : +1; - // FIXME: Arbitrary hack to allow later requirement sources to stop on + // FIXME: Arbitrary hack to allow later requirement sources to stomp on // earlier ones. We need a proper ordering here. return +1; } @@ -702,7 +702,6 @@ static int compareDependentTypes( } // Make sure typealiases are properly ordered, to avoid crashers. - // FIXME: Ideally we would eliminate typealiases earlier. if (auto *aa = a->getTypeAliasDecl()) { auto *ab = b->getTypeAliasDecl(); assert(ab != nullptr && "Should have handled this case above"); @@ -717,7 +716,6 @@ static int compareDependentTypes( = ProtocolType::compareProtocols(&protoa, &protob)) return compareProtocols; - // FIXME: Arbitrarily break the result here. if (aa != ab) return aa < ab ? -1 : +1; } @@ -1298,19 +1296,8 @@ bool GenericSignatureBuilder::addGenericParameterRequirements( auto PA = Impl->PotentialArchetypes[Key.findIndexIn(Impl->GenericParams)]; // Add the requirements from the declaration. - // FIXME: addAbstractTypeParamRequirements() should supply the source itself - // based on a parent source. - const RequirementSource *source; - if (GenericParam->getInherited().size() > 0 && - GenericParam->getInherited()[0].getTypeRepr()) { - source = RequirementSource::forExplicit( - *this, - GenericParam->getInherited()[0].getTypeRepr()); - } else { - source = RequirementSource::forAbstract(*this); - } llvm::SmallPtrSet visited; - return addAbstractTypeParamRequirements(GenericParam, PA, source, visited); + return addInheritedRequirements(GenericParam, PA, nullptr, visited); } void GenericSignatureBuilder::addGenericParameter(GenericTypeParamType *GenericParam) { @@ -1334,6 +1321,44 @@ bool GenericSignatureBuilder::addConformanceRequirement(PotentialArchetype *PAT, return addConformanceRequirement(PAT, Proto, Source, Visited); } +/// Visit all of the types that show up in the list of inherited +/// types. +/// +/// \returns true if any of the invocations of \c visitor returned true. +static bool visitInherited( + ArrayRef inheritedTypes, + llvm::function_ref visitor) { + // Local function that (recursively) adds inherited types. + bool isInvalid = false; + std::function visitInherited; + visitInherited = [&](Type inheritedType, const TypeRepr *typeRepr) { + // Decompose protocol compositions. + auto composition = dyn_cast_or_null(typeRepr); + if (auto compositionType + = inheritedType->getAs()) { + unsigned index = 0; + for (auto protoType : compositionType->getProtocols()) { + if (composition && index < composition->getTypes().size()) + visitInherited(protoType, composition->getTypes()[index]); + else + visitInherited(protoType, typeRepr); + + ++index; + } + return; + } + + isInvalid |= visitor(inheritedType, typeRepr); + }; + + // Visit all of the inherited types. + for (auto inherited : inheritedTypes) { + visitInherited(inherited.getType(), inherited.getTypeRepr()); + } + + return isInvalid; +} + bool GenericSignatureBuilder::addConformanceRequirement(PotentialArchetype *PAT, ProtocolDecl *Proto, const RequirementSource *Source, @@ -1348,8 +1373,9 @@ bool GenericSignatureBuilder::addConformanceRequirement(PotentialArchetype *PAT, bool inserted = Visited.insert(Proto).second; assert(inserted); (void) inserted; - - auto InnerSource = Source->viaAbstractProtocolRequirement(*this, Proto); + SWIFT_DEFER { + Visited.erase(Proto); + }; // Use the requirement signature to avoid rewalking the entire protocol. This // cannot compute the requirement signature directly, because that may be @@ -1361,46 +1387,41 @@ bool GenericSignatureBuilder::addConformanceRequirement(PotentialArchetype *PAT, auto subMap = SubstitutionMap::getProtocolSubstitutions( Proto, concreteSelf, ProtocolConformanceRef(Proto)); + auto innerSource = Source->viaAbstractProtocolRequirement(*this, Proto); for (auto rawReq : reqSig->getRequirements()) { auto req = rawReq.subst(subMap); assert(req && "substituting Self in requirement shouldn't fail"); - addRequirement(*req, InnerSource, Visited); - } - } else { - // Add all of the inherited protocol requirements, recursively. - if (auto resolver = getLazyResolver()) - resolver->resolveInheritedProtocols(Proto); - - for (auto InheritedProto : - Proto->getInheritedProtocols(getLazyResolver())) { - if (Visited.count(InheritedProto)) { - markPotentialArchetypeRecursive(T, InheritedProto, InnerSource); - continue; - } - if (addConformanceRequirement(T, InheritedProto, InnerSource, Visited)) + if (addRequirement(*req, innerSource, Visited)) return true; } - // Add requirements for each of the associated types. - for (auto Member : getProtocolMembers(Proto)) { - if (auto AssocType = dyn_cast(Member)) { - // Add requirements placed directly on this associated type. - auto AssocPA = T->getNestedType(AssocType, *this); + return false; + } - if (AssocPA != T) { - if (addAbstractTypeParamRequirements(AssocType, AssocPA, InnerSource, - Visited)) - return true; - } + // Add all of the inherited protocol requirements, recursively. + if (auto resolver = getLazyResolver()) + resolver->resolveInheritedProtocols(Proto); - continue; + if (addInheritedRequirements(Proto, PAT, Source, Visited)) + return true; + + // Add requirements for each of the associated types. + for (auto Member : getProtocolMembers(Proto)) { + if (auto AssocType = dyn_cast(Member)) { + // Add requirements placed directly on this associated type. + auto AssocPA = T->getNestedType(AssocType, *this); + + if (AssocPA != T) { + if (addInheritedRequirements(AssocType, AssocPA, Source, Visited)) + return true; } - // FIXME: Requirement declarations. + continue; } + + // FIXME: Requirement declarations. } - Visited.erase(Proto); return false; } @@ -1507,11 +1528,28 @@ bool GenericSignatureBuilder::addSuperclassRequirement(PotentialArchetype *T, // then the second `U: Foo` constraint introduces a `T == Int` // constraint. } else if (!Superclass->isExactSuperclassOf(T->Superclass, nullptr)) { - Diags.diagnose(Source->getLoc(), - diag::requires_superclass_conflict, - T->getDependentType(/*FIXME: */{ }, true), - T->Superclass, Superclass) - .highlight(T->SuperclassSource->getLoc()); + if (Source->getLoc().isValid()) { + // Figure out what kind of subject we have; it will affect the + // diagnostic. + auto subjectType = T->getDependentType(/*FIXME: */{ }, true); + unsigned kind; + if (auto gp = subjectType->getAs()) { + if (gp->getDecl() && + isa(gp->getDecl()->getDeclContext())) { + kind = 1; + subjectType = cast(gp->getDecl()->getDeclContext()) + ->getDeclaredInterfaceType(); + } else { + kind = 0; + } + } else { + kind = 2; + } + + Diags.diagnose(Source->getLoc(), diag::requires_superclass_conflict, + kind, subjectType, T->Superclass, Superclass) + .highlight(T->SuperclassSource->getLoc()); + } return true; } @@ -1800,36 +1838,61 @@ void GenericSignatureBuilder::markPotentialArchetypeRecursive( assocType->setInvalid(); } -bool GenericSignatureBuilder::addAbstractTypeParamRequirements( - AbstractTypeParamDecl *decl, - PotentialArchetype *pa, - const RequirementSource *source, - llvm::SmallPtrSetImpl &visited) { +bool GenericSignatureBuilder::addInheritedRequirements( + TypeDecl *decl, + PotentialArchetype *pa, + const RequirementSource *parentSource, + llvm::SmallPtrSetImpl &visited) { if (isa(decl) && decl->hasInterfaceType() && decl->getInterfaceType()->is()) return false; - // Otherwise, walk the 'inherited' list to identify requirements. + // Walk the 'inherited' list to identify requirements. if (auto resolver = getLazyResolver()) resolver->resolveInheritanceClause(decl); - return visitInherited(decl->getInherited(), [&](Type inheritedType, - SourceLoc loc) -> bool { + + return visitInherited( + decl->getInherited(), + [&](Type inheritedType, const TypeRepr *typeRepr) -> bool { + // Local function to get the source. + auto getSource = [&] { + if (parentSource) { + if (auto assocType = dyn_cast(decl)) { + // FIXME: Pass along the typeRepr! + auto proto = assocType->getProtocol(); + return parentSource->viaAbstractProtocolRequirement(*this, proto); + } + + // FIXME: Pass along the typeRepr. + auto proto = cast(decl); + return parentSource->viaAbstractProtocolRequirement(*this, proto); + } + + // Explicit requirement. + if (typeRepr) + return RequirementSource::forExplicit(*this, typeRepr); + + // An abstract explicit requirement. + return RequirementSource::forAbstract(*this); + }; + // Protocol requirement. if (auto protocolType = inheritedType->getAs()) { if (visited.count(protocolType->getDecl())) { - markPotentialArchetypeRecursive(pa, protocolType->getDecl(), source); + markPotentialArchetypeRecursive(pa, protocolType->getDecl(), + getSource()); return true; } - return addConformanceRequirement(pa, protocolType->getDecl(), source, + return addConformanceRequirement(pa, protocolType->getDecl(), getSource(), visited); } // Superclass requirement. if (inheritedType->getClassOrBoundGenericClass()) { - return addSuperclassRequirement(pa, inheritedType, source); + return addSuperclassRequirement(pa, inheritedType, getSource()); } // Note: anything else is an error, to be diagnosed later. @@ -1837,32 +1900,6 @@ bool GenericSignatureBuilder::addAbstractTypeParamRequirements( }); } -bool GenericSignatureBuilder::visitInherited( - ArrayRef inheritedTypes, - llvm::function_ref visitor) { - // Local function that (recursively) adds inherited types. - bool isInvalid = false; - std::function visitInherited; - visitInherited = [&](Type inheritedType, SourceLoc loc) { - // Decompose protocol compositions. - if (auto compositionType - = inheritedType->getAs()) { - for (auto protoType : compositionType->getProtocols()) - visitInherited(protoType, loc); - return; - } - - isInvalid |= visitor(inheritedType, loc); - }; - - // Visit all of the inherited types. - for (auto inherited : inheritedTypes) { - visitInherited(inherited.getType(), inherited.getLoc()); - } - - return isInvalid; -} - bool GenericSignatureBuilder::addRequirement(const RequirementRepr *Req) { auto source = RequirementSource::forExplicit(*this, Req); diff --git a/test/Generics/superclass_constraint.swift b/test/Generics/superclass_constraint.swift index 784c0d68a0a1f..6f74e3ed4c17d 100644 --- a/test/Generics/superclass_constraint.swift +++ b/test/Generics/superclass_constraint.swift @@ -87,3 +87,19 @@ class C2 : C, P4 { } // CHECK-NEXT: τ_0_0 : P4 [Explicit @ {{.*}}:46 -> Superclass (C2: P4)] // CHECK: Canonical generic signature: <τ_0_0 where τ_0_0 : C2> func superclassConformance3(t: T) where T : C, T : P4, T : C2 {} + +protocol P5: A { } // expected-error{{non-class type 'P5' cannot inherit from class 'A'}} + +protocol P6: A, Other { } // expected-error{{protocol 'P6' cannot be a subclass of both 'A' and 'Other'}} +// expected-error@-1{{non-class type 'P6' cannot inherit from class 'A'}} +// expected-error@-2{{non-class type 'P6' cannot inherit from class 'Other'}} + +func takeA(_: A) { } +func takeP5(_ t: T) { + takeA(t) // okay +} + +protocol P7 { // expected-error{{'Self.Assoc' cannot be a subclass of both 'A' and 'Other'}} + associatedtype Assoc: A, Other + // FIXME: expected-error@-1{{multiple inheritance from classes 'A' and 'Other'}} +} diff --git a/test/IDE/print_ast_tc_decls_errors.swift b/test/IDE/print_ast_tc_decls_errors.swift index 33fe88c0bb0b2..f226d8aa0a3e6 100644 --- a/test/IDE/print_ast_tc_decls_errors.swift +++ b/test/IDE/print_ast_tc_decls_errors.swift @@ -157,7 +157,7 @@ protocol ProtocolWithInheritance4 : FooClass, FooProtocol {} // expected-error { // NO-TYREPR: {{^}}protocol ProtocolWithInheritance4 : <>, FooProtocol {{{$}} // TYREPR: {{^}}protocol ProtocolWithInheritance4 : FooClass, FooProtocol {{{$}} -protocol ProtocolWithInheritance5 : FooClass, BarClass {} // expected-error {{non-class type 'ProtocolWithInheritance5' cannot inherit from class 'FooClass'}} expected-error {{non-class type 'ProtocolWithInheritance5' cannot inherit from class 'BarClass'}} +protocol ProtocolWithInheritance5 : FooClass, BarClass {} // expected-error {{non-class type 'ProtocolWithInheritance5' cannot inherit from class 'FooClass'}} expected-error {{non-class type 'ProtocolWithInheritance5' cannot inherit from class 'BarClass'}} expected-error{{protocol 'ProtocolWithInheritance5' cannot be a subclass of both 'FooClass' and 'BarClass'}} // NO-TYREPR: {{^}}protocol ProtocolWithInheritance5 : <>, <> {{{$}} // TYREPR: {{^}}protocol ProtocolWithInheritance5 : FooClass, BarClass {{{$}}