Skip to content

Commit dfc8f0a

Browse files
committed
NCGenerics: restrict conditional Copyable reqs
It doesn't really make sense for a conditional conformance requirement for `Copyable` to depend on any other requirement other than other `Copyable` conformance requirements. resolves rdar://124967739
1 parent 3bc570f commit dfc8f0a

File tree

3 files changed

+73
-2
lines changed

3 files changed

+73
-2
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7644,6 +7644,9 @@ ERROR(inverse_extension, none,
76447644
ERROR(copyable_illegal_deinit, none,
76457645
"deinitializer cannot be declared in %kind0 that conforms to 'Copyable'",
76467646
(const ValueDecl *))
7647+
ERROR(inverse_cannot_be_conditional_on_requirement, none,
7648+
"conditional conformance to suppressible %kind0 cannot depend on '%1%select{:|:| ==|:}2 %3'",
7649+
(const ProtocolDecl *, StringRef, unsigned, StringRef))
76477650
ERROR(inverse_type_member_in_conforming_type,none,
76487651
"%select{stored property %2|associated value %2}1 of "
76497652
"'%4'-conforming %kind3 has non-%4 type %0",

lib/Sema/TypeCheckInvertible.cpp

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,45 @@ static void checkInvertibleConformanceCommon(DeclContext *dc,
140140
if (conformance.isConcrete()) {
141141
auto concrete = conformance.getConcrete();
142142
if (auto *normalConf = dyn_cast<NormalProtocolConformance>(concrete)) {
143-
hasUnconditionalConformance =
144-
normalConf->getConditionalRequirements().empty();
145143
conformanceLoc = normalConf->getLoc();
146144
assert(conformanceLoc);
145+
146+
auto condReqs = normalConf->getConditionalRequirements();
147+
hasUnconditionalConformance = condReqs.empty();
148+
auto *thisProto = normalConf->getProtocol();
149+
150+
// Ensure that conditional conformance to an invertible protocol IP only
151+
// depends conformance requirements involving IP.
152+
//
153+
// In theory, it could depend on any invertible protocol, but it may be
154+
// confusing if we permitted that and this simplifies the model a bit.
155+
for (auto req : condReqs) {
156+
std::optional<StringRef> illegalSecondTypeStr;
157+
158+
// If we are diagnosing, fill-in the second-type string of this req.
159+
switch (req.getKind()) {
160+
case RequirementKind::Layout:
161+
illegalSecondTypeStr = req.getLayoutConstraint().getString();
162+
break;
163+
case RequirementKind::Conformance:
164+
if (req.getProtocolDecl() == thisProto)
165+
break; // permitted, don't fill-in.
166+
LLVM_FALLTHROUGH;
167+
case RequirementKind::Superclass:
168+
case RequirementKind::SameType:
169+
case RequirementKind::SameShape:
170+
illegalSecondTypeStr = req.getSecondType().getString();
171+
break;
172+
}
173+
174+
if (illegalSecondTypeStr)
175+
ctx.Diags.diagnose(conformanceLoc,
176+
diag::inverse_cannot_be_conditional_on_requirement,
177+
thisProto,
178+
req.getFirstType().getString(),
179+
static_cast<unsigned>(req.getKind()),
180+
*illegalSecondTypeStr);
181+
}
147182
}
148183
}
149184
assert(!conformance.isPack() && "not handled");
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// RUN: %target-typecheck-verify-swift \
2+
// RUN: -enable-experimental-feature NoncopyableGenerics \
3+
// RUN: -enable-experimental-feature NonescapableTypes
4+
5+
protocol P {}
6+
protocol Q {}
7+
class DoggoClass {}
8+
9+
struct Blah<T: ~Copyable & ~Escapable>: ~Copyable, ~Escapable {}
10+
extension Blah: Copyable {} // expected-error {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'T: Escapable'}}
11+
extension Blah: Escapable {} // expected-error {{conditional conformance to suppressible protocol 'Escapable' cannot depend on 'T: Copyable'}}
12+
13+
struct Yuck<T: ~Copyable & ~Escapable>: ~Copyable, ~Escapable {}
14+
extension Yuck: Copyable where T: ~Escapable {}
15+
extension Yuck: Escapable where T: ~Copyable {}
16+
17+
struct TryConformance<Whatever: ~Copyable>: ~Copyable {}
18+
extension TryConformance: Copyable
19+
where Whatever: P, Whatever: Q, Whatever: Sendable {}
20+
// expected-error@-2 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever: P'}}
21+
// expected-error@-3 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever: Q'}}
22+
// expected-error@-4 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever: Sendable'}}
23+
24+
struct TrySameType<Whatever: ~Copyable>: ~Copyable {}
25+
extension TrySameType: Copyable
26+
where Whatever == Int {}
27+
// expected-error@-2 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever == Int'}}
28+
29+
struct TryClassAndLayoutConstraints<Whatever: ~Copyable, Heckin>: ~Copyable {}
30+
extension TryClassAndLayoutConstraints: Copyable
31+
where Heckin: DoggoClass, Whatever: AnyObject {}
32+
// expected-error@-2 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever: AnyObject'}}
33+
// expected-error@-3 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Heckin: DoggoClass'}}

0 commit comments

Comments
 (0)