Skip to content

Commit 0b93ff4

Browse files
kavonkavon
authored
Merge pull request #65570 from kavon/ownership-specifiers-noncopyable-only
temporarily prevent Copyable types from using `consuming` and `borrowing`
2 parents 53e5723 + 87f190b commit 0b93ff4

16 files changed

+119
-24
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7123,6 +7123,11 @@ ERROR(moveonly_parameter_missing_ownership, none,
71237123
"noncopyable parameter must specify its ownership", ())
71247124
NOTE(moveonly_parameter_ownership_suggestion, none,
71257125
"add '%0' %1", (StringRef, StringRef))
7126+
ERROR(ownership_specifier_copyable,none,
7127+
"Copyable types cannot be 'consuming' or 'borrowing' yet", ())
7128+
ERROR(self_ownership_specifier_copyable,none,
7129+
"%0 is not yet valid on %1s in a Copyable type",
7130+
(SelfAccessKind, DescriptiveDeclKind))
71267131

71277132
//------------------------------------------------------------------------------
71287133
// MARK: Runtime discoverable attributes (@runtimeMetadata)

‎lib/Sema/TypeCheckAttr.cpp

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ void AttributeChecker::visitMutationAttr(DeclAttribute *attr) {
466466
}
467467

468468
auto DC = FD->getDeclContext();
469-
// mutation attributes may only appear in type context.
469+
// self-ownership attributes may only appear in type context.
470470
if (auto contextTy = DC->getDeclaredInterfaceType()) {
471471
// 'mutating' and 'nonmutating' are not valid on types
472472
// with reference semantics.
@@ -487,6 +487,27 @@ void AttributeChecker::visitMutationAttr(DeclAttribute *attr) {
487487
break;
488488
}
489489
}
490+
491+
// Unless we have the experimental no-implicit-copy feature enabled, Copyable
492+
// types can't use 'consuming' or 'borrowing' ownership specifiers.
493+
if (!Ctx.LangOpts.hasFeature(Feature::NoImplicitCopy)) {
494+
if (!contextTy->isPureMoveOnly()) {
495+
switch (attrModifier) { // check the modifier for the Copyable type.
496+
case SelfAccessKind::NonMutating:
497+
case SelfAccessKind::Mutating:
498+
case SelfAccessKind::LegacyConsuming:
499+
// already checked
500+
break;
501+
502+
case SelfAccessKind::Consuming:
503+
case SelfAccessKind::Borrowing:
504+
diagnoseAndRemoveAttr(attr, diag::self_ownership_specifier_copyable,
505+
attrModifier, FD->getDescriptiveKind());
506+
break;
507+
}
508+
}
509+
}
510+
490511
} else {
491512
diagnoseAndRemoveAttr(attr, diag::mutating_invalid_global_scope,
492513
attrModifier);

‎lib/Sema/TypeCheckType.cpp

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4280,7 +4280,33 @@ TypeResolver::resolveOwnershipTypeRepr(OwnershipTypeRepr *repr,
42804280
// Remember that we've seen an ownership specifier for this base type.
42814281
options |= TypeResolutionFlags::HasOwnership;
42824282

4283-
return resolveType(repr->getBase(), options);
4283+
auto result = resolveType(repr->getBase(), options);
4284+
if (result->hasError())
4285+
return result;
4286+
4287+
// Unless we have the experimental no-implicit-copy feature enabled, Copyable
4288+
// types can't use 'consuming' or 'borrowing' ownership specifiers.
4289+
if (!getASTContext().LangOpts.hasFeature(Feature::NoImplicitCopy)) {
4290+
if (!result->isPureMoveOnly()) {
4291+
// Prevent copyable types from using the non-underscored ownership parameter
4292+
// specifiers, other than 'inout'.
4293+
switch (ownershipRepr->getSpecifier()) {
4294+
case ParamSpecifier::Default:
4295+
case ParamSpecifier::InOut:
4296+
case ParamSpecifier::LegacyShared:
4297+
case ParamSpecifier::LegacyOwned:break;
4298+
4299+
case ParamSpecifier::Borrowing:
4300+
case ParamSpecifier::Consuming:
4301+
diagnoseInvalid(ownershipRepr,
4302+
ownershipRepr->getLoc(),
4303+
diag::ownership_specifier_copyable);
4304+
return ErrorType::get(getASTContext());
4305+
}
4306+
}
4307+
}
4308+
4309+
return result;
42844310
}
42854311

42864312
NeverNullType

‎test/Parse/ownership_modifiers.swift

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
struct borrowing {}
44
struct consuming {}
55

6-
struct Foo {}
6+
struct Foo: ~Copyable {}
77

88
func foo(x: borrowing Foo) {}
99
func bar(x: consuming Foo) {}
@@ -18,13 +18,13 @@ func worst(x: (borrowing consuming Foo) -> ()) {} // expected-error{{at most one
1818

1919
func zim(x: borrowing) {}
2020
func zang(x: consuming) {}
21-
func zung(x: borrowing consuming) {}
22-
func zip(x: consuming borrowing) {}
21+
func zung(x: borrowing consuming) {} // expected-error{{Copyable types cannot be 'consuming' or 'borrowing' yet}}
22+
func zip(x: consuming borrowing) {} // expected-error{{Copyable types cannot be 'consuming' or 'borrowing' yet}}
2323
func zap(x: (borrowing, consuming) -> ()) {}
24-
func zoop(x: (borrowing consuming, consuming borrowing) -> ()) {}
24+
func zoop(x: (borrowing consuming, consuming borrowing) -> ()) {} // expected-error 2{{Copyable types cannot be 'consuming' or 'borrowing' yet}}
2525

26-
func worster(x: borrowing borrowing borrowing) {} // expected-error{{at most one}}
27-
func worstest(x: (borrowing borrowing borrowing) -> ()) {} // expected-error{{at most one}}
26+
func worster(x: borrowing borrowing borrowing) {} // expected-error{{at most one}} // expected-error{{Copyable types cannot be 'consuming' or 'borrowing' yet}}
27+
func worstest(x: (borrowing borrowing borrowing) -> ()) {} // expected-error{{at most one}} // expected-error{{Copyable types cannot be 'consuming' or 'borrowing' yet}}
2828

2929
// Parameter specifier names are regular identifiers in other positions,
3030
// including argument labels.
@@ -47,7 +47,7 @@ func argumentLabel(anonConsumingInClosure: (_ consuming: Int) -> ()) {}
4747
func argumentLabel(anonSharedInClosure: (_ __shared: Int) -> ()) {}
4848
func argumentLabel(anonOwnedInClosure: (_ __owned: Int) -> ()) {}
4949

50-
struct MethodModifiers {
50+
struct MethodModifiers: ~Copyable {
5151
mutating func mutating() {}
5252
borrowing func borrowing() {}
5353
consuming func consuming() {}
@@ -59,3 +59,45 @@ struct MethodModifiers {
5959
borrowing consuming func tooManyC() {} // expected-error{{method must not be declared both 'borrowing' and 'consuming'}}
6060
borrowing mutating consuming func tooManyD() {} // expected-error 2 {{method must not be declared both }}
6161
}
62+
63+
64+
func chalk(_ a: consuming String, // expected-error{{Copyable types cannot be 'consuming' or 'borrowing' yet}}
65+
_ b: borrowing [Int], // expected-error{{Copyable types cannot be 'consuming' or 'borrowing' yet}}
66+
_ c: __shared [String],
67+
_ d: __owned Int?)
68+
{}
69+
70+
struct Stepping {
71+
consuming func perform() {} // expected-error {{'consuming' is not yet valid on instance methods in a Copyable type}}
72+
borrowing func doIt() {} // expected-error {{'borrowing' is not yet valid on instance methods in a Copyable type}}
73+
mutating func change() {}
74+
var ex: Int {
75+
__consuming get { 0 }
76+
}
77+
}
78+
79+
class Clapping {
80+
consuming func perform() {} // expected-error {{'consuming' is not yet valid on instance methods in a Copyable type}}
81+
borrowing func doIt() {} // expected-error {{'borrowing' is not yet valid on instance methods in a Copyable type}}
82+
var ex: Int {
83+
__consuming get { 0 }
84+
}
85+
}
86+
87+
protocol Popping {
88+
consuming func perform() // expected-error {{'consuming' is not yet valid on instance methods in a Copyable type}}
89+
borrowing func doIt() // expected-error {{'borrowing' is not yet valid on instance methods in a Copyable type}}
90+
mutating func change()
91+
var ex: Int {
92+
__consuming get
93+
}
94+
}
95+
96+
enum Exercising {
97+
consuming func perform() {} // expected-error {{'consuming' is not yet valid on instance methods in a Copyable type}}
98+
borrowing func doIt() {} // expected-error {{'borrowing' is not yet valid on instance methods in a Copyable type}}
99+
mutating func change() {}
100+
var ex: Int {
101+
__consuming get { 0 }
102+
}
103+
}

‎test/Parse/ownership_modifiers_no_errors.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-typecheck-verify-swift
1+
// RUN: %target-typecheck-verify-swift -enable-experimental-feature NoImplicitCopy
22

33
// This is a variation of `ownership_modifiers.swift` with the expected error
44
// lines removed, so that the file is parsed by the SwiftSyntax parser

‎test/SILGen/consuming_parameter.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-emit-silgen %s | %FileCheck %s
1+
// RUN: %target-swift-emit-silgen -enable-experimental-feature NoImplicitCopy %s | %FileCheck %s
22

33
func bar(_: String) {}
44

‎test/SILGen/moveonly.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-emit-silgen %s | %FileCheck %s
1+
// RUN: %target-swift-emit-silgen -enable-experimental-feature NoImplicitCopy %s | %FileCheck %s
22

33
//////////////////
44
// Declarations //

‎test/SILGen/moveonly_escaping_closure.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// RUN: %target-swift-emit-silgen -module-name moveonly_closure %s | %FileCheck %s
2-
// RUN: %target-swift-emit-sil -module-name moveonly_closure -verify %s
1+
// RUN: %target-swift-emit-silgen -enable-experimental-feature NoImplicitCopy -module-name moveonly_closure %s | %FileCheck %s
2+
// RUN: %target-swift-emit-sil -enable-experimental-feature NoImplicitCopy -module-name moveonly_closure -verify %s
33

44
@_moveOnly
55
struct Empty {}

‎test/SILGen/ownership_specifier_mangling.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-emit-silgen %s | %FileCheck %s
1+
// RUN: %target-swift-emit-silgen -enable-experimental-feature NoImplicitCopy %s | %FileCheck %s
22

33
// The internal `__shared` and `__owned` modifiers would always affect
44
// symbol mangling, even if they don't have a concrete impact on ABI. The

‎test/SILOptimizer/consuming_parameter.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -c -disable-availability-checking -Xllvm --sil-print-final-ossa-module -O -module-name=main -o /dev/null %s 2>&1 | %FileCheck %s
1+
// RUN: %target-swift-frontend -c -enable-experimental-feature NoImplicitCopy -disable-availability-checking -Xllvm --sil-print-final-ossa-module -O -module-name=main -o /dev/null %s 2>&1 | %FileCheck %s
22

33
// REQUIRES: concurrency
44

‎test/SILOptimizer/moveonly_addresschecker_destructure_through_deinit_diagnostics.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-emit-sil -sil-verify-all -verify -enable-experimental-feature MoveOnlyClasses -enable-experimental-feature MoveOnlyTuples %s
1+
// RUN: %target-swift-emit-sil -sil-verify-all -verify -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses -enable-experimental-feature MoveOnlyTuples %s
22

33
// This test validates that we properly emit errors if we partially invalidate
44
// through a type with a deinit.

‎test/SILOptimizer/moveonly_addresschecker_diagnostics.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-emit-sil -sil-verify-all -verify -enable-experimental-feature MoveOnlyClasses %s
1+
// RUN: %target-swift-emit-sil -sil-verify-all -verify -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s
22

33
//////////////////
44
// Declarations //

‎test/SILOptimizer/moveonly_objectchecker_diagnostics.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-emit-sil -sil-verify-all -verify -enable-experimental-feature MoveOnlyClasses %s
1+
// RUN: %target-swift-emit-sil -sil-verify-all -verify -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s
22

33
//////////////////
44
// Declarations //

‎test/SILOptimizer/moveonly_trivial_addresschecker_diagnostics.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-emit-sil -sil-verify-all -verify %s
1+
// RUN: %target-swift-emit-sil -enable-experimental-feature NoImplicitCopy -sil-verify-all -verify %s
22

33
//////////////////
44
// Declarations //

‎test/SILOptimizer/moveonly_trivial_objectchecker_diagnostics.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-emit-sil -sil-verify-all -verify %s
1+
// RUN: %target-swift-emit-sil -enable-experimental-feature NoImplicitCopy -sil-verify-all -verify %s
22

33
//////////////////
44
// Declarations //

‎test/attr/lexical.swift

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,14 @@ func foo() {
4444
_ = s3
4545
}
4646

47-
@_moveOnly struct MoveOnly {}
47+
struct MoveOnly: ~Copyable {}
4848

49-
@_eagerMove @_moveOnly struct MoveOnlyEagerly {} // expected-error {{@_eagerMove cannot be applied to NonCopyable types}}
49+
@_eagerMove struct MoveOnlyEagerly: ~Copyable {} // expected-error {{@_eagerMove cannot be applied to NonCopyable types}}
5050

5151
func zoo(@_eagerMove _ : consuming MoveOnly) {} // expected-error {{@_eagerMove cannot be applied to NonCopyable types}}
5252

53-
func zooo(@_noEagerMove _ : consuming C) {} // ok, only way to spell this behavior
53+
// TODO: Copyable types can't be consuming right now (rdar://108383660)
54+
//func zooo(@_noEagerMove _ : consuming C) {} // ok, only way to spell this behavior
5455

5556
extension MoveOnly {
5657
@_eagerMove // expected-error {{@_eagerMove cannot be applied to NonCopyable types}}

0 commit comments

Comments
 (0)