Skip to content

Commit 1198489

Browse files
kavonrxwei
authored andcommitted
disallow effectful getters for borrowed reads
This means properties returning noncopyable types or those marked with `@_borrowed` can't have effects. The compiler never accepted the latter correctly before. The synthesized coroutine `read` calls the user-defined `get`, but coroutines don't support `async` or `throws`. resolves rdar://106260787
1 parent aa365c5 commit 1198489

File tree

5 files changed

+68
-6
lines changed

5 files changed

+68
-6
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5819,6 +5819,9 @@ ERROR(borrowed_with_objc_dynamic,none,
58195819
ERROR(borrowed_on_objc_protocol_requirement,none,
58205820
"%0 cannot be '@_borrowed' if it is an @objc protocol requirement",
58215821
(DescriptiveDeclKind))
5822+
ERROR(borrowed_with_effect,none,
5823+
"%0 cannot be '@_borrowed' if it is 'async' or 'throws'",
5824+
(DescriptiveDeclKind))
58225825

58235826
//------------------------------------------------------------------------------
58245827
// MARK: dynamic
@@ -6742,6 +6745,8 @@ ERROR(noimplicitcopy_attr_valid_only_on_local_let_params,
67426745
ERROR(noimplicitcopy_attr_invalid_in_generic_context,
67436746
none, "'@_noImplicitCopy' attribute cannot be applied to entities in generic contexts", ())
67446747
ERROR(moveonly_generics, none, "move-only type %0 cannot be used with generics yet", (Type))
6748+
ERROR(moveonly_effectful_getter,none,
6749+
"%0 of move-only type cannot be 'async' or 'throws'", (DescriptiveDeclKind))
67456750
ERROR(noimplicitcopy_attr_not_allowed_on_moveonlytype,none,
67466751
"'@_noImplicitCopy' has no effect when applied to a move only type", ())
67476752
ERROR(moveonly_enums_do_not_support_indirect,none,

lib/Sema/TypeCheckStorage.cpp

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -610,14 +610,47 @@ IsSetterMutatingRequest::evaluate(Evaluator &evaluator,
610610
llvm_unreachable("bad storage kind");
611611
}
612612

613+
/*
614+
// An accessor that uses borrowed ownership cannot have effects, as the
615+
// coroutine accessor doesn't support `async` or `throws`
616+
if (auto accessor = dyn_cast<AccessorDecl>(FD)) {
617+
if (accessor->hasThrows() || accessor->hasAsync())
618+
if (accessor->)
619+
if (accessor->getResultInterfaceType()->isPureMoveOnly())
620+
accessor->diagnose(diag::moveonly_effectful_getter);
621+
}
622+
*/
623+
613624
OpaqueReadOwnership
614625
OpaqueReadOwnershipRequest::evaluate(Evaluator &evaluator,
615626
AbstractStorageDecl *storage) const {
616-
if (storage->getAttrs().hasAttribute<BorrowedAttr>())
627+
enum class DiagKind {
628+
BorrowedAttr,
629+
NoncopyableType
630+
};
631+
632+
auto usesBorrowed = [&](DiagKind kind) -> OpaqueReadOwnership {
633+
// Check for effects on the getter.
634+
if (auto *getter = storage->getEffectfulGetAccessor()) {
635+
switch (kind) {
636+
case DiagKind::NoncopyableType:
637+
getter->diagnose(diag::moveonly_effectful_getter,
638+
getter->getDescriptiveKind());
639+
break;
640+
case DiagKind::BorrowedAttr:
641+
getter->diagnose(diag::borrowed_with_effect,
642+
getter->getDescriptiveKind());
643+
break;
644+
}
645+
}
617646
return OpaqueReadOwnership::Borrowed;
647+
};
648+
649+
if (storage->getAttrs().hasAttribute<BorrowedAttr>())
650+
return usesBorrowed(DiagKind::BorrowedAttr);
618651

619652
if (storage->getValueInterfaceType()->isPureMoveOnly())
620-
return OpaqueReadOwnership::Borrowed;
653+
return usesBorrowed(DiagKind::NoncopyableType);
621654

622655
return OpaqueReadOwnership::Owned;
623656
}

test/Sema/forget.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,12 +135,11 @@ enum E: Error { case err }
135135
}
136136

137137
var validFile: File {
138-
__consuming get throws {
138+
__consuming get {
139139
if case let .valid(f) = self {
140140
return f
141141
}
142142
_forget self
143-
throw E.err
144143
}
145144
}
146145

test/Sema/moveonly_restrictions.swift

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
// RUN: %target-typecheck-verify-swift -enable-experimental-move-only -enable-experimental-feature MoveOnlyClasses
1+
// RUN: %target-typecheck-verify-swift -disable-availability-checking -enable-experimental-move-only -enable-experimental-feature MoveOnlyClasses
2+
3+
// REQUIRES: concurrency
24

35
class CopyableKlass {}
46

@@ -226,6 +228,19 @@ enum StrengthLevel: Int { // ensure move-only raw enums do not conform to RawRep
226228
}
227229
}
228230

231+
public class Holder {
232+
var one: MoveOnlyStruct {
233+
get async { } // expected-error {{getter of move-only type cannot be 'async' or 'throws'}}
234+
}
235+
var two: MoveOnlyKlass {
236+
get throws { } // expected-error {{getter of move-only type cannot be 'async' or 'throws'}}
237+
}
238+
}
229239

240+
struct StructHolder {
241+
var three: EMoveOnly {
242+
get async throws { } // expected-error {{getter of move-only type cannot be 'async' or 'throws'}}
243+
}
244+
}
230245

231246

test/attr/attr_borrowed.swift

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
// RUN: %target-typecheck-verify-swift
1+
// RUN: %target-typecheck-verify-swift -disable-availability-checking
22
// REQUIRES: objc_interop
3+
// REQUIRES: concurrency
34

45
import Foundation
56

@@ -18,3 +19,12 @@ var string = ""
1819
@_borrowed // expected-error {{property cannot be '@_borrowed' if it is '@objc dynamic'}}
1920
@objc dynamic var title: String { return "" }
2021
}
22+
23+
public class Holder {
24+
@_borrowed var one: String {
25+
get async { "" } // expected-error {{getter cannot be '@_borrowed' if it is 'async' or 'throws'}}
26+
}
27+
@_borrowed var two: String {
28+
get throws { "" } // expected-error {{getter cannot be '@_borrowed' if it is 'async' or 'throws'}}
29+
}
30+
}

0 commit comments

Comments
 (0)