From 995de881c7072402f600e7e8a8376bc3fa64e167 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 11 Apr 2017 15:01:06 -0700 Subject: [PATCH 1/4] [GSB] Teach addInheritedRequirements() and its callers to use UnresolvedType. Teach addInheritedRequirements() to take an UnresolvedType, so the requirements it adds can be delayed if needed. More importantly, teach the primary caller (addConformanceRequirement) not to use getNestedType directly; instead, form an appropriate Type and let the type-resolution machinery handle it. --- include/swift/AST/GenericSignatureBuilder.h | 2 +- lib/AST/GenericSignatureBuilder.cpp | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/include/swift/AST/GenericSignatureBuilder.h b/include/swift/AST/GenericSignatureBuilder.h index c594d9de585db..a51cc40a12af1 100644 --- a/include/swift/AST/GenericSignatureBuilder.h +++ b/include/swift/AST/GenericSignatureBuilder.h @@ -370,7 +370,7 @@ class GenericSignatureBuilder { /// relative to the given module. ConstraintResult addInheritedRequirements( TypeDecl *decl, - PotentialArchetype *pa, + UnresolvedType type, const RequirementSource *parentSource, llvm::SmallPtrSetImpl &visited, ModuleDecl *inferForModule); diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp index 42d8aa1729222..63dbfe1ed4262 100644 --- a/lib/AST/GenericSignatureBuilder.cpp +++ b/lib/AST/GenericSignatureBuilder.cpp @@ -2335,10 +2335,9 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement( for (auto Member : getProtocolMembers(Proto)) { if (auto AssocType = dyn_cast(Member)) { // Add requirements placed directly on this associated type. - auto AssocPA = PAT->getNestedType(AssocType, *this); - + Type assocType = DependentMemberType::get(concreteSelf, AssocType); auto assocResult = - addInheritedRequirements(AssocType, AssocPA, Source, Visited, + addInheritedRequirements(AssocType, assocType, Source, Visited, protoModule); if (isErrorResult(assocResult)) return assocResult; @@ -2964,7 +2963,7 @@ void GenericSignatureBuilder::markPotentialArchetypeRecursive( ConstraintResult GenericSignatureBuilder::addInheritedRequirements( TypeDecl *decl, - PotentialArchetype *pa, + UnresolvedType type, const RequirementSource *parentSource, llvm::SmallPtrSetImpl &visited, ModuleDecl *inferForModule) { @@ -3011,7 +3010,7 @@ ConstraintResult GenericSignatureBuilder::addInheritedRequirements( getFloatingSource(typeRepr, /*forInferred=*/true)); } - return addTypeRequirement(pa, inheritedType, + return addTypeRequirement(type, inheritedType, getFloatingSource(typeRepr, /*forInferred=*/false), UnresolvedHandlingKind::GenerateConstraints, @@ -3019,7 +3018,7 @@ ConstraintResult GenericSignatureBuilder::addInheritedRequirements( }; auto visitLayout = [&](LayoutConstraint layout, const TypeRepr *typeRepr) { - return addLayoutRequirement(pa, layout, + return addLayoutRequirement(type, layout, getFloatingSource(typeRepr, /*forInferred=*/false), UnresolvedHandlingKind::GenerateConstraints); From 17846e2be1c49f8431aafedb174a9d15965c7dcb Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 11 Apr 2017 17:04:02 -0700 Subject: [PATCH 2/4] [GSB] Cope with recursive requirements by delaying them. Rather than detecting recursion and bailing early, delay requirements that would form recursive types. Note that we aren't actually processing them later. --- include/swift/AST/GenericSignatureBuilder.h | 52 +++++-- lib/AST/Decl.cpp | 6 +- lib/AST/GenericEnvironment.cpp | 5 +- lib/AST/GenericSignature.cpp | 43 ++++-- lib/AST/GenericSignatureBuilder.cpp | 162 +++++++++++++++----- lib/Sema/TypeCheckGeneric.cpp | 30 ++-- 6 files changed, 220 insertions(+), 78 deletions(-) diff --git a/include/swift/AST/GenericSignatureBuilder.h b/include/swift/AST/GenericSignatureBuilder.h index a51cc40a12af1..2b099b95fbe2f 100644 --- a/include/swift/AST/GenericSignatureBuilder.h +++ b/include/swift/AST/GenericSignatureBuilder.h @@ -59,6 +59,22 @@ class TypeRepr; class ASTContext; class DiagnosticEngine; +/// Determines how to resolve a dependent type to a potential archetype. +enum class ArchetypeResolutionKind { + /// Always create a new potential archetype to describe this dependent type, + /// which might be invalid and may not provide complete information. + AlwaysPartial, + + /// Only create a potential archetype when it is well-formed (e.g., a nested + /// type should exist) and make sure we have complete information about + /// that potential archetype. + CompleteWellFormed, + + /// Only create a new potential archetype to describe this dependent type + /// if it is already known. + AlreadyKnown, +}; + /// \brief Collects a set of requirements of generic parameters, both explicitly /// stated and inferred, and determines the set of archetypes for each of /// the generic parameters. @@ -623,14 +639,15 @@ class GenericSignatureBuilder { public: /// \brief Resolve the given type to the potential archetype it names. /// - /// This routine will synthesize nested types as required to refer to a - /// potential archetype, even in cases where no requirement specifies the - /// requirement for such an archetype. FIXME: The failure to include such a - /// requirement will be diagnosed at some point later (when the types in the - /// signature are fully resolved). + /// The \c resolutionKind parameter describes how resolution should be + /// performed. If the potential archetype named by the given dependent type + /// already exists, it will be always returned. If it doesn't exist yet, + /// the \c resolutionKind dictates whether the potential archetype will + /// be created or whether null will be returned. /// /// For any type that cannot refer to an archetype, this routine returns null. - PotentialArchetype *resolveArchetype(Type type); + PotentialArchetype *resolveArchetype(Type type, + ArchetypeResolutionKind resolutionKind); /// \brief Resolve the given type as far as this Builder knows how. /// @@ -1171,6 +1188,10 @@ class GenericSignatureBuilder::FloatingRequirementSource { /// Return the "inferred" version of this source, if it isn't already /// inferred. FloatingRequirementSource asInferred(const TypeRepr *typeRepr) const; + + /// Whether this requirement source is recursive when composed with + /// the given type. + bool isRecursive(Type rootType, GenericSignatureBuilder &builder) const; }; class GenericSignatureBuilder::PotentialArchetype { @@ -1504,15 +1525,6 @@ class GenericSignatureBuilder::PotentialArchetype { PotentialArchetype *getNestedType(TypeAliasDecl *typealias, GenericSignatureBuilder &builder); - /// \brief Retrieve (or create) a nested type that is the current best - /// nested archetype anchor (locally) with the given name. - /// - /// When called on the archetype anchor, this will produce the named - /// archetype anchor. - PotentialArchetype *getNestedArchetypeAnchor( - Identifier name, - GenericSignatureBuilder &builder); - /// Describes the kind of update that is performed. enum class NestedTypeUpdate { /// Resolve an existing potential archetype, but don't create a new @@ -1525,6 +1537,16 @@ class GenericSignatureBuilder::PotentialArchetype { AddIfBetterAnchor, }; + /// \brief Retrieve (or create) a nested type that is the current best + /// nested archetype anchor (locally) with the given name. + /// + /// When called on the archetype anchor, this will produce the named + /// archetype anchor. + PotentialArchetype *getNestedArchetypeAnchor( + Identifier name, + GenericSignatureBuilder &builder, + NestedTypeUpdate kind = NestedTypeUpdate::AddIfMissing); + /// Update the named nested type when we know this type conforms to the given /// protocol. /// diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index f17ffa17843ff..3c48f67c29375 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -3130,10 +3130,14 @@ void ProtocolDecl::computeRequirementSignature() { GenericSignatureBuilder builder(getASTContext(), LookUpConformanceInModule(module)); builder.addGenericParameter(selfType); + auto selfPA = + builder.resolveArchetype(selfType, + ArchetypeResolutionKind::CompleteWellFormed); + builder.addRequirement( requirement, GenericSignatureBuilder::RequirementSource - ::forRequirementSignature(builder.resolveArchetype(selfType), this), + ::forRequirementSignature(selfPA, this), nullptr); builder.finalize(SourceLoc(), { selfType }); diff --git a/lib/AST/GenericEnvironment.cpp b/lib/AST/GenericEnvironment.cpp index 64313aeb3883a..0dcadc66de930 100644 --- a/lib/AST/GenericEnvironment.cpp +++ b/lib/AST/GenericEnvironment.cpp @@ -201,7 +201,10 @@ Type GenericEnvironment::QueryInterfaceTypeSubstitutions::operator()( Type contextType = self->getContextTypes()[index]; if (!contextType) { assert(self->Builder && "Missing generic signature builder for lazy query"); - auto potentialArchetype = self->Builder->resolveArchetype(type); + auto potentialArchetype = + self->Builder->resolveArchetype( + type, + ArchetypeResolutionKind::CompleteWellFormed); auto mutableSelf = const_cast(self); contextType = diff --git a/lib/AST/GenericSignature.cpp b/lib/AST/GenericSignature.cpp index 15c4e08341f61..004bd6ec757b9 100644 --- a/lib/AST/GenericSignature.cpp +++ b/lib/AST/GenericSignature.cpp @@ -489,7 +489,8 @@ bool GenericSignature::requiresClass(Type type, ModuleDecl &mod) { if (!type->isTypeParameter()) return false; auto &builder = *getGenericSignatureBuilder(mod); - auto pa = builder.resolveArchetype(type); + auto pa = + builder.resolveArchetype(type, ArchetypeResolutionKind::CompleteWellFormed); if (!pa) return false; pa = pa->getRepresentative(); @@ -519,7 +520,8 @@ Type GenericSignature::getSuperclassBound(Type type, ModuleDecl &mod) { if (!type->isTypeParameter()) return nullptr; auto &builder = *getGenericSignatureBuilder(mod); - auto pa = builder.resolveArchetype(type); + auto pa = + builder.resolveArchetype(type, ArchetypeResolutionKind::CompleteWellFormed); if (!pa) return nullptr; pa = pa->getRepresentative(); @@ -539,7 +541,8 @@ SmallVector GenericSignature::getConformsTo(Type type, if (!type->isTypeParameter()) return { }; auto &builder = *getGenericSignatureBuilder(mod); - auto pa = builder.resolveArchetype(type); + auto pa = + builder.resolveArchetype(type, ArchetypeResolutionKind::CompleteWellFormed); if (!pa) return { }; pa = pa->getRepresentative(); @@ -565,7 +568,8 @@ bool GenericSignature::conformsToProtocol(Type type, ProtocolDecl *proto, if (!type->isTypeParameter()) return false; auto &builder = *getGenericSignatureBuilder(mod); - auto pa = builder.resolveArchetype(type); + auto pa = + builder.resolveArchetype(type, ArchetypeResolutionKind::CompleteWellFormed); if (!pa) return false; pa = pa->getRepresentative(); @@ -593,7 +597,8 @@ Type GenericSignature::getConcreteType(Type type, ModuleDecl &mod) { if (!type->isTypeParameter()) return Type(); auto &builder = *getGenericSignatureBuilder(mod); - auto pa = builder.resolveArchetype(type); + auto pa = + builder.resolveArchetype(type, ArchetypeResolutionKind::CompleteWellFormed); if (!pa) return Type(); pa = pa->getRepresentative(); @@ -607,7 +612,8 @@ LayoutConstraint GenericSignature::getLayoutConstraint(Type type, if (!type->isTypeParameter()) return LayoutConstraint(); auto &builder = *getGenericSignatureBuilder(mod); - auto pa = builder.resolveArchetype(type); + auto pa = + builder.resolveArchetype(type, ArchetypeResolutionKind::CompleteWellFormed); if (!pa) return LayoutConstraint(); pa = pa->getRepresentative(); @@ -623,12 +629,16 @@ bool GenericSignature::areSameTypeParameterInContext(Type type1, Type type2, return true; auto &builder = *getGenericSignatureBuilder(mod); - auto pa1 = builder.resolveArchetype(type1); + auto pa1 = + builder.resolveArchetype(type1, + ArchetypeResolutionKind::CompleteWellFormed); assert(pa1 && "not a valid dependent type of this signature?"); pa1 = pa1->getRepresentative(); assert(!pa1->isConcreteType()); - auto pa2 = builder.resolveArchetype(type2); + auto pa2 = + builder.resolveArchetype(type2, + ArchetypeResolutionKind::CompleteWellFormed); assert(pa2 && "not a valid dependent type of this signature?"); pa2 = pa2->getRepresentative(); assert(!pa2->isConcreteType()); @@ -667,7 +677,9 @@ bool GenericSignature::isCanonicalTypeInContext(Type type, return !type.findIf([&](Type component) -> bool { if (!component->isTypeParameter()) return false; - auto pa = builder.resolveArchetype(component); + auto pa = + builder.resolveArchetype(component, + ArchetypeResolutionKind::CompleteWellFormed); if (!pa) return false; auto rep = pa->getArchetypeAnchor(builder); @@ -692,7 +704,9 @@ CanType GenericSignature::getCanonicalTypeInContext(Type type, // Resolve the potential archetype. This can be null in nested generic // types, which we can't immediately canonicalize. - auto pa = builder.resolveArchetype(Type(component)); + auto pa = + builder.resolveArchetype(Type(component), + ArchetypeResolutionKind::CompleteWellFormed); if (!pa) return None; auto rep = pa->getArchetypeAnchor(builder); @@ -768,7 +782,8 @@ ConformanceAccessPath GenericSignature::getConformanceAccessPath( // Resolve this type to a potential archetype. auto &builder = *getGenericSignatureBuilder(mod); - auto pa = builder.resolveArchetype(type); + auto pa = + builder.resolveArchetype(type, ArchetypeResolutionKind::CompleteWellFormed); auto equivClass = pa->getOrCreateEquivalenceClass(); // Dig out the conformance of this type to the given protocol, because we @@ -859,7 +874,11 @@ ConformanceAccessPath GenericSignature::getConformanceAccessPath( Type storedType = eraseAssociatedTypes(source->getStoredType()); // Dig out the potential archetype for this stored type. - auto pa = reqSigBuilder.resolveArchetype(storedType); + // FIXME: CompleteWellFormed here? + auto pa = + reqSigBuilder.resolveArchetype( + storedType, + ArchetypeResolutionKind::AlwaysPartial); auto equivClass = pa->getOrCreateEquivalenceClass(); // Find the conformance of this potential archetype to the protocol in diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp index 63dbfe1ed4262..f52e72196aeff 100644 --- a/lib/AST/GenericSignatureBuilder.cpp +++ b/lib/AST/GenericSignatureBuilder.cpp @@ -918,6 +918,7 @@ bool FloatingRequirementSource::isExplicit() const { } } + FloatingRequirementSource FloatingRequirementSource::asInferred( const TypeRepr *typeRepr) const { switch (kind) { @@ -935,6 +936,24 @@ FloatingRequirementSource FloatingRequirementSource::asInferred( } } +bool FloatingRequirementSource::isRecursive( + Type rootType, + GenericSignatureBuilder &builder) const { + llvm::SmallSet, 4> visitedAssocReqs; + for (auto storedSource = storage.dyn_cast(); + storedSource; storedSource = storedSource->parent) { + if (storedSource->kind != RequirementSource::ProtocolRequirement) + continue; + + if (!visitedAssocReqs.insert( + {storedSource->getStoredType()->getCanonicalType(), + storedSource->getProtocolDecl()}).second) + return true; + } + + return false; +} + GenericSignatureBuilder::PotentialArchetype::~PotentialArchetype() { for (const auto &nested : NestedTypes) { for (auto pa : nested.second) { @@ -1478,9 +1497,9 @@ PotentialArchetype *PotentialArchetype::getNestedType( Identifier nestedName, GenericSignatureBuilder &builder) { // If we already have a nested type with this name, return it. - if (!NestedTypes[nestedName].empty()) { - return NestedTypes[nestedName].front(); - } + auto known = NestedTypes.find(nestedName); + if (known != NestedTypes.end()) + return known->second.front(); // Retrieve the nested archetype anchor, which is the best choice (so far) // for this nested type. @@ -1503,7 +1522,8 @@ PotentialArchetype *PotentialArchetype::getNestedType( PotentialArchetype *PotentialArchetype::getNestedArchetypeAnchor( Identifier name, - GenericSignatureBuilder &builder) { + GenericSignatureBuilder &builder, + NestedTypeUpdate kind) { // Look for the best associated type or typealias within the protocols // we know about. AssociatedTypeDecl *bestAssocType = nullptr; @@ -1570,6 +1590,17 @@ PotentialArchetype *PotentialArchetype::getNestedArchetypeAnchor( if (resultPA) return resultPA; + // Check whether we can add a missing nested type for this case. + switch (kind) { + case NestedTypeUpdate::AddIfBetterAnchor: + case NestedTypeUpdate::AddIfMissing: + break; + + case NestedTypeUpdate::ResolveExisting: + // Don't add a new type; + return nullptr; + } + // Build an unresolved type if we don't have one yet. auto &nested = NestedTypes[name]; if (nested.empty()) { @@ -1639,34 +1670,36 @@ PotentialArchetype *PotentialArchetype::updateNestedTypeForConformance( // now) or a potential archetype with the appropriate associated type or // typealias. PotentialArchetype *resultPA = nullptr; - auto &allNested = NestedTypes[name]; + auto knownNestedTypes = NestedTypes.find(name); bool shouldUpdatePA = false; auto &builder = *getBuilder(); - for (auto existingPA : allNested) { - // Resolve an unresolved potential archetype. - if (existingPA->isUnresolvedNestedType) { - if (assocType) { - existingPA->resolveAssociatedType(assocType, builder); - } else { - existingPA->resolveTypeAlias(typealias, builder); - } + if (knownNestedTypes != NestedTypes.end()) { + for (auto existingPA : knownNestedTypes->second) { + // Resolve an unresolved potential archetype. + if (existingPA->isUnresolvedNestedType) { + if (assocType) { + existingPA->resolveAssociatedType(assocType, builder); + } else { + existingPA->resolveTypeAlias(typealias, builder); + } - // We've resolved this nested type; nothing more to do. - resultPA = existingPA; - shouldUpdatePA = true; - break; - } + // We've resolved this nested type; nothing more to do. + resultPA = existingPA; + shouldUpdatePA = true; + break; + } - // Do we have an associated-type match? - if (assocType && existingPA->getResolvedAssociatedType() == assocType) { - resultPA = existingPA; - break; - } + // Do we have an associated-type match? + if (assocType && existingPA->getResolvedAssociatedType() == assocType) { + resultPA = existingPA; + break; + } - // Do we have a typealias match? - if (typealias && existingPA->getTypeAliasDecl() == typealias) { - resultPA = existingPA; - break; + // Do we have a typealias match? + if (typealias && existingPA->getTypeAliasDecl() == typealias) { + resultPA = existingPA; + break; + } } } @@ -1685,6 +1718,7 @@ PotentialArchetype *PotentialArchetype::updateNestedTypeForConformance( else resultPA = new PotentialArchetype(this, typealias); + auto &allNested = NestedTypes[name]; allNested.push_back(resultPA); // We created a new type, which might be equivalent to a type by the @@ -1858,7 +1892,10 @@ Type GenericSignatureBuilder::PotentialArchetype::getTypeInContext( builder.getLookupConformanceFn()); // If the member type maps to an archetype, resolve that archetype. - if (auto memberPA = builder.resolveArchetype(memberType)) { + if (auto memberPA = + builder.resolveArchetype( + memberType, + ArchetypeResolutionKind::CompleteWellFormed)) { if (memberPA->getRepresentative() != representative) { return memberPA->getTypeInContext(builder, genericEnv); } @@ -1953,7 +1990,9 @@ void ArchetypeType::resolveNestedType( Type interfaceType = genericEnv->mapTypeOutOfContext(const_cast(this)); - auto parentPA = builder.resolveArchetype(interfaceType); + auto parentPA = + builder.resolveArchetype(interfaceType, + ArchetypeResolutionKind::CompleteWellFormed); auto memberPA = parentPA->getNestedType(nested.first, builder); auto result = memberPA->getTypeInContext(builder, genericEnv); assert(!nested.second || @@ -2132,7 +2171,9 @@ LazyResolver *GenericSignatureBuilder::getLazyResolver() const { return Context.getLazyResolver(); } -auto GenericSignatureBuilder::resolveArchetype(Type type) -> PotentialArchetype * { +PotentialArchetype *GenericSignatureBuilder::resolveArchetype( + Type type, + ArchetypeResolutionKind resolutionKind) { if (auto genericParam = type->getAs()) { unsigned index = GenericParamKey(genericParam).findIndexIn( Impl->GenericParams); @@ -2143,14 +2184,43 @@ auto GenericSignatureBuilder::resolveArchetype(Type type) -> PotentialArchetype } if (auto dependentMember = type->getAs()) { - auto base = resolveArchetype(dependentMember->getBase()); + auto base = resolveArchetype(dependentMember->getBase(), resolutionKind); if (!base) return nullptr; + // Figure out what kind of nested type update we want. + typedef PotentialArchetype::NestedTypeUpdate NestedTypeUpdate; + NestedTypeUpdate updateKind; + switch (resolutionKind) { + case ArchetypeResolutionKind::AlreadyKnown: + updateKind = NestedTypeUpdate::ResolveExisting; + break; + + case ArchetypeResolutionKind::AlwaysPartial: + case ArchetypeResolutionKind::CompleteWellFormed: + updateKind = NestedTypeUpdate::AddIfMissing; + break; + } + + // If we know the associated type already, get that specific type. if (auto assocType = dependentMember->getAssocType()) - return base->getNestedType(assocType, *this); + return base->updateNestedTypeForConformance(assocType, updateKind); + + // Resolve based on name alone. + auto name = dependentMember->getName(); + switch (resolutionKind) { + case ArchetypeResolutionKind::AlreadyKnown: { + auto known = base->NestedTypes.find(name); + if (known == base->NestedTypes.end()) + return nullptr; + + return known->second.front(); + } - return base->getNestedType(dependentMember->getName(), *this); + case ArchetypeResolutionKind::AlwaysPartial: + case ArchetypeResolutionKind::CompleteWellFormed: + return base->getNestedArchetypeAnchor(name, *this, updateKind); + } } return nullptr; @@ -2161,11 +2231,20 @@ auto GenericSignatureBuilder::resolve(UnresolvedType paOrT, -> Optional { auto pa = paOrT.dyn_cast(); if (auto type = paOrT.dyn_cast()) { - // FIXME: Limit the resolution of the archetype based on the source. - pa = resolveArchetype(type); - if (!pa) { + // If it's not a type parameter, + if (!type->isTypeParameter()) return ResolvedType::forConcreteType(type); - } + + // Determine what kind of resolution we want. + ArchetypeResolutionKind resolutionKind = + ArchetypeResolutionKind::AlwaysPartial; + if (!source.isExplicit() && source.isRecursive(type, *this)) + resolutionKind = ArchetypeResolutionKind::AlreadyKnown; + + // Attempt to resolve the type parameter to a potential archetype. If this + // fails, it's because we weren't allowed to resolve anything now. + pa = resolveArchetype(type, resolutionKind); + if (!pa) return None; } auto rep = pa->getRepresentative(); @@ -2276,6 +2355,7 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement( if (!PAT->addConformance(Proto, Source, *this)) return ConstraintResult::Resolved; +#if false // FIXME: Ad hoc recursion breaking. if (Visited.count(Proto)) { markPotentialArchetypeRecursive(PAT, Proto, Source); @@ -2288,6 +2368,7 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement( SWIFT_DEFER { Visited.erase(Proto); }; +#endif auto concreteSelf = PAT->getDependentType({}, /*allowUnresolved=*/true); auto protocolSubMap = SubstitutionMap::getProtocolSubstitutions( @@ -3444,7 +3525,9 @@ GenericSignatureBuilder::finalize(SourceLoc loc, if (concreteType->hasTypeParameter()) { concreteType.visit([&](Type type) { if (type->isTypeParameter()) { - if (auto referencedPA = resolveArchetype(type)) { + if (auto referencedPA = + resolveArchetype(type, + ArchetypeResolutionKind::AlreadyKnown)) { referencedPAs.insert(referencedPA->getRepresentative()); } } @@ -3470,7 +3553,8 @@ GenericSignatureBuilder::finalize(SourceLoc loc, return type.findIf([&](Type type) { if (type->isTypeParameter()) { - if (auto referencedPA = resolveArchetype(type)) { + if (auto referencedPA = + resolveArchetype(type, ArchetypeResolutionKind::AlreadyKnown)) { referencedPA = referencedPA->getRepresentative(); if (referencedPA == archetype) return true; diff --git a/lib/Sema/TypeCheckGeneric.cpp b/lib/Sema/TypeCheckGeneric.cpp index 5ff8c5a2fd110..342f58a2910df 100644 --- a/lib/Sema/TypeCheckGeneric.cpp +++ b/lib/Sema/TypeCheckGeneric.cpp @@ -42,7 +42,8 @@ Type DependentGenericTypeResolver::resolveDependentMemberType( DeclContext *DC, SourceRange baseRange, ComponentIdentTypeRepr *ref) { - auto archetype = Builder.resolveArchetype(baseTy); + auto archetype = + Builder.resolveArchetype(baseTy, ArchetypeResolutionKind::AlwaysPartial); assert(archetype && "Bad generic context nesting?"); return archetype @@ -52,7 +53,8 @@ Type DependentGenericTypeResolver::resolveDependentMemberType( Type DependentGenericTypeResolver::resolveSelfAssociatedType( Type selfTy, AssociatedTypeDecl *assocType) { - auto archetype = Builder.resolveArchetype(selfTy); + auto archetype = + Builder.resolveArchetype(selfTy, ArchetypeResolutionKind::AlwaysPartial); assert(archetype && "Bad generic context nesting?"); return archetype @@ -72,10 +74,12 @@ bool DependentGenericTypeResolver::areSameType(Type type1, Type type2) { if (!type1->hasTypeParameter() && !type2->hasTypeParameter()) return type1->isEqual(type2); - auto pa1 = Builder.resolveArchetype(type1); - auto pa2 = Builder.resolveArchetype(type2); + auto pa1 = + Builder.resolveArchetype(type1, ArchetypeResolutionKind::AlwaysPartial); + auto pa2 = + Builder.resolveArchetype(type2, ArchetypeResolutionKind::AlwaysPartial); if (pa1 && pa2) - return pa1->getArchetypeAnchor(Builder) == pa2->getArchetypeAnchor(Builder); + return pa1->isInSameEquivalenceClassAs(pa2); return type1->isEqual(type2); } @@ -194,7 +198,9 @@ Type CompleteGenericTypeResolver::resolveDependentMemberType( SourceRange baseRange, ComponentIdentTypeRepr *ref) { // Resolve the base to a potential archetype. - auto basePA = Builder.resolveArchetype(baseTy); + auto basePA = + Builder.resolveArchetype(baseTy, + ArchetypeResolutionKind::CompleteWellFormed); assert(basePA && "Missing potential archetype for base"); // Retrieve the potential archetype for the nested type. @@ -257,7 +263,8 @@ Type CompleteGenericTypeResolver::resolveDependentMemberType( Type CompleteGenericTypeResolver::resolveSelfAssociatedType( Type selfTy, AssociatedTypeDecl *assocType) { - return Builder.resolveArchetype(selfTy) + return Builder.resolveArchetype(selfTy, + ArchetypeResolutionKind::CompleteWellFormed) ->getNestedType(assocType, Builder) ->getDependentType(GenericParams, /*allowUnresolved=*/false); } @@ -274,10 +281,13 @@ bool CompleteGenericTypeResolver::areSameType(Type type1, Type type2) { if (!type1->hasTypeParameter() && !type2->hasTypeParameter()) return type1->isEqual(type2); - auto pa1 = Builder.resolveArchetype(type1); - auto pa2 = Builder.resolveArchetype(type2); + // FIXME: Want CompleteWellFormed here? + auto pa1 = + Builder.resolveArchetype(type1, ArchetypeResolutionKind::AlwaysPartial); + auto pa2 = + Builder.resolveArchetype(type2, ArchetypeResolutionKind::AlwaysPartial); if (pa1 && pa2) - return pa1->getArchetypeAnchor(Builder) == pa2->getArchetypeAnchor(Builder); + return pa1->isInSameEquivalenceClassAs(pa2); return type1->isEqual(type2); } From a5a162a17ca58bff9ead022107b75665cc55f6cf Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 17 Apr 2017 23:03:00 -0700 Subject: [PATCH 3/4] [GSB] Install basic recursion checking. Recursive protocol conformances still aren't functional, so reinstate some checking for recursive protocol conformances. It doesn't catch everything that the previous version did---but we don't crash on such cases anymore, either. --- lib/AST/GenericSignatureBuilder.cpp | 32 ++++++++++--------- .../decl/protocol/recursive_requirement.swift | 10 +++--- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp index f52e72196aeff..9f451ca7202e3 100644 --- a/lib/AST/GenericSignatureBuilder.cpp +++ b/lib/AST/GenericSignatureBuilder.cpp @@ -2355,21 +2355,6 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement( if (!PAT->addConformance(Proto, Source, *this)) return ConstraintResult::Resolved; -#if false - // FIXME: Ad hoc recursion breaking. - if (Visited.count(Proto)) { - markPotentialArchetypeRecursive(PAT, Proto, Source); - return ConstraintResult::Conflicting; - } - - bool inserted = Visited.insert(Proto).second; - assert(inserted); - (void) inserted; - SWIFT_DEFER { - Visited.erase(Proto); - }; -#endif - auto concreteSelf = PAT->getDependentType({}, /*allowUnresolved=*/true); auto protocolSubMap = SubstitutionMap::getProtocolSubstitutions( Proto, concreteSelf, ProtocolConformanceRef(Proto)); @@ -3091,6 +3076,23 @@ ConstraintResult GenericSignatureBuilder::addInheritedRequirements( getFloatingSource(typeRepr, /*forInferred=*/true)); } + // Check for direct recursion. + if (auto assocType = dyn_cast(decl)) { + auto proto = assocType->getProtocol(); + if (auto inheritedProto = inheritedType->getAs()) { + if (inheritedProto->getDecl() == proto || + inheritedProto->getDecl()->inheritsFrom(proto)) { + auto source = getFloatingSource(typeRepr, /*forInferred=*/false); + if (auto resolved = resolve(type, source)) { + if (auto pa = resolved->getPotentialArchetype()) { + markPotentialArchetypeRecursive(pa, proto, source.getSource(pa)); + return ConstraintResult::Conflicting; + } + } + } + } + } + return addTypeRequirement(type, inheritedType, getFloatingSource(typeRepr, /*forInferred=*/false), diff --git a/test/decl/protocol/recursive_requirement.swift b/test/decl/protocol/recursive_requirement.swift index 8c5a0b428d303..53f0da53beade 100644 --- a/test/decl/protocol/recursive_requirement.swift +++ b/test/decl/protocol/recursive_requirement.swift @@ -41,7 +41,7 @@ struct Y2 : P2 { } func f(_ z: T) { - _ = X2() + _ = X2() // expected-error{{type 'T.A' does not conform to protocol 'P2'}} } // ----- @@ -69,11 +69,11 @@ f2(Y3()) // ----- protocol Alpha { - associatedtype Beta: Gamma // expected-error{{type may not reference itself as a requirement}} + associatedtype Beta: Gamma } protocol Gamma { - associatedtype Delta: Alpha // expected-error{{type may not reference itself as a requirement}} + associatedtype Delta: Alpha } // FIXME: Redundancy diagnostics are odd here. @@ -111,7 +111,7 @@ protocol AsExistentialAssocTypeAgainB { // SR-547 protocol A { - associatedtype B1: B // expected-error{{type may not reference itself as a requirement}} + associatedtype B1: B associatedtype C1: C mutating func addObserver(_ observer: B1, forProperty: C1) @@ -122,7 +122,7 @@ protocol C { } protocol B { - associatedtype BA: A // expected-error{{type may not reference itself as a requirement}} + associatedtype BA: A associatedtype BC: C func observeChangeOfProperty(_ property: BC, observable: BA) From 54f132c487fa84daa72491d26a0fdcd3d06d3393 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 17 Apr 2017 23:11:49 -0700 Subject: [PATCH 4/4] [GSB] Delete all of the "visited" sets, which are now unused. Now that we detect recursion based on repetition within the potential archetypes, we no longer rely on passing down "visited" sets to detect/diagnose recursion. Remove them. --- include/swift/AST/GenericSignatureBuilder.h | 18 +------- lib/AST/GenericSignatureBuilder.cpp | 50 +++++---------------- 2 files changed, 11 insertions(+), 57 deletions(-) diff --git a/include/swift/AST/GenericSignatureBuilder.h b/include/swift/AST/GenericSignatureBuilder.h index 2b099b95fbe2f..c9af5de98f69f 100644 --- a/include/swift/AST/GenericSignatureBuilder.h +++ b/include/swift/AST/GenericSignatureBuilder.h @@ -267,12 +267,6 @@ class GenericSignatureBuilder { ProtocolDecl *Proto, const RequirementSource *Source); - ConstraintResult addConformanceRequirement( - PotentialArchetype *T, - ProtocolDecl *Proto, - const RequirementSource *Source, - llvm::SmallPtrSetImpl &Visited); - public: /// \brief Add a new same-type requirement between two fully resolved types /// (output of \c GenericSignatureBuilder::resolve). @@ -338,9 +332,7 @@ class GenericSignatureBuilder { UnresolvedType subject, UnresolvedType constraint, FloatingRequirementSource source, - UnresolvedHandlingKind unresolvedHandling, - llvm::SmallPtrSetImpl *visited - = nullptr); + UnresolvedHandlingKind unresolvedHandling); /// \brief Add a new conformance requirement specifying that the given /// potential archetypes are equivalent. @@ -388,7 +380,6 @@ class GenericSignatureBuilder { TypeDecl *decl, UnresolvedType type, const RequirementSource *parentSource, - llvm::SmallPtrSetImpl &visited, ModuleDecl *inferForModule); /// Visit all of the potential archetypes. @@ -480,13 +471,6 @@ class GenericSignatureBuilder { ModuleDecl *inferForModule, const SubstitutionMap *subMap = nullptr); - ConstraintResult addRequirement( - const Requirement &req, - FloatingRequirementSource source, - ModuleDecl *inferForModule, - const SubstitutionMap *subMap, - llvm::SmallPtrSetImpl &Visited); - /// \brief Add all of a generic signature's parameters and requirements. void addGenericSignature(GenericSignature *sig); diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp index 9f451ca7202e3..716745dfb7633 100644 --- a/lib/AST/GenericSignatureBuilder.cpp +++ b/lib/AST/GenericSignatureBuilder.cpp @@ -2273,9 +2273,8 @@ bool GenericSignatureBuilder::addGenericParameterRequirements( auto PA = Impl->PotentialArchetypes[Key.findIndexIn(Impl->GenericParams)]; // Add the requirements from the declaration. - llvm::SmallPtrSet visited; return isErrorResult( - addInheritedRequirements(GenericParam, PA, nullptr, visited, + addInheritedRequirements(GenericParam, PA, nullptr, GenericParam->getModuleContext())); } @@ -2293,14 +2292,6 @@ void GenericSignatureBuilder::addGenericParameter(GenericTypeParamType *GenericP Impl->PotentialArchetypes.push_back(PA); } -ConstraintResult GenericSignatureBuilder::addConformanceRequirement( - PotentialArchetype *PAT, - ProtocolDecl *Proto, - const RequirementSource *Source) { - llvm::SmallPtrSet Visited; - return addConformanceRequirement(PAT, Proto, Source, Visited); -} - /// Visit all of the types that show up in the list of inherited /// types. static ConstraintResult visitInherited( @@ -2349,8 +2340,7 @@ static ConstraintResult visitInherited( ConstraintResult GenericSignatureBuilder::addConformanceRequirement( PotentialArchetype *PAT, ProtocolDecl *Proto, - const RequirementSource *Source, - llvm::SmallPtrSetImpl &Visited) { + const RequirementSource *Source) { // Add the requirement, if we haven't done so already. if (!PAT->addConformance(Proto, Source, *this)) return ConstraintResult::Resolved; @@ -2370,7 +2360,7 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement( /*inferred=*/false); for (auto req : reqSig->getRequirements()) { auto reqResult = addRequirement(req, innerSource, nullptr, - &protocolSubMap, Visited); + &protocolSubMap); if (isErrorResult(reqResult)) return reqResult; } @@ -2384,7 +2374,7 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement( auto protoModule = Proto->getParentModule(); auto inheritedReqResult = - addInheritedRequirements(Proto, PAT, Source, Visited, protoModule); + addInheritedRequirements(Proto, PAT, Source, protoModule); if (isErrorResult(inheritedReqResult)) return inheritedReqResult; @@ -2403,8 +2393,7 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement( // Add requirements placed directly on this associated type. Type assocType = DependentMemberType::get(concreteSelf, AssocType); auto assocResult = - addInheritedRequirements(AssocType, assocType, Source, Visited, - protoModule); + addInheritedRequirements(AssocType, assocType, Source, protoModule); if (isErrorResult(assocResult)) return assocResult; @@ -2581,13 +2570,7 @@ ConstraintResult GenericSignatureBuilder::addTypeRequirement( UnresolvedType subject, UnresolvedType constraint, FloatingRequirementSource source, - UnresolvedHandlingKind unresolvedHandling, - llvm::SmallPtrSetImpl *visited) { - // Make sure we always have a "visited" set to pass down. - SmallPtrSet visitedSet; - if (!visited) - visited = &visitedSet; - + UnresolvedHandlingKind unresolvedHandling) { // Resolve the constraint. auto resolvedConstraint = resolve(constraint, source); if (!resolvedConstraint) { @@ -2685,7 +2668,7 @@ ConstraintResult GenericSignatureBuilder::addTypeRequirement( for (auto *proto : layout.getProtocols()) { auto *protoDecl = proto->getDecl(); if (isErrorResult(addConformanceRequirement(subjectPA, protoDecl, - resolvedSource, *visited))) + resolvedSource))) anyErrors = true; } @@ -3031,7 +3014,6 @@ ConstraintResult GenericSignatureBuilder::addInheritedRequirements( TypeDecl *decl, UnresolvedType type, const RequirementSource *parentSource, - llvm::SmallPtrSetImpl &visited, ModuleDecl *inferForModule) { if (isa(decl) && decl->hasInterfaceType() && @@ -3096,8 +3078,7 @@ ConstraintResult GenericSignatureBuilder::addInheritedRequirements( return addTypeRequirement(type, inheritedType, getFloatingSource(typeRepr, /*forInferred=*/false), - UnresolvedHandlingKind::GenerateConstraints, - &visited); + UnresolvedHandlingKind::GenerateConstraints); }; auto visitLayout = [&](LayoutConstraint layout, const TypeRepr *typeRepr) { @@ -3206,21 +3187,11 @@ ConstraintResult GenericSignatureBuilder::addRequirement( llvm_unreachable("Unhandled requirement?"); } -ConstraintResult GenericSignatureBuilder::addRequirement( - const Requirement &req, - FloatingRequirementSource source, - ModuleDecl *inferForModule, - const SubstitutionMap *subMap) { - llvm::SmallPtrSet visited; - return addRequirement(req, source, inferForModule, subMap, visited); -} - ConstraintResult GenericSignatureBuilder::addRequirement( const Requirement &req, FloatingRequirementSource source, ModuleDecl *inferForModule, - const SubstitutionMap *subMap, - llvm::SmallPtrSetImpl &Visited) { + const SubstitutionMap *subMap) { auto subst = [&](Type t) { if (subMap) return t.subst(*subMap); @@ -3245,8 +3216,7 @@ ConstraintResult GenericSignatureBuilder::addRequirement( } return addTypeRequirement(firstType, secondType, source, - UnresolvedHandlingKind::GenerateConstraints, - &Visited); + UnresolvedHandlingKind::GenerateConstraints); } case RequirementKind::Layout: {