From 9cba638c0402ae7292379ec7239394684a2913c2 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 8 Sep 2016 00:46:48 -0700 Subject: [PATCH] AST: Fix derivation of conformances in Substitution::subst() in the presence of same-type constraints Applying a substitution list to a Substitution is done in two steps: 1) First, apply the substitution list to the original replacement type R to get the substituted replacement type R'. 2) Second, for each abstract conformance of R from the original substitution, look up a concrete conformance of R' from the substitution list. With minimized generic signatures, we would omit conformances of a nested type T.A if they could be derived from some other conformance already in the substitution list. However, the derivation process was incorrect, because it would only look at the canonical parent type T, and not any other parent type that had a child A which was same-type equivalent to T.A. For example, consider the following code: protocol P1 { associatedtype A : P3 } protocol P2 { associatedtype A : P4 } protocol P3 {} protocol P4 {} func doSomething(...) {} func doSomethingElse(...) where T1.A == T2.A { doSomething(...) } If we specialize doSomethingElse() with a pair of concrete types to replace T1 and T2, we would need to find a concrete conformance to replace the abstract conformance T2.A : P4 in the call to doSomething(). Since the conformance of T2.A : P4 is a redundant requirement, it does not appear in the conformance map; furthermore, T1.A and T2.A are same-type equivalent, so they map to the same archetype. We would therefore look at the canonical parent of T2.A, which is T1, and look up the conformance of T1 : P1 in the substitution list. However, the only requirement P1 imposes on A is the conformance A : P3. There's no conformance A : P4 inside T1 : P1, so we would hit an assertion. Indeed, the conformance T1.A : P4 must be recovered from the conformance of T2 : P2, because P2 requires that A : P4. This patch ensures that the conformance can be found by changing the ArchetypeConformanceMap from a simple mapping of archetypes to conformances into a structure that records same-type constraints as well. So instead of just looking at the canonical parent of the archetype T1.A, we consult the structure to check all archetypes that have T1.A as a child, in this case both T1 and T2. T2 : P2 contains the conformance we need, allowing the above code to be specialized correctly. --- include/swift/AST/Substitution.h | 30 ++++- lib/AST/GenericEnvironment.cpp | 35 +++++- lib/AST/Substitution.cpp | 118 +++++++++++------- .../specialize_same_type_constraint.swift | 53 ++++++++ 4 files changed, 182 insertions(+), 54 deletions(-) create mode 100644 test/SILOptimizer/specialize_same_type_constraint.swift diff --git a/include/swift/AST/Substitution.h b/include/swift/AST/Substitution.h index 9ba7f21fd316c..232d61a625456 100644 --- a/include/swift/AST/Substitution.h +++ b/include/swift/AST/Substitution.h @@ -19,6 +19,7 @@ #include "swift/AST/Type.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/Optional.h" namespace llvm { class raw_ostream; @@ -28,11 +29,30 @@ namespace swift { class ArchetypeType; class GenericEnvironment; class ProtocolConformanceRef; - -/// DenseMap type used internally by Substitution::subst to track conformances -/// applied to archetypes. -using ArchetypeConformanceMap - = llvm::DenseMap>; + +/// Data structure type used internally by Substitution::subst to track +/// conformances applied to archetypes. +class ArchetypeConformanceMap { + using ParentType = std::pair; + + llvm::DenseMap> map; + llvm::DenseMap> parents; + + Optional + lookupArchetypeConformance(ProtocolDecl *proto, + ArrayRef conformances) const; + +public: + Optional + lookupArchetypeConformance(ArchetypeType *replacement, + ProtocolDecl *proto) const; + + void addArchetypeConformances(ArchetypeType *replacement, + ArrayRef conformances); + + void addArchetypeParent(ArchetypeType *replacement, ArchetypeType *parent, + AssociatedTypeDecl *assocType); +}; /// Substitution - A substitution into a generic specialization. class Substitution { diff --git a/lib/AST/GenericEnvironment.cpp b/lib/AST/GenericEnvironment.cpp index 416841163c5e8..16eaffd7536bd 100644 --- a/lib/AST/GenericEnvironment.cpp +++ b/lib/AST/GenericEnvironment.cpp @@ -98,8 +98,39 @@ getSubstitutionMap(ModuleDecl *mod, // Record the replacement type and its conformances. subsMap[archetype] = sub.getReplacement(); - conformanceMap[archetype] = sub.getConformances(); + conformanceMap.addArchetypeConformances(archetype, sub.getConformances()); } - + + for (auto reqt : sig->getRequirements()) { + if (reqt.getKind() != RequirementKind::SameType) + continue; + + auto first = reqt.getFirstType()->getAs(); + auto second = reqt.getSecondType()->getAs(); + + if (!first || !second) + continue; + + auto archetype = mapTypeIntoContext(mod, first)->getAs(); + if (!archetype) + continue; + + auto firstBase = first->getBase(); + auto secondBase = second->getBase(); + + auto firstBaseArchetype = mapTypeIntoContext(mod, firstBase)->getAs(); + auto secondBaseArchetype = mapTypeIntoContext(mod, secondBase)->getAs(); + + if (!firstBaseArchetype || !secondBaseArchetype) + continue; + + if (archetype->getParent() != firstBaseArchetype) + conformanceMap.addArchetypeParent(archetype, firstBaseArchetype, + first->getAssocType()); + if (archetype->getParent() != secondBaseArchetype) + conformanceMap.addArchetypeParent(archetype, secondBaseArchetype, + second->getAssocType()); + } + assert(subs.empty() && "did not use all substitutions?!"); } diff --git a/lib/AST/Substitution.cpp b/lib/AST/Substitution.cpp index 82b0c1f78dd65..33e7c4f924c01 100644 --- a/lib/AST/Substitution.cpp +++ b/lib/AST/Substitution.cpp @@ -24,38 +24,9 @@ using namespace swift; -bool Substitution::operator==(const Substitution &other) const { - // The archetypes may be missing, but we can compare them directly - // because archetypes are always canonical. - return - Replacement->getCanonicalType() == other.Replacement->getCanonicalType() && - Conformance.equals(other.Conformance); -} - -Substitution::Substitution(Type Replacement, - ArrayRef Conformance) - : Replacement(Replacement), Conformance(Conformance) -{ - // The replacement type must be materializable. - assert(Replacement->isMaterializable() - && "cannot substitute with a non-materializable type"); -} - -Substitution Substitution::subst(Module *module, - GenericSignature *sig, - GenericEnvironment *env, - ArrayRef subs) const { - TypeSubstitutionMap subMap; - ArchetypeConformanceMap conformanceMap; - - assert(sig && env); - env->getSubstitutionMap(module, sig, subs, subMap, conformanceMap); - return subst(module, subMap, conformanceMap); -} - -static Optional +Optional ArchetypeConformanceMap:: lookupArchetypeConformance(ProtocolDecl *proto, - ArrayRef conformances) { + ArrayRef conformances) const { for (ProtocolConformanceRef found : conformances) { auto foundProto = found.getRequirement(); if (foundProto == proto) { @@ -73,40 +44,93 @@ lookupArchetypeConformance(ProtocolDecl *proto, return None; } -static Optional +Optional ArchetypeConformanceMap:: lookupArchetypeConformance(ArchetypeType *replacement, - ProtocolDecl *proto, - ArchetypeConformanceMap &conformanceMap) { + ProtocolDecl *proto) const { // Check for conformances for the type that apply to the original // substituted archetype. - auto it = conformanceMap.find(replacement); - if (it != conformanceMap.end()) { - if (auto conformance = lookupArchetypeConformance(proto, it->second)) { + auto foundReplacement = map.find(replacement); + if (foundReplacement != map.end()) { + auto substReplacement = foundReplacement->second; + if (auto conformance = lookupArchetypeConformance(proto, substReplacement)) return conformance; - } } - // Check if we have substitutions for the parent. - if (auto *parent = replacement->getParent()) { - auto *assocType = replacement->getAssocType(); - auto *parentProto = assocType->getProtocol(); + // Check if we have substitutions from one of our parent archetypes. + auto foundParents = parents.find(replacement); + if (foundParents == parents.end()) + return None; + + for (auto parent : foundParents->second) { + auto *parentProto = parent.second->getProtocol(); auto conformance = - lookupArchetypeConformance(parent, parentProto, conformanceMap); + lookupArchetypeConformance(parent.first, parentProto); if (conformance) { if (!conformance->isConcrete()) return ProtocolConformanceRef(proto); auto sub = conformance->getConcrete()->getTypeWitnessSubstAndDecl( - assocType, nullptr).first; + parent.second, nullptr).first; - return lookupArchetypeConformance(proto, sub.getConformances()); + if (auto result = lookupArchetypeConformance(proto, sub.getConformances())) + return result; } } return None; } +void ArchetypeConformanceMap:: +addArchetypeConformances(ArchetypeType *replacement, + ArrayRef conformances) { + assert(replacement); + + auto result = map.insert(std::make_pair(replacement, conformances)); + assert(result.second); + (void) result; + + if (auto *parent = replacement->getParent()) + addArchetypeParent(replacement, parent, replacement->getAssocType()); +} + +void ArchetypeConformanceMap:: +addArchetypeParent(ArchetypeType *replacement, + ArchetypeType *parent, + AssociatedTypeDecl *assocType) { + assert(replacement && parent && assocType); + parents[replacement].push_back(std::make_pair(parent, assocType)); +} + +bool Substitution::operator==(const Substitution &other) const { + // The archetypes may be missing, but we can compare them directly + // because archetypes are always canonical. + return + Replacement->getCanonicalType() == other.Replacement->getCanonicalType() && + Conformance.equals(other.Conformance); +} + +Substitution::Substitution(Type Replacement, + ArrayRef Conformance) + : Replacement(Replacement), Conformance(Conformance) +{ + // The replacement type must be materializable. + assert(Replacement->isMaterializable() + && "cannot substitute with a non-materializable type"); +} + +Substitution Substitution::subst(Module *module, + GenericSignature *sig, + GenericEnvironment *env, + ArrayRef subs) const { + TypeSubstitutionMap subMap; + ArchetypeConformanceMap conformanceMap; + + assert(sig && env); + env->getSubstitutionMap(module, sig, subs, subMap, conformanceMap); + return subst(module, subMap, conformanceMap); +} + Substitution Substitution::subst(Module *module, TypeSubstitutionMap &subMap, ArchetypeConformanceMap &conformanceMap) const { @@ -143,8 +167,8 @@ Substitution Substitution::subst(Module *module, // If the original type was an archetype, check the conformance map. if (auto replacementArch = Replacement->getAs()) { - conformance = lookupArchetypeConformance(replacementArch, proto, - conformanceMap); + conformance = conformanceMap.lookupArchetypeConformance( + replacementArch, proto); } // If that didn't find anything, we can still synthesize AnyObject diff --git a/test/SILOptimizer/specialize_same_type_constraint.swift b/test/SILOptimizer/specialize_same_type_constraint.swift new file mode 100644 index 0000000000000..6e4cd75aee96a --- /dev/null +++ b/test/SILOptimizer/specialize_same_type_constraint.swift @@ -0,0 +1,53 @@ +// RUN: %target-swift-frontend -O -emit-sil -primary-file %s | %FileCheck %s + +protocol FirstChild {} + +protocol FirstParent { + associatedtype Child : FirstChild + + var firstChild: Child { get } +} + +protocol SecondChild {} + +protocol SecondParent { + associatedtype Child : SecondChild + + var secondChild: Child { get } +} + +@_semantics("optimize.sil.never") +func takesFirstChild(t: T) {} + +@_semantics("optimize.sil.never") +func takesSecondChild(t: T) {} + +@inline(never) +func doStuff(f: First, s: Second) + where First.Child == Second.Child { + takesFirstChild(t: f.firstChild) + takesSecondChild(t: f.firstChild) + + takesFirstChild(t: s.secondChild) + takesSecondChild(t: s.secondChild) +} + +struct ConcreteChild : FirstChild, SecondChild {} + +struct ConcreteFirstParent : FirstParent { + var firstChild: ConcreteChild { return ConcreteChild() } +} + +struct ConcreteSecondParent : SecondParent { + var secondChild: ConcreteChild { return ConcreteChild() } +} + +doStuff(f: ConcreteFirstParent(), + s: ConcreteSecondParent()) + +// CHECK-LABEL: sil shared [noinline] @_TTSf4d_d___TTSg5GV31specialize_same_type_constraint19ConcreteFirstParentVS_13ConcreteChild_GS0_S1__S_11FirstParentS__GVS_20ConcreteSecondParentS1__GS3_S1__S_12SecondParentS____TF31specialize_same_type_constraint7doStuffu0_RxS_11FirstParent_S_12SecondParentwx5Childzw_5ChildrFT1fx1sq__T_ +// CHECK: [[FIRST:%.*]] = function_ref @_TF31specialize_same_type_constraint15takesFirstChilduRxS_10FirstChildrFT1tx_T_ +// CHECK: apply [[FIRST]]({{.*}}) : $@convention(thin) <τ_0_0 where τ_0_0 : FirstChild> (@in τ_0_0) -> () +// CHECK: [[SECOND:%.*]] = function_ref @_TF31specialize_same_type_constraint16takesSecondChilduRxS_11SecondChildrFT1tx_T_ +// CHECK: apply [[SECOND]]({{.*}}) : $@convention(thin) <τ_0_0 where τ_0_0 : SecondChild> (@in τ_0_0) -> () +// CHECK: return