From 125ad63f718839cc1e8ecc5b97e23223a851b04d Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 16 Nov 2016 13:34:32 -0800 Subject: [PATCH 1/5] [Archetype builder] Remove RequirementSource::Kind::OuterScope. "Explicit" covers everything we need now. NFC --- include/swift/AST/ArchetypeBuilder.h | 12 +----------- lib/AST/ASTContext.cpp | 3 +-- lib/AST/ArchetypeBuilder.cpp | 14 ++------------ test/Generics/requirement_inference.swift | 2 +- test/SILGen/generic_witness.swift | 6 +++--- 5 files changed, 8 insertions(+), 29 deletions(-) diff --git a/include/swift/AST/ArchetypeBuilder.h b/include/swift/AST/ArchetypeBuilder.h index 777d0fdc9cdd5..bba7febd16f17 100644 --- a/include/swift/AST/ArchetypeBuilder.h +++ b/include/swift/AST/ArchetypeBuilder.h @@ -83,11 +83,6 @@ class RequirementSource { /// /// These are dropped when building the GenericSignature. Inherited, - - /// The requirement came from an outer scope. - /// FIXME: eliminate this in favor of keeping requirement sources in - /// GenericSignatures, at least non-canonical ones? - OuterScope, }; RequirementSource(Kind kind, SourceLoc loc) : StoredKind(kind), Loc(loc) { } @@ -252,13 +247,8 @@ class ArchetypeBuilder { void addRequirement(const Requirement &req, RequirementSource source); /// \brief Add all of a generic signature's parameters and requirements. - /// - /// FIXME: Requirements from the generic signature are treated as coming from - /// an outer scope. Setting \c treatRequirementsAsExplicit to true disables - /// this behavior. void addGenericSignature(GenericSignature *sig, - GenericEnvironment *genericEnv, - bool treatRequirementsAsExplicit = false); + GenericEnvironment *genericEnv); /// \brief Build the generic signature. GenericSignature *getGenericSignature(); diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 2c8afc50c0ff1..010fc39b9f08d 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -1263,8 +1263,7 @@ ArchetypeBuilder *ASTContext::getOrCreateArchetypeBuilder( // Create a new archetype builder with the given signature. auto builder = new ArchetypeBuilder(*mod, Diags); - builder->addGenericSignature(sig, nullptr, - /*treatRequirementsAsExplicit=*/true); + builder->addGenericSignature(sig, nullptr); // Store this archetype builder. Impl.ArchetypeBuilders[{sig, mod}] diff --git a/lib/AST/ArchetypeBuilder.cpp b/lib/AST/ArchetypeBuilder.cpp index c61104a7229e5..02cf4a311c28e 100644 --- a/lib/AST/ArchetypeBuilder.cpp +++ b/lib/AST/ArchetypeBuilder.cpp @@ -61,10 +61,6 @@ void RequirementSource::dump(llvm::raw_ostream &out, out << "inferred"; break; - case OuterScope: - out << "outer"; - break; - case Inherited: out << "inherited"; break; @@ -685,7 +681,6 @@ ArchetypeBuilder::PotentialArchetype::getType(ArchetypeBuilder &builder) { switch (conforms.second.getKind()) { case RequirementSource::Explicit: case RequirementSource::Inferred: - case RequirementSource::OuterScope: case RequirementSource::Protocol: case RequirementSource::Redundant: Protos.push_back(conforms.first); @@ -2010,14 +2005,10 @@ ArchetypeBuilder::mapTypeOutOfContext(ModuleDecl *M, } void ArchetypeBuilder::addGenericSignature(GenericSignature *sig, - GenericEnvironment *env, - bool treatRequirementsAsExplicit) { + GenericEnvironment *env) { if (!sig) return; - RequirementSource::Kind sourceKind = treatRequirementsAsExplicit - ? RequirementSource::Explicit - : RequirementSource::OuterScope; - + RequirementSource::Kind sourceKind = RequirementSource::Explicit; for (auto param : sig->getGenericParams()) { addGenericParameter(param); @@ -2057,7 +2048,6 @@ static void collectRequirements(ArchetypeBuilder &builder, switch (source.getKind()) { case RequirementSource::Explicit: case RequirementSource::Inferred: - case RequirementSource::OuterScope: // The requirement was explicit and required, keep it. break; diff --git a/test/Generics/requirement_inference.swift b/test/Generics/requirement_inference.swift index bb3d5fe8185b3..9f3e3747c33a3 100644 --- a/test/Generics/requirement_inference.swift +++ b/test/Generics/requirement_inference.swift @@ -57,7 +57,7 @@ class Fox : P1 { class Box { // CHECK-LABEL: .unpack@ // CHECK-NEXT: Requirements: -// CHECK-NEXT: T : Fox [outer] +// CHECK-NEXT: T : Fox [explicit] func unpack(_ x: X1) {} } diff --git a/test/SILGen/generic_witness.swift b/test/SILGen/generic_witness.swift index cfc332d6095a0..708952d1b67a6 100644 --- a/test/SILGen/generic_witness.swift +++ b/test/SILGen/generic_witness.swift @@ -50,7 +50,7 @@ struct Canvas where I.Paint : Pen { extension Canvas : Medium {} -// CHECK-LABEL: sil hidden [transparent] [thunk] @_TTWuRx15generic_witness3Inkwx5PaintS_3PenrGVS_6Canvasx_S_6MediumS_FS4_4drawuRd__S_6Pencilwd__6StrokezWx7TextureS1__rfT5paintWxS7_S1__6pencilqd___T_ : $@convention(witness_method) <τ_0_0 where τ_0_0 : Ink, τ_0_0.Paint : Pen><τ_1_0 where τ_1_0 : Pencil, τ_0_0.Paint == τ_1_0.Stroke> (@in τ_0_0.Paint, @in τ_1_0, @in_guaranteed Canvas<τ_0_0>) -> () -// CHECK: [[FN:%.*]] = function_ref @_TFV15generic_witness6Canvas4drawuRd__S_6Pencilwx5Paintzwd__6StrokerfT5paintwxS2_6pencilqd___T_ : $@convention(method) <τ_0_0 where τ_0_0 : Ink, τ_0_0.Paint : Pen><τ_1_0 where τ_1_0 : Pencil, τ_0_0.Paint == τ_1_0.Stroke> (@in τ_0_0.Paint, @in τ_1_0, Canvas<τ_0_0>) -> () -// CHECK: apply [[FN]]<τ_0_0, τ_1_0, τ_0_0.Paint>({{.*}}) : $@convention(method) <τ_0_0 where τ_0_0 : Ink, τ_0_0.Paint : Pen><τ_1_0 where τ_1_0 : Pencil, τ_0_0.Paint == τ_1_0.Stroke> (@in τ_0_0.Paint, @in τ_1_0, Canvas<τ_0_0>) -> () +// CHECK-LABEL: sil hidden [transparent] [thunk] @_TTWuRx15generic_witness3Inkwx5PaintS_3PenrGVS_6Canvasx_S_6MediumS_FS4_4drawuRd__S_6Pencilwd__6StrokezWx7TextureS1__rfT5paintWxS7_S1__6pencilqd___T_ : $@convention(witness_method) <τ_0_0 where τ_0_0 : Ink><τ_1_0 where τ_1_0 : Pencil, τ_0_0.Paint == τ_1_0.Stroke> (@in τ_0_0.Paint, @in τ_1_0, @in_guaranteed Canvas<τ_0_0>) -> () +// CHECK: [[FN:%.*]] = function_ref @_TFV15generic_witness6Canvas4drawuRd__S_6Pencilwx5Paintzwd__6StrokerfT5paintwxS2_6pencilqd___T_ : $@convention(method) <τ_0_0 where τ_0_0 : Ink><τ_1_0 where τ_1_0 : Pencil, τ_0_0.Paint == τ_1_0.Stroke> (@in τ_0_0.Paint, @in τ_1_0, Canvas<τ_0_0>) -> () +// CHECK: apply [[FN]]<τ_0_0, τ_1_0>({{.*}}) : $@convention(method) <τ_0_0 where τ_0_0 : Ink><τ_1_0 where τ_1_0 : Pencil, τ_0_0.Paint == τ_1_0.Stroke> (@in τ_0_0.Paint, @in τ_1_0, Canvas<τ_0_0>) -> () // CHECK: } From 6a7d257bb770d19d08df33badabec01f28464722 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 16 Nov 2016 13:48:58 -0800 Subject: [PATCH 2/5] [Archetype builder] Mark same-type constraints on nested types as redundant. When a same-type constraint equates a potential archetype with a same-type constraint, it implies equality of the nested types with the appropriate witnesses. However, these were getting marked as "explicit" when they should be "redundant". Fixes rdar://problem/29295062. --- lib/AST/ArchetypeBuilder.cpp | 7 ++++--- test/Generics/requirement_inference.swift | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/AST/ArchetypeBuilder.cpp b/lib/AST/ArchetypeBuilder.cpp index 02cf4a311c28e..5cdf9806caee7 100644 --- a/lib/AST/ArchetypeBuilder.cpp +++ b/lib/AST/ArchetypeBuilder.cpp @@ -1275,6 +1275,7 @@ bool ArchetypeBuilder::addSameTypeRequirementToConcrete( T->SameTypeSource = Source; // Recursively resolve the associated types to their concrete types. + RequirementSource nestedSource(RequirementSource::Redundant, Source.getLoc()); for (auto nested : T->getNestedTypes()) { AssociatedTypeDecl *assocType = nested.second.front()->getResolvedAssociatedType(); @@ -1283,7 +1284,7 @@ bool ArchetypeBuilder::addSameTypeRequirementToConcrete( concreteArchetype->getNestedType(nested.first); addSameTypeRequirementToConcrete(nested.second.front(), witnessType.getValue(), - Source); + nestedSource); } else { assert(conformances.count(assocType->getProtocol()) > 0 && "missing conformance?"); @@ -1293,11 +1294,11 @@ bool ArchetypeBuilder::addSameTypeRequirementToConcrete( if (auto witnessPA = resolveArchetype(witnessType)) { addSameTypeRequirementBetweenArchetypes(nested.second.front(), witnessPA, - Source); + nestedSource); } else { addSameTypeRequirementToConcrete(nested.second.front(), witnessType, - Source); + nestedSource); } } } diff --git a/test/Generics/requirement_inference.swift b/test/Generics/requirement_inference.swift index 9f3e3747c33a3..81615f568eb47 100644 --- a/test/Generics/requirement_inference.swift +++ b/test/Generics/requirement_inference.swift @@ -136,6 +136,20 @@ func inferSameType2(_: T) where U.P4Assoc : P2, T.P3Assoc == U.P // CHECK-NEXT: Generic signature func inferSameType3(_: T) where T.CommonAssoc : P1, T : PCommonAssoc2 {} +struct X6 : PAssoc { + typealias Assoc = X6 +} + +// CHECK-LABEL: .inferSameType4@ +// CHECK-NEXT: Requirements: +// CHECK-NEXT: T : PAssoc [explicit @ +// CHECK-NEXT: T[.PAssoc].Assoc == X6 [explicit +// CHECK-NEXT: T[.PAssoc].Assoc[.PAssoc].Assoc == X6.Assoc [redundant +// CHECK-NEXT: T[.PAssoc].Assoc[.PAssoc].Assoc[.PAssoc].Assoc == X6.Assoc [redundant +// CHECK-NEXT: Generic signature: +// CHECK-NEXT: Canonical generic signature: <τ_0_0 where τ_0_0 : PAssoc, τ_0_0.Assoc == X6> +func inferSameType4(_: T) where T.Assoc : PAssoc, T.Assoc.Assoc : PAssoc, T.Assoc == X6 {} + protocol P5 { associatedtype Element } From 34f807375b9637afd139301a0c6b25740cb68f37 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 16 Nov 2016 13:59:48 -0800 Subject: [PATCH 3/5] [Archetype builder] Allow non-concrete conformances with same-type-to-concrete requirements. Fixes rdar://problem/29279577. --- lib/AST/ArchetypeBuilder.cpp | 18 ++++++++++++------ test/Generics/same_type_constraints_objc.swift | 8 ++++++++ 2 files changed, 20 insertions(+), 6 deletions(-) create mode 100644 test/Generics/same_type_constraints_objc.swift diff --git a/lib/AST/ArchetypeBuilder.cpp b/lib/AST/ArchetypeBuilder.cpp index 5cdf9806caee7..238ab47e07942 100644 --- a/lib/AST/ArchetypeBuilder.cpp +++ b/lib/AST/ArchetypeBuilder.cpp @@ -1252,7 +1252,7 @@ bool ArchetypeBuilder::addSameTypeRequirementToConcrete( } // Make sure the concrete type fulfills the requirements on the archetype. - DenseMap conformances; + DenseMap conformances; if (!Concrete->is()) { for (auto conforms : T->getConformsTo()) { auto protocol = conforms.first; @@ -1265,8 +1265,7 @@ bool ArchetypeBuilder::addSameTypeRequirementToConcrete( return true; } - assert(conformance->isConcrete() && "Abstract conformance?"); - conformances[protocol] = conformance->getConcrete(); + conformances.insert({protocol, *conformance}); } } @@ -1288,9 +1287,16 @@ bool ArchetypeBuilder::addSameTypeRequirementToConcrete( } else { assert(conformances.count(assocType->getProtocol()) > 0 && "missing conformance?"); - auto witness = conformances[assocType->getProtocol()] - ->getTypeWitness(assocType, getLazyResolver()); - auto witnessType = witness.getReplacement(); + auto conformance = conformances.find(assocType->getProtocol())->second; + Type witnessType; + if (conformance.isConcrete()) { + witnessType = conformance.getConcrete() + ->getTypeWitness(assocType, getLazyResolver()) + .getReplacement(); + } else { + witnessType = DependentMemberType::get(Concrete, assocType); + } + if (auto witnessPA = resolveArchetype(witnessType)) { addSameTypeRequirementBetweenArchetypes(nested.second.front(), witnessPA, diff --git a/test/Generics/same_type_constraints_objc.swift b/test/Generics/same_type_constraints_objc.swift new file mode 100644 index 0000000000000..c76dd1db5920c --- /dev/null +++ b/test/Generics/same_type_constraints_objc.swift @@ -0,0 +1,8 @@ +// RUN: %target-parse-verify-swift +// REQUIRES: objc_interop + +@objc protocol Q {} + +struct X1 { } + +extension X1 where T == Q { } From c73316e0a32e3390f15011715da9c64674ee22ea Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 16 Nov 2016 20:35:51 -0800 Subject: [PATCH 4/5] Update test case for same-type-to-concrete redundancy fixes. --- test/SILOptimizer/prespecialize.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/SILOptimizer/prespecialize.swift b/test/SILOptimizer/prespecialize.swift index 60b67fa3a1d70..d81de8a6fa9c7 100644 --- a/test/SILOptimizer/prespecialize.swift +++ b/test/SILOptimizer/prespecialize.swift @@ -11,7 +11,7 @@ // CHECK-LABEL: sil [noinline] @_TF13prespecialize4testFTRGSaSi_4sizeSi_T_ // // function_ref specialized Collection.makeIterator() -> IndexingIterator -// CHECK: function_ref @_TTSgq5GVs14CountableRangeSi_GS_Si_s10Collections___TFesRxs10Collectionwx8IteratorzGVs16IndexingIteratorx_wx8_ElementzWxS0_7Element_rS_12makeIteratorfT_GS1_x_ +// CHECK: function_ref @_TTSgq5GVs14CountableRangeSi_GS_Si_s10Collections___TFesRxs10Collectionwx8IteratorzGVs16IndexingIteratorx_rS_12makeIteratorfT_GS1_x_ // // function_ref specialized IndexingIterator.next() -> A._Element? // CHECK: function_ref @_TTSgq5GVs14CountableRangeSi_GS_Si_s14_IndexableBases___TFVs16IndexingIterator4nextfT_GSqwx8_Element_ From be791e5c8353fb5438c41001ca7235cca05349d2 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 16 Nov 2016 20:48:47 -0800 Subject: [PATCH 5/5] [Archetype builder] Check concrete types against superclass constraints. This was a blatant hole in our checking, admitting ill-formed generic signatures that would crash down the line. Fixes rdar://problem/29288428. --- lib/AST/ArchetypeBuilder.cpp | 26 +++++++++++++++++++++++ test/Generics/same_type_constraints.swift | 15 +++++++++++++ 2 files changed, 41 insertions(+) diff --git a/lib/AST/ArchetypeBuilder.cpp b/lib/AST/ArchetypeBuilder.cpp index 238ab47e07942..c4213430f9d57 100644 --- a/lib/AST/ArchetypeBuilder.cpp +++ b/lib/AST/ArchetypeBuilder.cpp @@ -988,6 +988,21 @@ bool ArchetypeBuilder::addSuperclassRequirement(PotentialArchetype *T, }); } + // Make sure the concrete type fulfills the superclass requirement + // of the archetype. + if (T->isConcreteType()) { + Type concrete = T->getConcreteType(); + if (!Superclass->isExactSuperclassOf(concrete, getLazyResolver())) { + Diags.diagnose(T->getSameTypeSource().getLoc(), + diag::type_does_not_inherit, + T->getRootParam(), concrete, Superclass) + .highlight(Source.getLoc()); + return true; + } + + return false; + } + // Local function to handle the update of superclass conformances // when the superclass constraint changes. auto updateSuperclassConformances = [&] { @@ -1273,6 +1288,17 @@ bool ArchetypeBuilder::addSameTypeRequirementToConcrete( T->ArchetypeOrConcreteType = NestedType::forConcreteType(Concrete); T->SameTypeSource = Source; + // Make sure the concrete type fulfills the superclass requirement + // of the archetype. + if (T->Superclass) { + if (!T->Superclass->isExactSuperclassOf(Concrete, getLazyResolver())) { + Diags.diagnose(Source.getLoc(), diag::type_does_not_inherit, + T->getRootParam(), Concrete, T->Superclass) + .highlight(T->SuperclassSource->getLoc()); + return true; + } + } + // Recursively resolve the associated types to their concrete types. RequirementSource nestedSource(RequirementSource::Redundant, Source.getLoc()); for (auto nested : T->getNestedTypes()) { diff --git a/test/Generics/same_type_constraints.swift b/test/Generics/same_type_constraints.swift index 838eb2f9e0099..fe7b38f15ca19 100644 --- a/test/Generics/same_type_constraints.swift +++ b/test/Generics/same_type_constraints.swift @@ -318,3 +318,18 @@ struct EventHorizon : Timewarp { func activate(_ t: T) {} activate(Teleporter()) + +// rdar://problem/29288428 +class C {} + +protocol P9 { + associatedtype A +} + +struct X7 where T.A : C { } + +extension X7 where T.A == Int { } // expected-error 2{{'T' requires that 'Int' inherit from 'C'}} + +struct X8 { } + +extension X8 where T == Int { } // expected-error 2{{'T' requires that 'Int' inherit from 'C'}}