From 04ba576e3f1b807a58d3fda8d3f6e4c6ab55894f Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 24 Feb 2017 14:25:03 -0800 Subject: [PATCH 1/4] [GenericSig Builder] Introduce an equivalence-class abstraction. Introduce an equivalence-class abstraction that captures all of the members of the equivalence class in a separate type that will maintain the "truth" about the meaning of the equivalence class, rather than having that information distributed amongst the potential archetypes within the class. For now, use it to capture the members of the equivalence classes, so we have one SmallVector per equivalence class rather than N SmallVectors. --- include/swift/AST/GenericSignatureBuilder.h | 64 +++++++++----- lib/AST/GenericSignatureBuilder.cpp | 98 +++++++++++++++------ 2 files changed, 111 insertions(+), 51 deletions(-) diff --git a/include/swift/AST/GenericSignatureBuilder.h b/include/swift/AST/GenericSignatureBuilder.h index 70010bfdd48fe..e8d332bb6f121 100644 --- a/include/swift/AST/GenericSignatureBuilder.h +++ b/include/swift/AST/GenericSignatureBuilder.h @@ -377,6 +377,16 @@ class GenericSignatureBuilder { using RequirementRHS = llvm::PointerUnion3; + /// Describes an equivalence class of potential archetypes. + struct EquivalenceClass { + /// The members of the equivalence class. + TinyPtrVector members; + + /// Construct a new equivalence class containing only the given + /// potential archetype (which represents itself). + EquivalenceClass(PotentialArchetype *representative); + }; + friend class RequirementSource; private: @@ -690,8 +700,10 @@ class GenericSignatureBuilder::PotentialArchetype { } identifier; /// \brief The representative of the equivalence class of potential archetypes - /// to which this potential archetype belongs. - mutable PotentialArchetype *Representative; + /// to which this potential archetype belongs, or (for the representative) + /// the equivalence class itself. + mutable llvm::PointerUnion + representativeOrEquivClass; /// Same-type constraints between this potential archetype and any other /// archetype in its equivalence class. @@ -754,63 +766,54 @@ class GenericSignatureBuilder::PotentialArchetype { /// the old name. Identifier OrigName; - /// The equivalence class of this potential archetype. - llvm::TinyPtrVector EquivalenceClass; - /// \brief Construct a new potential archetype for an unresolved /// associated type. PotentialArchetype(PotentialArchetype *parent, Identifier name) - : parentOrBuilder(parent), identifier(name), Representative(this), - isUnresolvedNestedType(true), + : parentOrBuilder(parent), identifier(name), isUnresolvedNestedType(true), IsRecursive(false), Invalid(false), RecursiveConcreteType(false), RecursiveSuperclassType(false), DiagnosedRename(false) { assert(parent != nullptr && "Not an associated type?"); - EquivalenceClass.push_back(this); } /// \brief Construct a new potential archetype for an associated type. PotentialArchetype(PotentialArchetype *parent, AssociatedTypeDecl *assocType) : parentOrBuilder(parent), identifier(assocType), - Representative(this), isUnresolvedNestedType(false), - IsRecursive(false), Invalid - (false), + isUnresolvedNestedType(false), IsRecursive(false), Invalid(false), RecursiveConcreteType(false), RecursiveSuperclassType(false), DiagnosedRename(false) { assert(parent != nullptr && "Not an associated type?"); - EquivalenceClass.push_back(this); } /// \brief Construct a new potential archetype for a type alias. PotentialArchetype(PotentialArchetype *parent, TypeAliasDecl *typeAlias) : parentOrBuilder(parent), identifier(typeAlias), - Representative(this), isUnresolvedNestedType(false), + isUnresolvedNestedType(false), IsRecursive(false), Invalid(false), RecursiveConcreteType(false), RecursiveSuperclassType(false), DiagnosedRename(false) { assert(parent != nullptr && "Not an associated type?"); - EquivalenceClass.push_back(this); } /// \brief Construct a new potential archetype for a generic parameter. PotentialArchetype(GenericSignatureBuilder *builder, GenericParamKey genericParam) : parentOrBuilder(builder), identifier(genericParam), - Representative(this), isUnresolvedNestedType(false), + isUnresolvedNestedType(false), IsRecursive(false), Invalid(false), RecursiveConcreteType(false), RecursiveSuperclassType(false), DiagnosedRename(false) { - EquivalenceClass.push_back(this); } /// \brief Retrieve the representative for this archetype, performing /// path compression on the way. PotentialArchetype *getRepresentative() const; - /// Retrieve the generic signature builder with which this archetype is associated. + /// Retrieve the generic signature builder with which this archetype is + /// associated. GenericSignatureBuilder *getBuilder() const { const PotentialArchetype *pa = this; while (auto parent = pa->getParent()) @@ -927,9 +930,24 @@ class GenericSignatureBuilder::PotentialArchetype { /// the number of associated type references. unsigned getNestingDepth() const; + /// Retrieve the equivalence class, if it's already present. + /// + /// Otherwise, return null. + EquivalenceClass *getEquivalenceClassIfPresent() const { + return getRepresentative()->representativeOrEquivClass + .dyn_cast(); + } + + /// Retrieve or create the equivalence class. + EquivalenceClass *getOrCreateEquivalenceClass() const; + /// Retrieve the equivalence class containing this potential archetype. - ArrayRef getEquivalenceClass() { - return getRepresentative()->EquivalenceClass; + TinyPtrVector getEquivalenceClassMembers() const { + if (auto equivClass = getEquivalenceClassIfPresent()) + return equivClass->members; + + return TinyPtrVector( + const_cast(this)); } /// \brief Retrieve the potential archetype to be used as the anchor for @@ -988,16 +1006,16 @@ class GenericSignatureBuilder::PotentialArchetype { /// True if the potential archetype has been bound by a concrete type /// constraint. bool isConcreteType() const { - if (Representative != this) - return Representative->isConcreteType(); + if (getRepresentative() != this) + return getRepresentative()->isConcreteType(); return static_cast(ConcreteType); } /// Get the concrete type this potential archetype is constrained to. Type getConcreteType() const { - if (Representative != this) - return Representative->getConcreteType(); + if (getRepresentative() != this) + return getRepresentative()->getConcreteType(); return ConcreteType; } diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp index a048d81ec23a8..9a98765dcc175 100644 --- a/lib/AST/GenericSignatureBuilder.cpp +++ b/lib/AST/GenericSignatureBuilder.cpp @@ -40,6 +40,8 @@ using llvm::DenseMap; namespace { typedef GenericSignatureBuilder::PotentialArchetype PotentialArchetype; + typedef GenericSignatureBuilder::EquivalenceClass EquivalenceClass; + } // end anonymous namespace struct GenericSignatureBuilder::Implementation { @@ -440,6 +442,8 @@ GenericSignatureBuilder::PotentialArchetype::~PotentialArchetype() { delete pa; } } + + delete representativeOrEquivClass.dyn_cast(); } std::string GenericSignatureBuilder::PotentialArchetype::getDebugName() const { @@ -508,7 +512,7 @@ PotentialArchetype::findAnyConcreteTypeSourceAsWritten() const { if (!rep->isConcreteType()) return nullptr; // Otherwise, go look for the source. - for (auto pa : rep->getEquivalenceClass()) { + for (auto pa : rep->getEquivalenceClassMembers()) { if (pa->ConcreteTypeSource && pa->ConcreteTypeSource->getLoc().isValid()) return pa->ConcreteTypeSource; @@ -585,7 +589,8 @@ struct GenericSignatureBuilder::ResolvedType { static ResolvedType forNewTypeAlias(PotentialArchetype *pa) { assert(pa->getParent() && pa->getTypeAliasDecl() && - pa->ConcreteType.isNull() && pa->getEquivalenceClass().size() == 1 && + pa->ConcreteType.isNull() && + pa->getEquivalenceClassMembers().size() == 1 && "not a new typealias"); return ResolvedType(pa); } @@ -702,22 +707,44 @@ bool GenericSignatureBuilder::PotentialArchetype::addConformance( return true; } -auto GenericSignatureBuilder::PotentialArchetype::getRepresentative() const - -> PotentialArchetype *{ +auto PotentialArchetype::getOrCreateEquivalenceClass() const -> EquivalenceClass * { + // The equivalence class is stored on the representative. + auto representative = getRepresentative(); + if (representative != this) + return representative->getOrCreateEquivalenceClass(); + + // If we already have an equivalence class, return it. + if (auto equivClass = getEquivalenceClassIfPresent()) + return equivClass; + + // Create a new equivalence class. + auto equivClass = + new EquivalenceClass(const_cast(this)); + representativeOrEquivClass = equivClass; + return equivClass; +} + +auto PotentialArchetype::getRepresentative() const -> PotentialArchetype * { + auto representative = + representativeOrEquivClass.dyn_cast(); + if (!representative) + return const_cast(this); + // Find the representative. - PotentialArchetype *Result = Representative; - while (Result != Result->Representative) - Result = Result->Representative; + PotentialArchetype *result = representative; + while (auto nextRepresentative = + result->representativeOrEquivClass.dyn_cast()) + result = nextRepresentative; // Perform (full) path compression. - const PotentialArchetype *FixUp = this; - while (FixUp != FixUp->Representative) { - const PotentialArchetype *Next = FixUp->Representative; - FixUp->Representative = Result; - FixUp = Next; + const PotentialArchetype *fixUp = this; + while (auto nextRepresentative = + fixUp->representativeOrEquivClass.dyn_cast()) { + fixUp->representativeOrEquivClass = nextRepresentative; + fixUp = nextRepresentative; } - return Result; + return result; } /// Canonical ordering for dependent types in generic signatures. @@ -848,14 +875,14 @@ auto GenericSignatureBuilder::PotentialArchetype::getArchetypeAnchor( // Find the best archetype within this equivalence class. PotentialArchetype *rep = getRepresentative(); auto anchor = rep; - for (auto pa : rep->getEquivalenceClass()) { + for (auto pa : rep->getEquivalenceClassMembers()) { if (compareDependentTypes(&pa, &anchor) < 0) anchor = pa; } #ifndef NDEBUG // Make sure that we did, in fact, get one that is better than all others. - for (auto pa : anchor->getEquivalenceClass()) { + for (auto pa : anchor->getEquivalenceClassMembers()) { assert((pa == anchor || compareDependentTypes(&anchor, &pa) < 0) && compareDependentTypes(&pa, &anchor) >= 0 && "archetype anchor isn't a total order"); @@ -1038,7 +1065,7 @@ auto GenericSignatureBuilder::PotentialArchetype::getNestedType( // We know something concrete about the parent PA, so we need to propagate // that information to this new archetype. if (isConcreteType()) { - for (auto equivT : rep->EquivalenceClass) { + for (auto equivT : rep->getEquivalenceClassMembers()) { concretizeNestedTypeFromConcreteParent( equivT, sameNestedTypeSource, nestedPA, builder, [&](ProtocolDecl *proto) -> ProtocolConformanceRef { @@ -1346,14 +1373,14 @@ void GenericSignatureBuilder::PotentialArchetype::dump(llvm::raw_ostream &Out, } } - if (Representative != this) { + if (getRepresentative() != this) { Out << " [represented by " << getRepresentative()->getDebugName() << "]"; } - if (EquivalenceClass.size() > 1) { + if (getEquivalenceClassMembers().size() > 1) { Out << " [equivalence class "; bool isFirst = true; - for (auto equiv : EquivalenceClass) { + for (auto equiv : getEquivalenceClassMembers()) { if (equiv == this) continue; if (isFirst) isFirst = false; @@ -1374,6 +1401,11 @@ void GenericSignatureBuilder::PotentialArchetype::dump(llvm::raw_ostream &Out, } } +#pragma mark Equivalence classes +EquivalenceClass::EquivalenceClass(PotentialArchetype *representative) { + members.push_back(representative); +} + GenericSignatureBuilder::GenericSignatureBuilder( ASTContext &ctx, std::function lookupConformance) @@ -1443,7 +1475,7 @@ auto GenericSignatureBuilder::resolve(UnresolvedType paOrT, // We're assuming that an equivalence class with a type alias representative // doesn't have a "true" (i.e. associated type) potential archetype. - assert(llvm::all_of(rep->getEquivalenceClass(), + assert(llvm::all_of(rep->getEquivalenceClassMembers(), [&](PotentialArchetype *pa) { return pa->getParent() && pa->getTypeAliasDecl(); }) && @@ -1826,10 +1858,20 @@ bool GenericSignatureBuilder::addSameTypeRequirementBetweenArchetypes( if (T1->getParent() == nullptr) updateExistingSource = false; + // Merge the equivalence classes. + auto equivClass = T1->getOrCreateEquivalenceClass(); + auto equivClass2Members = T2->getEquivalenceClassMembers(); + for (auto equiv : equivClass2Members) + equivClass->members.push_back(equiv); + + // Grab the old equivalence class, if present. We'll delete it at the end. + auto equivClass2 = T2->getEquivalenceClassIfPresent(); + SWIFT_DEFER { + delete equivClass2; + }; + // Make T1 the representative of T2, merging the equivalence classes. - T2->Representative = T1; - for (auto equiv : T2->EquivalenceClass) - T1->EquivalenceClass.push_back(equiv); + T2->representativeOrEquivClass = T1; // Superclass requirements. if (T2->Superclass) { @@ -1845,7 +1887,7 @@ bool GenericSignatureBuilder::addSameTypeRequirementBetweenArchetypes( // Recursively merge the associated types of T2 into T1. auto sameNestedTypeSource = RequirementSource::forNestedTypeNameMatch(*this); - for (auto equivT2 : T2->EquivalenceClass) { + for (auto equivT2 : equivClass2Members) { for (auto T2Nested : equivT2->NestedTypes) { auto T1Nested = T1->getNestedType(T2Nested.first, *this); if (addSameTypeRequirement(T1Nested, T2Nested.second.front(), @@ -1971,7 +2013,7 @@ bool GenericSignatureBuilder::addSameTypeRequirementToConcrete( // Eagerly resolve any existing nested types to their concrete forms (others // will be "concretized" as they are constructed, in getNestedType). - for (auto equivT : rep->EquivalenceClass) { + for (auto equivT : rep->getEquivalenceClassMembers()) { for (auto nested : equivT->getNestedTypes()) { concretizeNestedTypeFromConcreteParent( equivT, Source, nested.second.front(), *this, @@ -2576,7 +2618,7 @@ GenericSignatureBuilder::finalize(SourceLoc loc, // Don't allow two generic parameters to be equivalent, because then we // don't actually have two parameters. - for (auto other : rep->getEquivalenceClass()) { + for (auto other : rep->getEquivalenceClassMembers()) { // If it isn't a generic parameter, skip it. if (other == pa || other->getParent() != nullptr) continue; @@ -2694,7 +2736,7 @@ void GenericSignatureBuilder::checkRedundantConcreteTypeConstraints( // Gather the concrete constraints within this equivalence class. SmallVector concreteConstraints; - for (auto pa : representative->getEquivalenceClass()) { + for (auto pa : representative->getEquivalenceClassMembers()) { auto source = pa->getConcreteTypeSourceAsWritten(); if (!source) continue; @@ -2901,7 +2943,7 @@ static SmallVector getSameTypeComponents( PotentialArchetype *rep) { SmallPtrSet visited; SmallVector components; - for (auto pa : rep->getEquivalenceClass()) { + for (auto pa : rep->getEquivalenceClassMembers()) { // If we've already seen this potential archetype, there's nothing else to // do. if (visited.count(pa) != 0) continue; From ba81b66f2654a1c79395c67418cbeb785710edfd Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 24 Feb 2017 14:47:59 -0800 Subject: [PATCH 2/4] [GenericSigBuilder] Migrate the concrete type into to EquivalenceClass. --- include/swift/AST/GenericSignatureBuilder.h | 16 ++++++++++------ lib/AST/GenericSignatureBuilder.cpp | 18 +++++++++--------- test/Constraints/same_types.swift | 2 +- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/include/swift/AST/GenericSignatureBuilder.h b/include/swift/AST/GenericSignatureBuilder.h index e8d332bb6f121..1b47f3361399e 100644 --- a/include/swift/AST/GenericSignatureBuilder.h +++ b/include/swift/AST/GenericSignatureBuilder.h @@ -379,6 +379,9 @@ class GenericSignatureBuilder { /// Describes an equivalence class of potential archetypes. struct EquivalenceClass { + /// Concrete type to which this equivalence class is equal. + Type concreteType; + /// The members of the equivalence class. TinyPtrVector members; @@ -1006,17 +1009,18 @@ class GenericSignatureBuilder::PotentialArchetype { /// True if the potential archetype has been bound by a concrete type /// constraint. bool isConcreteType() const { - if (getRepresentative() != this) - return getRepresentative()->isConcreteType(); + if (auto equivClass = getEquivalenceClassIfPresent()) + return static_cast(equivClass->concreteType); - return static_cast(ConcreteType); + return false; } /// Get the concrete type this potential archetype is constrained to. Type getConcreteType() const { - if (getRepresentative() != this) - return getRepresentative()->getConcreteType(); - return ConcreteType; + if (auto equivClass = getEquivalenceClassIfPresent()) + return equivClass->concreteType; + + return Type(); } void setIsRecursive() { IsRecursive = true; } diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp index 9a98765dcc175..8cb2d3b6c84f9 100644 --- a/lib/AST/GenericSignatureBuilder.cpp +++ b/lib/AST/GenericSignatureBuilder.cpp @@ -1822,7 +1822,8 @@ bool GenericSignatureBuilder::addSameTypeRequirementBetweenArchetypes( // Merge any concrete constraints. Type concrete1 = T1->getConcreteType(); Type concrete2 = T2->getConcreteType(); - + + // FIXME: This seems early. if (concrete1 && concrete2) { bool mismatch = addSameTypeRequirement( concrete1, concrete2, Source, [&](Type type1, Type type2) { @@ -1834,10 +1835,6 @@ bool GenericSignatureBuilder::addSameTypeRequirementBetweenArchetypes( }); if (mismatch) return true; - } else if (concrete2) { - assert(!T1->ConcreteType - && "already formed archetype for concrete-constrained parameter"); - T1->ConcreteType = concrete2; } // Don't mark requirements as redundant if they come from one of our @@ -1873,6 +1870,10 @@ bool GenericSignatureBuilder::addSameTypeRequirementBetweenArchetypes( // Make T1 the representative of T2, merging the equivalence classes. T2->representativeOrEquivClass = T1; + // Same-type-to-concrete requirement. + if (equivClass2 && equivClass2->concreteType && !equivClass->concreteType) + equivClass->concreteType = equivClass2->concreteType; + // Superclass requirements. if (T2->Superclass) { addSuperclassRequirement(T1, T2->getSuperclass(), @@ -1930,9 +1931,6 @@ bool GenericSignatureBuilder::addSameTypeRequirementToConcrete( // If this is a better source, record it. updateRequirementSource(T->ConcreteTypeSource, Source); - if (!rep->ConcreteType) - rep->ConcreteType = Concrete; - return false; } @@ -1991,7 +1989,9 @@ bool GenericSignatureBuilder::addSameTypeRequirementToConcrete( } // Record the requirement. - rep->ConcreteType = Concrete; + auto equivClass = rep->getOrCreateEquivalenceClass(); + if (!equivClass->concreteType) + equivClass->concreteType = Concrete; // Make sure the concrete type fulfills the superclass requirement // of the archetype. diff --git a/test/Constraints/same_types.swift b/test/Constraints/same_types.swift index 593f251f847f7..5588856f7a264 100644 --- a/test/Constraints/same_types.swift +++ b/test/Constraints/same_types.swift @@ -75,7 +75,7 @@ func test4(_ t: T) -> Y where T.Bar == Y { func fail3(_ t: T) -> X where T.Bar == X { // expected-error {{'X' does not conform to required protocol 'Fooable'}} - return t.bar + return t.bar // expected-error{{cannot convert return expression of type 'T.Bar' to return type 'X'}} } func test5(_ t: T) -> X where T.Bar.Foo == X { From 6492cc301c28796af4d550739d116b63caffaba4 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 24 Feb 2017 14:54:02 -0800 Subject: [PATCH 3/4] [GenericSigBuilder] Eliminate some redundancy in same-type-to-concrete checking. --- lib/AST/GenericSignatureBuilder.cpp | 51 +++++++++-------------------- test/Constraints/same_types.swift | 2 +- 2 files changed, 17 insertions(+), 36 deletions(-) diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp index 8cb2d3b6c84f9..5d3ac48a03e1f 100644 --- a/lib/AST/GenericSignatureBuilder.cpp +++ b/lib/AST/GenericSignatureBuilder.cpp @@ -1911,50 +1911,36 @@ bool GenericSignatureBuilder::addSameTypeRequirementToConcrete( PotentialArchetype *T, Type Concrete, const RequirementSource *Source) { - auto rep = T->getRepresentative(); + // Record the concrete type and its source. + if (!T->ConcreteType) { + // FIXME: Always record this. + T->ConcreteType = Concrete; + T->ConcreteTypeSource = Source; + } - // If there is an existing source on this potential archetype, make sure - // we have the same type. - // FIXME: Delay until finalize(). - if (auto existingSource = T->ConcreteTypeSource) { + // If we've already been bound to a type, match that type. + if (auto oldConcrete = T->getConcreteType()) { bool mismatch = addSameTypeRequirement( - T->ConcreteType, Concrete, Source, [&](Type type1, Type type2) { + oldConcrete, Concrete, Source, [&](Type type1, Type type2) { Diags.diagnose(Source->getLoc(), diag::requires_same_type_conflict, T->isGenericParam(), T->getDependentType(/*FIXME: */{ }, true), type1, type2); + }); if (mismatch) return true; - // If this is a better source, record it. - updateRequirementSource(T->ConcreteTypeSource, Source); - + // Nothing more to do; the types matched. return false; } - // If we've already been bound to a type, match that type. - if (T != rep) { - if (auto oldConcrete = rep->getConcreteType()) { - bool mismatch = addSameTypeRequirement( - oldConcrete, Concrete, Source, [&](Type type1, Type type2) { - Diags.diagnose(Source->getLoc(), - diag::requires_same_type_conflict, - T->isGenericParam(), - T->getDependentType(/*FIXME: */{ }, true), type1, - type2); - - }); - - if (mismatch) return true; - return false; - } - } - - // Record the concrete type and its source. - T->ConcreteType = Concrete; - T->ConcreteTypeSource = Source; + // Record the requirement. + auto rep = T->getRepresentative(); + auto equivClass = rep->getOrCreateEquivalenceClass(); + if (!equivClass->concreteType) + equivClass->concreteType = Concrete; // Make sure the concrete type fulfills the requirements on the archetype. // FIXME: Move later... @@ -1988,11 +1974,6 @@ bool GenericSignatureBuilder::addSameTypeRequirementToConcrete( updateRequirementSource(conforms.second, concreteSource); } - // Record the requirement. - auto equivClass = rep->getOrCreateEquivalenceClass(); - if (!equivClass->concreteType) - equivClass->concreteType = Concrete; - // Make sure the concrete type fulfills the superclass requirement // of the archetype. if (rep->Superclass) { diff --git a/test/Constraints/same_types.swift b/test/Constraints/same_types.swift index 5588856f7a264..593f251f847f7 100644 --- a/test/Constraints/same_types.swift +++ b/test/Constraints/same_types.swift @@ -75,7 +75,7 @@ func test4(_ t: T) -> Y where T.Bar == Y { func fail3(_ t: T) -> X where T.Bar == X { // expected-error {{'X' does not conform to required protocol 'Fooable'}} - return t.bar // expected-error{{cannot convert return expression of type 'T.Bar' to return type 'X'}} + return t.bar } func test5(_ t: T) -> X where T.Bar.Foo == X { From 29256e526c547adc86136e898347aa6b6948579d Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 24 Feb 2017 15:43:17 -0800 Subject: [PATCH 4/4] [GenericSigBuilder] Track all same-type-to-concrete constraints. When we see a second same-type-to-concrete constraint on a particular potential archetype, record it. Previously, we were checking it and then updating the requirement source eagerly. That won't work with proper recursion detection, and meant that we missed out on some obvious redundant-same-type-constraint diagnostics. The scheme here is to build up the equivalence classes without losing any information, and then determine which information is redundant at the end. --- include/swift/AST/GenericSignatureBuilder.h | 28 ++-- lib/AST/GenericSignatureBuilder.cpp | 164 +++++++++++--------- test/Constraints/same_types.swift | 4 +- 3 files changed, 113 insertions(+), 83 deletions(-) diff --git a/include/swift/AST/GenericSignatureBuilder.h b/include/swift/AST/GenericSignatureBuilder.h index 1b47f3361399e..8210878b0db02 100644 --- a/include/swift/AST/GenericSignatureBuilder.h +++ b/include/swift/AST/GenericSignatureBuilder.h @@ -736,13 +736,15 @@ class GenericSignatureBuilder::PotentialArchetype { llvm::MapVector> NestedTypes; - /// The concrete type to which a this potential archetype has been + /// The concrete types to which this potential archetype has been /// constrained. - Type ConcreteType; + /// + /// This vector runs parallel to ConcreteTypeSources. + llvm::TinyPtrVector concreteTypes; - /// The source of the concrete type requirement, if one was written - /// on this potential archetype. - const RequirementSource *ConcreteTypeSource = nullptr; + /// The source of the concrete type requirements that were written on + /// this potential archetype. + llvm::TinyPtrVector concreteTypeSources; /// Whether this is an unresolved nested type. unsigned isUnresolvedNestedType : 1; @@ -971,15 +973,21 @@ class GenericSignatureBuilder::PotentialArchetype { SameTypeConstraints.end()); } - /// Retrieve the concrete type source as written on this potential archetype. - const RequirementSource *getConcreteTypeSourceAsWritten() const { - return ConcreteTypeSource; + /// Retrieve the concrete types as written on this potential archetype. + const llvm::TinyPtrVector& getConcreteTypesAsWritten() const { + return concreteTypes; + } + + /// Retrieve the concrete type sources as written on this potential archetype. + ArrayRef getConcreteTypeSourcesAsWritten() const { + return concreteTypeSources; } /// Find a source of the same-type constraint that maps this potential /// archetype to a concrete type somewhere in the equivalence class of this - /// type. - const RequirementSource *findAnyConcreteTypeSourceAsWritten() const; + /// type along with the concrete type that was written there. + Optional> + findAnyConcreteTypeSourceAsWritten() const; /// \brief Retrieve (or create) a nested type with the given name. PotentialArchetype *getNestedType(Identifier Name, diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp index 5d3ac48a03e1f..ba3906ecf521f 100644 --- a/lib/AST/GenericSignatureBuilder.cpp +++ b/lib/AST/GenericSignatureBuilder.cpp @@ -501,24 +501,38 @@ void GenericSignatureBuilder::PotentialArchetype::resolveAssociatedType( --builder.Impl->NumUnresolvedNestedTypes; } -const RequirementSource * +Optional> PotentialArchetype::findAnyConcreteTypeSourceAsWritten() const { - // If we have a concrete type source, use that. - if (ConcreteTypeSource && ConcreteTypeSource->getLoc().isValid()) - return ConcreteTypeSource; + using Result = std::pair; + + // Local function to look for a source in the given potential archetype. + auto lookInPA = [](const PotentialArchetype *pa) -> Optional { + for (unsigned i : indices(pa->concreteTypeSources)) { + auto source = pa->concreteTypeSources[i]; + if (source->getLoc().isValid()) + return Result(pa->concreteTypes[i], source); + } + + return None; + }; + + // If we have a concrete type source with location information, use that. + if (auto result = lookInPA(this)) + return result; // If we don't have a concrete type, there's no source. auto rep = getRepresentative(); - if (!rep->isConcreteType()) return nullptr; + if (!rep->isConcreteType()) return None; // Otherwise, go look for the source. for (auto pa : rep->getEquivalenceClassMembers()) { - if (pa->ConcreteTypeSource && - pa->ConcreteTypeSource->getLoc().isValid()) - return pa->ConcreteTypeSource; + if (pa != this) { + if (auto result = lookInPA(pa)) + return result; + } } - return nullptr; + return None; } bool GenericSignatureBuilder::updateRequirementSource( @@ -589,7 +603,7 @@ struct GenericSignatureBuilder::ResolvedType { static ResolvedType forNewTypeAlias(PotentialArchetype *pa) { assert(pa->getParent() && pa->getTypeAliasDecl() && - pa->ConcreteType.isNull() && + pa->concreteTypes.empty() && pa->getEquivalenceClassMembers().size() == 1 && "not a new typealias"); return ResolvedType(pa); @@ -1340,17 +1354,18 @@ void GenericSignatureBuilder::PotentialArchetype::dump(llvm::raw_ostream &Out, } // Print concrete type. - if (ConcreteType) { + for (unsigned i : indices(concreteTypes)) { + auto concreteType = concreteTypes[i]; Out << " == "; - ConcreteType.print(Out); - if (ConcreteTypeSource) { - Out << " "; - if (!ConcreteTypeSource->isDerivedRequirement()) - Out << "*"; - Out << "["; - ConcreteTypeSource->print(Out, SrcMgr); - Out << "]"; - } + concreteType.print(Out); + + auto concreteTypeSource = concreteTypeSources[i]; + Out << " "; + if (!concreteTypeSource->isDerivedRequirement()) + Out << "*"; + Out << "["; + concreteTypeSource->print(Out, SrcMgr); + Out << "]"; } // Print requirements. @@ -1673,10 +1688,10 @@ bool GenericSignatureBuilder::addSuperclassRequirement(PotentialArchetype *T, Type concrete = T->getConcreteType(); if (!Superclass->isExactSuperclassOf(concrete, getLazyResolver())) { if (auto source = T->findAnyConcreteTypeSourceAsWritten()) { - Diags.diagnose(source->getLoc(), diag::type_does_not_inherit, + Diags.diagnose(source->second->getLoc(), diag::type_does_not_inherit, T->getDependentType(/*FIXME:*/{ }, /*allowUnresolved=*/true), - concrete, Superclass) + source->first, Superclass) .highlight(Source->getLoc()); } return true; @@ -1912,11 +1927,8 @@ bool GenericSignatureBuilder::addSameTypeRequirementToConcrete( Type Concrete, const RequirementSource *Source) { // Record the concrete type and its source. - if (!T->ConcreteType) { - // FIXME: Always record this. - T->ConcreteType = Concrete; - T->ConcreteTypeSource = Source; - } + T->concreteTypes.push_back(Concrete); + T->concreteTypeSources.push_back(Source); // If we've already been bound to a type, match that type. if (auto oldConcrete = T->getConcreteType()) { @@ -2540,11 +2552,11 @@ GenericSignatureBuilder::finalize(SourceLoc loc, // Check for recursive same-type bindings. if (isRecursiveConcreteType(archetype, /*isSuperclass=*/false)) { if (auto source = archetype->findAnyConcreteTypeSourceAsWritten()) { - Diags.diagnose(source->getLoc(), + Diags.diagnose(source->second->getLoc(), diag::recursive_same_type_constraint, archetype->getDependentType(genericParams, /*allowUnresolved=*/true), - archetype->getConcreteType()); + source->first); } archetype->RecursiveConcreteType = true; @@ -2590,7 +2602,7 @@ GenericSignatureBuilder::finalize(SourceLoc loc, // because then we don't actually have a parameter. if (rep->getConcreteType()) { if (auto source = rep->findAnyConcreteTypeSourceAsWritten()) - Diags.diagnose(source->getLoc(), + Diags.diagnose(source->second->getLoc(), diag::requires_generic_param_made_equal_to_concrete, rep->getDependentType(genericParams, /*allowUnresolved=*/true)); @@ -2718,46 +2730,49 @@ void GenericSignatureBuilder::checkRedundantConcreteTypeConstraints( // Gather the concrete constraints within this equivalence class. SmallVector concreteConstraints; for (auto pa : representative->getEquivalenceClassMembers()) { - auto source = pa->getConcreteTypeSourceAsWritten(); - if (!source) continue; - - // Save this constraint. - auto constraint = ConcreteConstraint{pa, pa->ConcreteType, source}; - concreteConstraints.push_back(constraint); - - // Check whether this constraint is better than the best we've seen so far - // at being the representative constraint against which others will be - // compared. - if (!representativeConstraint) { - representativeConstraint = constraint; - continue; - } - - // We prefer derived constraints to non-derived constraints. - bool thisIsDerived = source->isDerivedRequirement(); - bool representativeIsDerived = - representativeConstraint->source->isDerivedRequirement(); - if (thisIsDerived != representativeIsDerived) { - if (thisIsDerived) + auto types = pa->getConcreteTypesAsWritten(); + auto sources = pa->getConcreteTypeSourcesAsWritten(); + for (unsigned i : indices(types)) { + auto source = sources[i]; + + // Save this constraint. + auto constraint = ConcreteConstraint{pa, types[i], source}; + concreteConstraints.push_back(constraint); + + // Check whether this constraint is better than the best we've seen so far + // at being the representative constraint against which others will be + // compared. + if (!representativeConstraint) { representativeConstraint = constraint; + continue; + } - continue; - } + // We prefer derived constraints to non-derived constraints. + bool thisIsDerived = source->isDerivedRequirement(); + bool representativeIsDerived = + representativeConstraint->source->isDerivedRequirement(); + if (thisIsDerived != representativeIsDerived) { + if (thisIsDerived) + representativeConstraint = constraint; - // We prefer constraints with locations to constraints without locations. - bool thisHasValidSourceLoc = source->getLoc().isValid(); - bool representativeHasValidSourceLoc = - representativeConstraint->source->getLoc().isValid(); - if (thisHasValidSourceLoc != representativeHasValidSourceLoc) { - if (thisHasValidSourceLoc) - representativeConstraint = constraint; + continue; + } - continue; - } + // We prefer constraints with locations to constraints without locations. + bool thisHasValidSourceLoc = source->getLoc().isValid(); + bool representativeHasValidSourceLoc = + representativeConstraint->source->getLoc().isValid(); + if (thisHasValidSourceLoc != representativeHasValidSourceLoc) { + if (thisHasValidSourceLoc) + representativeConstraint = constraint; - // Otherwise, order via the constraint itself. - if (constraint < *representativeConstraint) - representativeConstraint = constraint; + continue; + } + + // Otherwise, order via the constraint itself. + if (constraint < *representativeConstraint) + representativeConstraint = constraint; + } } // Sort the concrete constraints, so we get deterministic ordering of @@ -2935,20 +2950,25 @@ static SmallVector getSameTypeComponents( // Find the best anchor and concrete type source for this component. PotentialArchetype *anchor = component[0]; - auto bestConcreteTypeSource = anchor->getConcreteTypeSourceAsWritten(); + const RequirementSource *bestConcreteTypeSource = nullptr; + auto considerNewSource = [&](const RequirementSource *source) { + if (!bestConcreteTypeSource || + source->compare(bestConcreteTypeSource) < 0) + bestConcreteTypeSource = source; + }; - for (auto componentPA : ArrayRef(component).slice(1)){ + if (!anchor->getConcreteTypeSourcesAsWritten().empty()) + bestConcreteTypeSource = anchor->getConcreteTypeSourcesAsWritten()[0]; + + for (auto componentPA : ArrayRef(component)) { // Update the anchor. if (compareDependentTypes(&componentPA, &anchor) < 0) anchor = componentPA; // If this potential archetype has a better concrete type source than // the best we've seen, take it. - if (auto concreteSource = componentPA->getConcreteTypeSourceAsWritten()) { - if (!bestConcreteTypeSource || - concreteSource->compare(bestConcreteTypeSource) < 0) - bestConcreteTypeSource = concreteSource; - } + for (auto source: componentPA->getConcreteTypeSourcesAsWritten()) + considerNewSource(source); } // Record the anchor. diff --git a/test/Constraints/same_types.swift b/test/Constraints/same_types.swift index 593f251f847f7..31cb69f1b1edf 100644 --- a/test/Constraints/same_types.swift +++ b/test/Constraints/same_types.swift @@ -87,6 +87,7 @@ func test6(_ t: T) -> (Y, X) where T.Bar == Y { } func test7(_ t: T) -> (Y, X) where T.Bar == Y, T.Bar.Foo == X { + // expected-warning@-1{{redundant same-type constraint 'T.Bar.Foo' == 'X'}} return (t.bar, t.bar.foo) } @@ -99,8 +100,9 @@ func fail4(_ t: T) -> (Y, Z) func fail5(_ t: T) -> (Y, Z) where - T.Bar.Foo == Z, + T.Bar.Foo == Z, // expected-warning{{redundant same-type constraint 'T.Bar.Foo' == 'Z'}} T.Bar == Y { // expected-error{{associated type 'T.Bar.Foo' cannot be equal to both 'Z' and 'X'}} + // expected-note@-1{{same-type constraint 'T.Bar.Foo' == 'Y.Foo' (aka 'X') implied here}} return (t.bar, t.bar.foo) // expected-error{{cannot convert return expression of type 'X' to return type 'Z'}} }