Skip to content

Commit c9c3b3f

Browse files
kavonkavon
committed
[NCGenerics] only print ~Copyable in interface
We can't simply emit the desugared, expanded version of the requirements because there's no way to pretty-print the type `some ~Copyable` when the `~Copyable`'s get replaced with the absence of `Copyable`. We'd be left with just `some _` or need to invent a new top type so we can write `some Top`. Thus, it's best to simply reverse the expansion of default requirements. This patch introduces a `collapseDefaultRequirements` that reverses what `expandDefaultRequirements` does in `RequirementLowering`. It eliminates all automatically inferred `Copyable` requirements and puts back the `~Copyable` into the requirements list just before printing.
1 parent 666d11f commit c9c3b3f

File tree

5 files changed

+172
-25
lines changed

5 files changed

+172
-25
lines changed

‎lib/AST/ASTPrinter.cpp

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -943,6 +943,7 @@ class PrintAST : public ASTVisitor<PrintAST> {
943943
InnermostOnly = 4,
944944
SwapSelfAndDependentMemberType = 8,
945945
PrintInherited = 16,
946+
CollapseDefaultReqs = 32,
946947
};
947948

948949
void printInheritedFromRequirementSignature(ProtocolDecl *proto,
@@ -1561,7 +1562,7 @@ void PrintAST::printInheritedFromRequirementSignature(ProtocolDecl *proto,
15611562
Decl *attachingTo) {
15621563
printGenericSignature(
15631564
proto->getRequirementSignatureAsGenericSignature(),
1564-
PrintInherited,
1565+
PrintInherited | CollapseDefaultReqs,
15651566
[&](const Requirement &req) {
15661567
// Skip the inferred 'Self : AnyObject' constraint if this is an
15671568
// @objc protocol.
@@ -1580,7 +1581,7 @@ void PrintAST::printInheritedFromRequirementSignature(ProtocolDecl *proto,
15801581

15811582
void PrintAST::printWhereClauseFromRequirementSignature(ProtocolDecl *proto,
15821583
Decl *attachingTo) {
1583-
unsigned flags = PrintRequirements;
1584+
unsigned flags = PrintRequirements | CollapseDefaultReqs;
15841585
if (isa<AssociatedTypeDecl>(attachingTo))
15851586
flags |= SwapSelfAndDependentMemberType;
15861587
printGenericSignature(
@@ -1684,6 +1685,50 @@ void PrintAST::printGenericSignature(
16841685
}
16851686
}
16861687

1688+
/// \returns the invertible protocol kind iff this requirement was expanded as
1689+
/// a default conformance to an invertible protocol.
1690+
static llvm::Optional<InvertibleProtocolKind>
1691+
extractDefaultedInvertible(const Requirement &req) {
1692+
if (req.getKind() != RequirementKind::Conformance)
1693+
return llvm::None;
1694+
1695+
auto constraintTy = req.getSecondType();
1696+
if (auto kp = constraintTy->getKnownProtocol())
1697+
return getInvertibleProtocolKind(*kp);
1698+
1699+
return llvm::None;
1700+
}
1701+
1702+
/// Undoes what expandDefaultRequirements does during requirement desugaring
1703+
/// so that inverses are emitted in interface files and the default requirements
1704+
/// are cleaned-up.
1705+
static
1706+
void collapseDefaultRequirements(ArrayRef<GenericTypeParamType *> genericParams,
1707+
SmallVectorImpl<Requirement> &reqs) {
1708+
// Tracks which default conformance requirements were encountered and removed.
1709+
llvm::SmallDenseMap<CanType, InvertibleProtocolSet> state;
1710+
for (auto *gtpt : genericParams)
1711+
state[gtpt->getCanonicalType()] = InvertibleProtocolSet::full();
1712+
1713+
reqs.erase(llvm::remove_if(reqs, [&](Requirement req) {
1714+
auto invertible = extractDefaultedInvertible(req);
1715+
if (!invertible)
1716+
return false;
1717+
1718+
state[req.getFirstType()->getCanonicalType()].remove(*invertible);
1719+
return true;
1720+
}), reqs.end());
1721+
1722+
// The remaining inverses are added as requirements.
1723+
for (auto *gtpt : genericParams) {
1724+
auto neededInverses = state[gtpt->getCanonicalType()];
1725+
for (auto ip : neededInverses) {
1726+
reqs.push_back({RequirementKind::Layout, gtpt,
1727+
LayoutConstraint::getInverseConstraint(ip)});
1728+
}
1729+
}
1730+
}
1731+
16871732
void PrintAST::printSingleDepthOfGenericSignature(
16881733
ArrayRef<GenericTypeParamType *> genericParams,
16891734
ArrayRef<Requirement> requirements, unsigned flags,
@@ -1695,14 +1740,20 @@ void PrintAST::printSingleDepthOfGenericSignature(
16951740

16961741
void PrintAST::printSingleDepthOfGenericSignature(
16971742
ArrayRef<GenericTypeParamType *> genericParams,
1698-
ArrayRef<Requirement> requirements, bool &isFirstReq, unsigned flags,
1743+
ArrayRef<Requirement> origRequirements, bool &isFirstReq, unsigned flags,
16991744
llvm::function_ref<bool(const Requirement &)> filter) {
17001745
bool printParams = (flags & PrintParams);
17011746
bool printRequirements = (flags & PrintRequirements);
17021747
printRequirements &= Options.PrintGenericRequirements;
17031748
bool printInherited = (flags & PrintInherited);
17041749
bool swapSelfAndDependentMemberType =
17051750
(flags & SwapSelfAndDependentMemberType);
1751+
bool collapseDefaults = (flags & CollapseDefaultReqs);
1752+
1753+
// Remove default requirements and reconstitute inverses.
1754+
SmallVector<Requirement, 8> requirements(origRequirements);
1755+
if (collapseDefaults)
1756+
collapseDefaultRequirements(genericParams, requirements);
17061757

17071758
unsigned typeContextDepth = 0;
17081759
SubstitutionMap subMap;
@@ -2567,7 +2618,7 @@ void PrintAST::printDeclGenericRequirements(GenericContext *decl) {
25672618
return;
25682619

25692620
Printer.printStructurePre(PrintStructureKind::DeclGenericParameterClause);
2570-
printGenericSignature(genericSig, PrintRequirements,
2621+
printGenericSignature(genericSig, PrintRequirements | CollapseDefaultReqs,
25712622
[parentSig](const Requirement &req) {
25722623
if (parentSig)
25732624
return !parentSig->isRequirementSatisfied(req);
@@ -2722,7 +2773,8 @@ void PrintAST::printSynthesizedExtensionImpl(Type ExtendedType,
27222773
auto Sig = ED->getGenericSignature();
27232774
printSingleDepthOfGenericSignature(Sig.getGenericParams(),
27242775
Sig.getRequirements(),
2725-
IsFirst, PrintRequirements,
2776+
IsFirst,
2777+
PrintRequirements | CollapseDefaultReqs,
27262778
[](const Requirement &Req){
27272779
return true;
27282780
});
@@ -2820,6 +2872,9 @@ void PrintAST::printExtension(ExtensionDecl *decl) {
28202872
auto baseGenericSig = decl->getExtendedNominal()->getGenericSignature();
28212873
assert(baseGenericSig &&
28222874
"an extension can't be generic if the base type isn't");
2875+
// NOTE: We do _not_ CollapseDefaultReqs for the where-clause of an
2876+
// extension, because conditions involving Copyable here are not
2877+
// automatically inferred based on the extension itself.
28232878
printGenericSignature(genericSig, PrintRequirements,
28242879
[baseGenericSig](const Requirement &req) -> bool {
28252880
// Only include constraints that are not satisfied by the base type.
@@ -6969,7 +7024,8 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
69697024
printFunctionExtInfo(T);
69707025
printGenericSignature(T->getGenericSignature(),
69717026
PrintAST::PrintParams |
6972-
PrintAST::PrintRequirements);
7027+
PrintAST::PrintRequirements |
7028+
PrintAST::CollapseDefaultReqs);
69737029
Printer << " ";
69747030

69757031
visitAnyFunctionTypeParams(T->getParams(), /*printLabels*/true);

‎lib/AST/Requirement.cpp

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,12 @@ CheckRequirementResult Requirement::checkRequirement(
130130
}
131131

132132
if (auto *archetypeType = firstType->getAs<ArchetypeType>()) {
133-
auto layout = archetypeType->getLayoutConstraint();
134-
if (layout && layout.merge(getLayoutConstraint()))
135-
return CheckRequirementResult::Success;
136-
137-
return CheckRequirementResult::RequirementFailure;
133+
if (auto layout = archetypeType->getLayoutConstraint()) {
134+
if (layout.merge(getLayoutConstraint()))
135+
return CheckRequirementResult::Success;
136+
else
137+
return CheckRequirementResult::RequirementFailure;
138+
}
138139
}
139140

140141
if (getLayoutConstraint()->isClass()) {
@@ -144,6 +145,18 @@ CheckRequirementResult Requirement::checkRequirement(
144145
return CheckRequirementResult::RequirementFailure;
145146
}
146147

148+
// An inverse constraint is "satisfied" if the type does not conform.
149+
if (auto ip = getLayoutConstraint()->getInverseKind()) {
150+
switch (*ip) {
151+
case InvertibleProtocolKind::Copyable:
152+
if (firstType->isNoncopyable())
153+
return CheckRequirementResult::Success;
154+
break;
155+
}
156+
157+
return CheckRequirementResult::RequirementFailure;
158+
}
159+
147160
// TODO: Statically check other layout constraints, once they can
148161
// be spelled in Swift.
149162
return CheckRequirementResult::Success;

‎lib/Frontend/ModuleInterfaceSupport.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,13 @@ class InheritedProtocolCollector {
704704
inherited->isSpecificProtocol(KnownProtocolKind::Actor))
705705
return TypeWalker::Action::SkipChildren;
706706

707+
// Do not synthesize an extension to print a conformance to an
708+
// invertible protocol, as their conformances are always re-inferred
709+
// using the interface itself.
710+
if (auto kp = inherited->getKnownProtocolKind())
711+
if (getInvertibleProtocolKind(*kp))
712+
return TypeWalker::Action::SkipChildren;
713+
707714
if (inherited->isSPI() && printOptions.printPublicInterface())
708715
return TypeWalker::Action::Continue;
709716

‎test/ModuleInterface/Inputs/NoncopyableGenerics_Misc.swift

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
// These are purely to add test coverage of different constructs when emitting modules
2+
3+
public protocol _NoCopyP: ~Copyable {}
4+
25
public struct _Toys {
36
static func test_parallelAssignment() {
47
var y: Int
@@ -18,5 +21,28 @@ public struct _Toys {
1821
}
1922
}
2023

21-
public static func pickOne<T>(_ a: T, _ b: T) -> T { return a }
24+
public static func allCopyable1<T>(_ a: T, _ b: T) -> T { return a }
25+
26+
public static func allCopyable2<T>(_ s: T)
27+
where T: _NoCopyP {}
28+
29+
public static func oneCopyable1<T, V: ~Copyable>(_ s: T, _ v: borrowing V)
30+
where T: _NoCopyP {}
31+
32+
public static func oneCopyable2<T, V>(_ s: borrowing T, _ v: V)
33+
where T: _NoCopyP, T: ~Copyable {}
34+
35+
public static func oneCopyable3<T, V>(_ s: borrowing T, _ v: V)
36+
where T: _NoCopyP & ~Copyable {}
37+
38+
public static func basic_some(_ s: some _NoCopyP) {}
39+
40+
public static func basic_some_nc(_ s: borrowing some _NoCopyP & ~Copyable) {}
2241
}
42+
43+
public struct ExplicitHello<T: ~Copyable>: ~Copyable {
44+
let thing: T
45+
}
46+
extension ExplicitHello: Copyable where T: Copyable {}
47+
48+
public struct Hello<T: ~Copyable> {}

‎test/ModuleInterface/noncopyable_generics.swift

Lines changed: 58 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,28 +17,73 @@
1717
// RUN: %FileCheck %s --check-prefix=CHECK-MISC < %t/NoncopyableGenerics_Misc.swiftinterface
1818
// RUN: %FileCheck %s < %t/Swiftskell.swiftinterface
1919

20+
// See if we can compile a module through just the interface and typecheck using it.
21+
22+
// RUN: %target-swift-frontend -compile-module-from-interface \
23+
// RUN: -enable-experimental-feature NoncopyableGenerics \
24+
// RUN: %t/NoncopyableGenerics_Misc.swiftinterface -o %t/NoncopyableGenerics_Misc.swiftmodule
25+
26+
// RUN: %target-swift-frontend -compile-module-from-interface \
27+
// RUN: -enable-experimental-feature NoncopyableGenerics \
28+
// RUN: %t/Swiftskell.swiftinterface -o %t/Swiftskell.swiftmodule
29+
30+
// RUN: %target-swift-frontend -typecheck -I %t %s \
31+
// RUN: -enable-experimental-feature NoncopyableGenerics
32+
2033
// REQUIRES: asserts
2134

2235
import NoncopyableGenerics_Misc
2336

24-
// CHECK-MISC: public struct rdar118697289_S1<Element> where Element : Swift.Copyable {
25-
// CHECK-MISC: public struct rdar118697289_S2<Element> where Element : Swift.Copyable {
26-
// CHECK-MISC: public static func pickOne<T>(_ a: T, _ b: T) -> T where T : Swift.Copyable
37+
// CHECK-MISC: public struct _Toys {
38+
// CHECK-MISC: public struct rdar118697289_S1<Element> {
39+
// CHECK-MISC: public struct rdar118697289_S2<Element> {
40+
// CHECK-MISC: public static func allCopyable1<T>(_ a: T, _ b: T) -> T
41+
42+
// CHECK-MISC: #if compiler(>=5.3) && $NoncopyableGenerics
43+
// CHECK-MISC-NEXT: public static func allCopyable2<T>(_ s: T) where T : NoncopyableGenerics_Misc._NoCopyP
44+
45+
// CHECK-MISC: #if compiler(>=5.3) && $NoncopyableGenerics
46+
// CHECK-MISC-NEXT: public static func oneCopyable1<T, V>(_ s: T, _ v: borrowing V) where T : {{.*}}._NoCopyP, V : ~Copyable
47+
48+
// CHECK-MISC: #if compiler(>=5.3) && $NoncopyableGenerics
49+
// CHECK-MISC-NEXT: public static func oneCopyable2<T, V>(_ s: borrowing T, _ v: V) where T : {{.*}}._NoCopyP, T : ~Copyable
50+
51+
// CHECK-MISC: #if compiler(>=5.3) && $NoncopyableGenerics
52+
// CHECK-MISC-NEXT: public static func oneCopyable3<T, V>(_ s: borrowing T, _ v: V) where T : {{.*}}._NoCopyP, T : ~Copyable
53+
54+
// CHECK-MISC: #if compiler(>=5.3) && $NoncopyableGenerics
55+
// CHECK-MISC-NEXT: public static func basic_some(_ s: some _NoCopyP)
56+
57+
// CHECK-MISC: #if compiler(>=5.3) && $NoncopyableGenerics
58+
// CHECK-MISC-NEXT: public static func basic_some_nc(_ s: borrowing some _NoCopyP & ~Copyable)
59+
60+
// CHECK-MISC: #if compiler(>=5.3) && $NoncopyableGenerics
61+
// CHECK-MISC-NEXT: public struct ExplicitHello<T> : ~Copyable where T : ~Copyable {
62+
63+
// CHECK-MISC: #if compiler(>=5.3) && $NoncopyableGenerics
64+
// CHECK-MISC-NEXT: extension {{.*}}.ExplicitHello : Swift.Copyable where T : Swift.Copyable {
65+
66+
// CHECK-MISC: #if compiler(>=5.3) && $NoncopyableGenerics
67+
// CHECK-MISC-NEXT: public struct Hello<T> where T : ~Copyable {
68+
69+
// CHECK-MISC: #if compiler(>=5.3) && $NoncopyableGenerics
70+
// CHECK-MISC-NEXT: extension {{.*}}.Hello : Swift.Copyable where T : Swift.Copyable {
71+
72+
////////////////////////////////////////////////////////////////////////
73+
// At the end, ensure there are no synthesized Copyable extensions
2774

28-
// CHECK-MISC: extension NoncopyableGenerics_Misc._Toys : Swift.Copyable {}
29-
// CHECK-MISC: extension NoncopyableGenerics_Misc._Toys.rdar118697289_S1 : Swift.Copyable {}
30-
// CHECK-MISC: extension NoncopyableGenerics_Misc._Toys.rdar118697289_S2 : Swift.Copyable {}
75+
// CHECK-MISC-NOT: extension {{.*}}_Toys : Swift.Copyable
3176

3277
import Swiftskell
3378

3479
// CHECK: #if compiler(>=5.3) && $NoncopyableGenerics
35-
// CHECK-NEXT: public protocol Show {
80+
// CHECK-NEXT: public protocol Show : ~Copyable {
3681

3782
// CHECK: #if compiler(>=5.3) && $NoncopyableGenerics
3883
// CHECK-NEXT: public func print(_ s: borrowing some Show & ~Copyable)
3984

4085
// CHECK: #if compiler(>=5.3) && $NoncopyableGenerics
41-
// CHECK-NEXT: public protocol Eq {
86+
// CHECK-NEXT: public protocol Eq : ~Copyable {
4287

4388
// CHECK: #if compiler(>=5.3) && $NoncopyableGenerics
4489
// CHECK-NEXT: extension Swiftskell.Eq {
@@ -47,10 +92,10 @@ import Swiftskell
4792
// CHECK-NEXT: extension Swiftskell.Eq where Self : Swift.Equatable {
4893

4994
// CHECK: #if compiler(>=5.3) && $NoncopyableGenerics
50-
// CHECK-NEXT: public struct Vector<T> {
95+
// CHECK-NEXT: public struct Vector<T> where T : ~Copyable {
5196

5297
// CHECK: #if compiler(>=5.3) && $NoncopyableGenerics
53-
// CHECK-NEXT: public enum Maybe<Value> {
98+
// CHECK-NEXT: public enum Maybe<Value> where Value : ~Copyable {
5499

55100
// CHECK: #if compiler(>=5.3) && $NoncopyableGenerics
56101
// CHECK-NEXT: extension Swiftskell.Maybe : Swiftskell.Show {
@@ -59,13 +104,13 @@ import Swiftskell
59104
// CHECK-NEXT: extension Swiftskell.Maybe : Swiftskell.Eq where Value : Swiftskell.Eq {
60105

61106
// CHECK: #if compiler(>=5.3) && $NoncopyableGenerics
62-
// CHECK-NEXT: public func maybe<A, B>(_ defaultVal: B, _ fn: (consuming A) -> B) -> (consuming Swiftskell.Maybe<A>) -> B where B : Swift.Copyable
107+
// CHECK-NEXT: public func maybe<A, B>(_ defaultVal: B, _ fn: (consuming A) -> B) -> (consuming Swiftskell.Maybe<A>) -> B where A : ~Copyable
63108

64109
// CHECK: #if compiler(>=5.3) && $NoncopyableGenerics
65-
// CHECK-NEXT: @inlinable public func fromMaybe<A>(_ defaultVal: A) -> (Swiftskell.Maybe<A>) -> A where A : Swift.Copyable {
110+
// CHECK-NEXT: @inlinable public func fromMaybe<A>(_ defaultVal: A) -> (Swiftskell.Maybe<A>) -> A {
66111

67112
// CHECK: #if compiler(>=5.3) && $NoncopyableGenerics
68-
// CHECK-NEXT: public func isJust<A>(_ m: borrowing Swiftskell.Maybe<A>) -> Swift.Bool
113+
// CHECK-NEXT: public func isJust<A>(_ m: borrowing Swiftskell.Maybe<A>) -> Swift.Bool where A : ~Copyable
69114

70115

71116
struct FileDescriptor: ~Copyable, Eq, Show {

0 commit comments

Comments
 (0)