Skip to content

Commit a1e14ae

Browse files
committed
Sema: ext's must add solo invertible conformances
This helps prevent confusion after not inferring requirements if the extension adds a Copyable conformance.
1 parent 2893e3d commit a1e14ae

File tree

6 files changed

+82
-16
lines changed

6 files changed

+82
-16
lines changed

Diff for: include/swift/AST/DiagnosticsSema.def

+3
Original file line numberDiff line numberDiff line change
@@ -7799,6 +7799,9 @@ ERROR(suppress_nonsuppressable_protocol,none,
77997799
"conformance to %0 cannot be suppressed", (const ProtocolDecl *))
78007800
WARNING(suppress_already_suppressed_protocol,none,
78017801
"already suppressed conformance to %0", (const ProtocolDecl *))
7802+
ERROR(extension_conforms_to_invertible_and_others, none,
7803+
"conformance to '%0' must be declared in a separate extension",
7804+
(StringRef))
78027805

78037806
// -- older ones below --
78047807
ERROR(noncopyable_parameter_requires_ownership, none,

Diff for: lib/AST/ASTPrinter.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -2922,6 +2922,7 @@ static bool isExtensionAddingInvertibleConformance(const ExtensionDecl *ext) {
29222922
auto conformances = ext->getLocalConformances();
29232923
for (auto *conf : conformances) {
29242924
if (conf->getProtocol()->getInvertibleProtocolKind()) {
2925+
assert(conformances.size() == 1 && "expected solo conformance");
29252926
return true;
29262927
}
29272928
}

Diff for: lib/Frontend/ModuleInterfaceSupport.cpp

+30-15
Original file line numberDiff line numberDiff line change
@@ -801,7 +801,7 @@ class InheritedProtocolCollector {
801801
}
802802

803803
/// If there were any conditional conformances that couldn't be printed,
804-
/// make a dummy extension that conforms to all of them, constrained by a
804+
/// make dummy extension(s) that conforms to all of them, constrained by a
805805
/// fake protocol.
806806
bool printInaccessibleConformanceExtensionIfNeeded(
807807
raw_ostream &out, const PrintOptions &printOptions,
@@ -810,20 +810,35 @@ class InheritedProtocolCollector {
810810
return false;
811811
assert(nominal->isGenericContext());
812812

813-
if (!printOptions.printPublicInterface())
814-
out << "@_spi(" << DummyProtocolName << ")\n";
815-
out << "@available(*, unavailable)\nextension ";
816-
nominal->getDeclaredType().print(out, printOptions);
817-
out << " : ";
818-
llvm::interleave(
819-
ConditionalConformanceProtocols,
820-
[&out, &printOptions](const ProtocolType *protoTy) {
821-
protoTy->print(out, printOptions);
822-
},
823-
[&out] { out << ", "; });
824-
out << " where "
825-
<< nominal->getGenericSignature().getGenericParams().front()->getName()
826-
<< " : " << DummyProtocolName << " {}\n";
813+
auto emitExtension =
814+
[&](ArrayRef<const ProtocolType *> conformanceProtos) {
815+
if (!printOptions.printPublicInterface())
816+
out << "@_spi(" << DummyProtocolName << ")\n";
817+
out << "@available(*, unavailable)\nextension ";
818+
nominal->getDeclaredType().print(out, printOptions);
819+
out << " : ";
820+
llvm::interleave(
821+
conformanceProtos,
822+
[&out, &printOptions](const ProtocolType *protoTy) {
823+
protoTy->print(out, printOptions);
824+
},
825+
[&out] { out << ", "; });
826+
out << " where "
827+
<< nominal->getGenericSignature().getGenericParams()[0]->getName()
828+
<< " : " << DummyProtocolName << " {}\n";
829+
};
830+
831+
// We have to print conformances for invertible protocols in separate
832+
// extensions, so do those first and save the rest for one extension.
833+
SmallVector<const ProtocolType *, 8> regulars;
834+
for (auto *proto : ConditionalConformanceProtocols) {
835+
if (proto->getDecl()->getInvertibleProtocolKind()) {
836+
emitExtension(proto);
837+
continue;
838+
}
839+
regulars.push_back(proto);
840+
}
841+
emitExtension(regulars);
827842
return true;
828843
}
829844

Diff for: lib/Sema/TypeCheckDeclPrimary.cpp

+24
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,28 @@ class CheckRepressions {
154154
}
155155
};
156156

157+
/// If the extension adds a conformance to an invertible protocol, ensure that
158+
/// it does not add a conformance to any other protocol. So these are illegal:
159+
///
160+
/// extension S: Copyable & P {}
161+
/// extension S: Q, Copyable {}
162+
///
163+
/// This policy is in place because extensions adding a conformance to an
164+
/// invertible protocol do _not_ add default requirements on generic parameters,
165+
/// so it would be confusing to mix them together in the same extension.
166+
static void checkExtensionAddsSoloInvertibleProtocol(const ExtensionDecl *ext) {
167+
auto localConfs = ext->getLocalConformances();
168+
if (localConfs.size() <= 1)
169+
return;
170+
171+
for (auto *conf : localConfs) {
172+
if (auto ip = conf->getProtocol()->getInvertibleProtocolKind()) {
173+
ext->diagnose(diag::extension_conforms_to_invertible_and_others,
174+
getInvertibleProtocolKindName(*ip));
175+
}
176+
}
177+
}
178+
157179
/// Check the inheritance clause of a type declaration or extension thereof.
158180
///
159181
/// This routine performs detailed checking of the inheritance clause of the
@@ -3979,6 +4001,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
39794001
diagnoseExtensionOfMarkerProtocol(ED);
39804002

39814003
checkTupleExtension(ED);
4004+
4005+
checkExtensionAddsSoloInvertibleProtocol(ED);
39824006
}
39834007

39844008
void visitTopLevelCodeDecl(TopLevelCodeDecl *TLCD) {

Diff for: test/Parse/inverses.swift

+17
Original file line numberDiff line numberDiff line change
@@ -129,3 +129,20 @@ protocol Q: ~Copyable {}
129129
protocol R: ~Copyable {}
130130
struct Blooper<T: ~Copyable>: ~Copyable {}
131131
extension Blooper: (Q & (R & (~Copyable & P))) {} // expected-error {{cannot suppress 'Copyable' in extension}}
132+
133+
protocol Edible {}
134+
protocol Portable {}
135+
typealias Alias = Portable & Copyable
136+
137+
struct Burrito<Filling: ~Copyable>: ~Copyable {}
138+
extension Burrito: Alias {} // expected-error {{conformance to 'Copyable' must be declared in a separate extension}}
139+
// expected-note@-1 {{'Burrito<Filling>' declares conformance to protocol 'Copyable' here}}
140+
141+
extension Burrito: Copyable & Edible & P {} // expected-error {{redundant conformance of 'Burrito<Filling>' to protocol 'Copyable'}}
142+
143+
struct Blah<T: ~Copyable>: ~Copyable {}
144+
extension Blah: P, Q, Copyable, R {} // expected-error {{generic struct 'Blah' required to be 'Copyable' but is marked with '~Copyable'}}
145+
// expected-error@-1 {{conformance to 'Copyable' must be declared in a separate extension}}
146+
147+
enum Hello<Gesture: ~Copyable>: ~Copyable {}
148+
extension Hello: Copyable & Edible & P {} // expected-error {{conformance to 'Copyable' must be declared in a separate extension}}

Diff for: validation-test/ParseableInterface/rdar128577611.swift

+7-1
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,10 @@ struct InternalStruct {}
77
extension [Int: InternalStruct]: Sendable {}
88

99
// CHECK: @available(*, unavailable)
10-
// CHECK: extension Swift.Dictionary : Swift.Copyable, Swift.Escapable, Swift.Sendable where Key : _ConstraintThatIsNotPartOfTheAPIOfThisLibrary {}
10+
// CHECK-NEXT: extension Swift.Dictionary : Swift.Copyable where Key : _ConstraintThatIsNotPartOfTheAPIOfThisLibrary {}
11+
12+
// CHECK: @available(*, unavailable)
13+
// CHECK-NEXT: extension Swift.Dictionary : Swift.Escapable where Key : _ConstraintThatIsNotPartOfTheAPIOfThisLibrary {}
14+
15+
// CHECK: @available(*, unavailable)
16+
// CHECK-NEXT: extension Swift.Dictionary : Swift.Sendable where Key : _ConstraintThatIsNotPartOfTheAPIOfThisLibrary {}

0 commit comments

Comments
 (0)