Skip to content

Commit 85340ea

Browse files
committed
[HasNoncopyableAnnotationRequest] handle where
Adds rudiementary support for searching `where` clauses for `~Copyable` constraints, but the implementation is known to have flaws.
1 parent 3d89647 commit 85340ea

File tree

5 files changed

+100
-23
lines changed

5 files changed

+100
-23
lines changed

lib/Sema/TypeCheckDecl.cpp

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -942,21 +942,6 @@ bool HasNoncopyableAnnotationRequest::evaluate(Evaluator &evaluator, TypeDecl *d
942942

943943
assert(isa<NominalTypeDecl>(decl) && "todo: handle non-nominals");
944944

945-
// For protocols, inspect its generic signature for a Copyable requirement
946-
if (auto *proto = dyn_cast<ProtocolDecl>(decl)) {
947-
auto *copyableProto = ctx.getProtocol(KnownProtocolKind::Copyable);
948-
if (!copyableProto)
949-
llvm_unreachable("missing Copyable protocol!");
950-
951-
auto sig = proto->getGenericSignature();
952-
auto requiresCopyable =
953-
sig->requiresProtocol(proto->getSelfInterfaceType(), copyableProto);
954-
955-
// TODO: report that something implied Copyable despite writing ~Copyable?
956-
957-
return !requiresCopyable;
958-
}
959-
960945
// Handle the legacy '@_moveOnly' attribute
961946
if (decl->getAttrs().hasAttribute<MoveOnlyAttr>()) {
962947
assert(isa<StructDecl>(decl) || isa<EnumDecl>(decl));
@@ -1008,6 +993,52 @@ bool HasNoncopyableAnnotationRequest::evaluate(Evaluator &evaluator, TypeDecl *d
1008993
foundInverse(repr->getLoc(), *kp);
1009994
}
1010995

996+
// Check the where-clause for a constraint Subject: ~Copyable where Subject
997+
// refers to the where-clause owner.
998+
999+
Type owner;
1000+
if (auto *proto = dyn_cast<ProtocolDecl>(decl)) {
1001+
owner = proto->getSelfInterfaceType();
1002+
} else if (auto *nom = dyn_cast<NominalTypeDecl>(decl)) {
1003+
owner = nom->getDeclaredInterfaceType();
1004+
}
1005+
// TODO(kavon): handle other TypeDecls
1006+
1007+
1008+
// Checks a requirement in a where-clause
1009+
auto checkRequirement = [&](const Requirement &req,
1010+
RequirementRepr *repr) -> bool {
1011+
if (req.getKind() != RequirementKind::Conformance)
1012+
return false;
1013+
1014+
auto constraintTyRepr = repr->getConstraintRepr();
1015+
if (!dyn_cast_or_null<InverseTypeRepr>(constraintTyRepr))
1016+
return false;
1017+
1018+
auto kp = req.getSecondType()->getKnownProtocol();
1019+
if (!kp)
1020+
return false; // diagnosed elsewhere.
1021+
1022+
auto subject = req.getFirstType();
1023+
if (!subject)
1024+
return false;
1025+
1026+
if (subject->getCanonicalType() == owner->getCanonicalType())
1027+
foundInverse(constraintTyRepr->getLoc(), *kp);
1028+
1029+
return false;
1030+
};
1031+
1032+
if (owner) {
1033+
if (auto *nominal = dyn_cast<NominalTypeDecl>(decl)) {
1034+
WhereClauseOwner(nominal).visitRequirements(TypeResolutionStage::Structural,
1035+
checkRequirement);
1036+
} else if (auto *assocTy = dyn_cast<AssociatedTypeDecl>(decl)) {
1037+
WhereClauseOwner(assocTy).visitRequirements(TypeResolutionStage::Structural,
1038+
checkRequirement);
1039+
}
1040+
}
1041+
10111042
return !defaults.contains(KnownProtocolKind::Copyable);
10121043
}
10131044

lib/Sema/TypeCheckDeclPrimary.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1783,11 +1783,13 @@ static void checkProtocolRefinementRequirements(ProtocolDecl *proto) {
17831783

17841784
if (EnabledNoncopyableGenerics) {
17851785
// For any protocol 'P', there is an implied requirement 'Self: Copyable',
1786-
// unless it was suppressed via `Self: ~Copyable`; so skip if present.
1786+
// unless it was suppressed via `Self: ~Copyable`. So if this suppression
1787+
// annotation exists yet Copyable was implied anyway, emit a diagnostic.
17871788
if (otherProto->isSpecificProtocol(KnownProtocolKind::Copyable))
1788-
continue;
1789+
if (!proto->isNoncopyable())
1790+
continue; // no ~Copyable annotation
17891791

1790-
// TODO: report that something implied Copyable despite writing ~Copyable?
1792+
// TODO(kavon): emit tailored error diagnostic to remove the ~Copyable
17911793
}
17921794

17931795
// GenericSignature::getRequiredProtocols() canonicalizes the protocol

test/Generics/inverse_copyable_generics.swift

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ extension Cond: Copyable where T: Copyable {} // expected-note {{requirement fro
88
public typealias AlwaysCopyable<T> = Cond<T>
99
public typealias CondAlias<T> = Cond<T> where T: ~Copyable
1010

11-
func checkCopyable<T>(_ t: T) {}
11+
func checkCopyable<T>(_ t: T) {} // expected-note {{generic parameter 'T' has an implicit Copyable requirement}}
1212

1313
func test<C, NC: ~Copyable>(_ a: Cond<C>, _ b: borrowing Cond<NC>) {
1414
checkCopyable(a)
@@ -29,3 +29,33 @@ protocol Removed: ~Copyable {
2929
protocol Plain {
3030
func requiresCopyableSelf(_ t: AlwaysCopyable<Self>)
3131
}
32+
33+
// FIXME: detection of ~Copyable in where-clauses is half-baked.
34+
protocol RemovedAgain where Self: ~Copyable { // expected-warning {{protocol 'RemovedAgain' should be declared to refine 'Copyable' due to a same-type constraint on 'Self'}}
35+
func requiresCopyableSelf(_ t: AlwaysCopyable<Self>)
36+
}
37+
38+
struct StructContainment<T: ~Copyable> {
39+
// expected-note@-1 {{consider removing '~Copyable' from generic parameter 'T' so it conforms to the 'Copyable' protocol}}
40+
// expected-note@-2 {{consider removing implicit 'Copyable' conformance from generic struct 'StructContainment'}}
41+
42+
var storage: T
43+
// expected-error@-1 {{stored property 'storage' of 'Copyable'-conforming generic struct 'StructContainment' has noncopyable type 'T'}}
44+
}
45+
46+
enum EnumContainment<T: ~Copyable> {
47+
// expected-note@-1{{consider removing '~Copyable' from generic parameter 'T' so it conforms to the 'Copyable' protocol}}
48+
// expected-note@-2{{consider removing implicit 'Copyable' conformance from generic enum 'EnumContainment'}}
49+
50+
case some(T) // expected-error {{associated value 'some' of 'Copyable'-conforming generic enum 'EnumContainment' has noncopyable type 'T'}}
51+
case other(Int)
52+
case none
53+
}
54+
55+
class ClassContainment<T: ~Copyable> {
56+
var storage: T
57+
init(_ t: consuming T) {
58+
storage = t
59+
checkCopyable(t) // expected-error {{noncopyable type 'T' cannot be substituted for copyable generic parameter 'T' in 'checkCopyable'}}
60+
}
61+
}

test/Generics/inverse_protocols_errors.swift

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
// REQUIRES: asserts
44

55
protocol RegularProto {}
6-
protocol NCProto: ~Copyable, RegularProto { // FIXME: diagnose the ~Copyable annotation when it's implied.
6+
protocol NCProto: ~Copyable, RegularProto {
7+
// expected-warning@-1 {{protocol 'NCProto' should be declared to refine 'Copyable' due to a same-type constraint on 'Self'}}
78
func checkIfCopyableSelf(_ s: Self)
89
}
910

@@ -40,3 +41,9 @@ struct NCThinger<T: ~Copyable>: ~Copyable, Hello {
4041
// expected-note@-3 {{add 'inout' for a mutable reference}}
4142
// expected-note@-4 {{add 'consuming' to take the value from the caller}}
4243
}
44+
45+
struct ExtraNoncopyStruct: ~Copyable, ~Copyable {}
46+
// expected-error@-1 {{duplicate inverse constraint}}
47+
// expected-note@-2 {{previous inverse constraint here}}
48+
49+
protocol ExtraNoncopyProto: ~Copyable, ~Copyable {}

test/Parse/inverses.swift

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,20 @@ struct S4: ~(Copyable & Equatable) {} // expected-error {{type 'Copyable & Equat
2222
func blah<T>(_ t: T) where T: ~Copyable,
2323
T: ~Hashable {} // expected-error@:31 {{type 'Hashable' is not invertible}}
2424

25-
func foo<T: ~Copyable>(x: T) {}
25+
func foo<T: ~Copyable>(x: borrowing T) {}
2626

2727
struct Buurap<T: ~Copyable> {}
2828

29-
protocol Foo where Self: ~Copyable {
29+
// expected-warning@+1 {{protocol 'Foo' should be declared to refine 'Copyable' due to a same-type constraint on 'Self'}}
30+
protocol Foo: ~Copyable // expected-note {{previous inverse constraint here}}
31+
where Self: ~Copyable { // expected-error {{duplicate inverse constraint}}
32+
3033
func test<T>(_ t: T) where T: ~Self // expected-error {{type 'Self' is not invertible}}
3134
}
3235

3336
protocol Sando { func make() }
3437

35-
class C: ~Copyable,
38+
class C: ~Copyable, // expected-error {{classes cannot be noncopyable}}
3639
~Sando // expected-error {{type 'Sando' is not invertible}}
3740
{}
3841

@@ -54,6 +57,10 @@ func greenBay<each T: ~Copyable>(_ r: repeat each T) {}
5457

5558
typealias Clone = Copyable
5659
func dup<D: ~Clone>(_ d: D) {}
60+
// expected-error@-1 {{noncopyable parameter must specify its ownership}}
61+
// expected-note@-2 {{add 'borrowing'}}
62+
// expected-note@-3 {{add 'inout'}}
63+
// expected-note@-4 {{add 'consuming'}}
5764

5865
func superb(_ thing: some ~Copyable, thing2: some ~Clone) {}
5966

0 commit comments

Comments
 (0)