Skip to content

Commit e01573d

Browse files
committed
Add a @_borrowed attribute to force the use of an opaque read accessor.
This is strawman syntax pending a proposal; I pitched some ideas here: https://forums.swift.org/t/value-ownership-when-reading-from-a-storage-declaration/15076
1 parent 44b3a47 commit e01573d

File tree

8 files changed

+101
-10
lines changed

8 files changed

+101
-10
lines changed

include/swift/AST/Attr.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,9 @@ SIMPLE_DECL_ATTR(_nonoverride, NonOverride,
381381
DECL_ATTR(_dynamicReplacement, DynamicReplacement,
382382
OnAbstractFunction | OnVar | OnSubscript | UserInaccessible,
383383
80)
384+
SIMPLE_DECL_ATTR(_borrowed, Borrowed,
385+
OnVar | OnSubscript | UserInaccessible |
386+
NotSerialized, 81)
384387

385388
#undef TYPE_ATTR
386389
#undef DECL_ATTR_ALIAS

include/swift/AST/DiagnosticsSema.def

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3866,6 +3866,16 @@ ERROR(nonobjc_not_allowed,none,
38663866
#undef OBJC_DIAG_SELECT
38673867
#undef OBJC_ATTR_SELECT
38683868

3869+
//------------------------------------------------------------------------------
3870+
// MARK: @_borrowed
3871+
//------------------------------------------------------------------------------
3872+
ERROR(borrowed_with_objc_dynamic,none,
3873+
"%0 cannot be '@_borrowed' if it is '@objc dynamic'",
3874+
(DescriptiveDeclKind))
3875+
ERROR(borrowed_on_objc_protocol_requirement,none,
3876+
"%0 cannot be '@_borrowed' if it is an @objc protocol requirement",
3877+
(DescriptiveDeclKind))
3878+
38693879
//------------------------------------------------------------------------------
38703880
// MARK: dynamic
38713881
//------------------------------------------------------------------------------

lib/SILGen/SILGenExpr.cpp

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2617,6 +2617,14 @@ loadIndexValuesForKeyPathComponent(SILGenFunction &SGF, SILLocation loc,
26172617
return indexValues;
26182618
}
26192619

2620+
static AccessorDecl *
2621+
getRepresentativeAccessorForKeyPath(AbstractStorageDecl *storage) {
2622+
if (storage->requiresOpaqueGetter())
2623+
return storage->getGetter();
2624+
assert(storage->requiresOpaqueReadCoroutine());
2625+
return storage->getReadCoroutine();
2626+
}
2627+
26202628
static SILFunction *getOrCreateKeyPathGetter(SILGenModule &SGM,
26212629
SILLocation loc,
26222630
AbstractStorageDecl *property,
@@ -2629,16 +2637,16 @@ static SILFunction *getOrCreateKeyPathGetter(SILGenModule &SGM,
26292637
// back to the declaration whose getter introduced the witness table
26302638
// entry.
26312639
if (isa<ProtocolDecl>(property->getDeclContext())) {
2632-
auto getter = property->getGetter();
2633-
if (!SILDeclRef::requiresNewWitnessTableEntry(getter)) {
2640+
auto accessor = getRepresentativeAccessorForKeyPath(property);
2641+
if (!SILDeclRef::requiresNewWitnessTableEntry(accessor)) {
26342642
// Find the getter that does have a witness table entry.
2635-
auto wtableGetter =
2636-
cast<AccessorDecl>(SILDeclRef::getOverriddenWitnessTableEntry(getter));
2643+
auto wtableAccessor =
2644+
cast<AccessorDecl>(SILDeclRef::getOverriddenWitnessTableEntry(accessor));
26372645

26382646
// Substitute the 'Self' type of the base protocol.
26392647
subs = SILGenModule::mapSubstitutionsForWitnessOverride(
2640-
getter, wtableGetter, subs);
2641-
property = wtableGetter->getStorage();
2648+
accessor, wtableAccessor, subs);
2649+
property = wtableAccessor->getStorage();
26422650
}
26432651
}
26442652

@@ -3239,7 +3247,7 @@ getIdForKeyPathComponentComputedProperty(SILGenModule &SGM,
32393247
// observed property into a stored property.
32403248
strategy = strategy.getReadStrategy();
32413249
if (strategy.getKind() != AccessStrategy::Storage ||
3242-
!storage->getGetter()) {
3250+
!getRepresentativeAccessorForKeyPath(storage)) {
32433251
return getIdForKeyPathComponentComputedProperty(SGM, storage, strategy);
32443252
}
32453253
LLVM_FALLTHROUGH;
@@ -3250,12 +3258,13 @@ getIdForKeyPathComponentComputedProperty(SILGenModule &SGM,
32503258
// TODO: If the getter has shared linkage (say it's synthesized for a
32513259
// Clang-imported thing), we'll need some other sort of
32523260
// stable identifier.
3253-
auto getterRef = SILDeclRef(storage->getGetter(), SILDeclRef::Kind::Func);
3261+
auto getterRef = SILDeclRef(getRepresentativeAccessorForKeyPath(storage),
3262+
SILDeclRef::Kind::Func);
32543263
return SGM.getFunction(getterRef, NotForDefinition);
32553264
}
32563265
case AccessStrategy::DispatchToAccessor: {
32573266
// Identify the property by its vtable or wtable slot.
3258-
return SGM.getAccessorDeclRef(storage->getGetter());
3267+
return SGM.getAccessorDeclRef(getRepresentativeAccessorForKeyPath(storage));
32593268
}
32603269
case AccessStrategy::BehaviorStorage:
32613270
llvm_unreachable("unpossible");
@@ -3336,7 +3345,8 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc,
33363345
// Properties that only dispatch via ObjC lookup do not have nor need
33373346
// property descriptors, since the selector identifies the storage.
33383347
&& (!storage->hasAnyAccessors()
3339-
|| !getAccessorDeclRef(storage->getGetter()).isForeign);
3348+
|| !getAccessorDeclRef(getRepresentativeAccessorForKeyPath(storage))
3349+
.isForeign);
33403350
};
33413351

33423352
auto strategy = storage->getAccessStrategy(AccessSemantics::Ordinary,

lib/Sema/TypeCheckAttr.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,32 @@ class AttributeEarlyChecker : public AttributeVisitor<AttributeEarlyChecker> {
187187
TC.diagnose(attr->getLocation(), diag::alignment_not_power_of_two);
188188
}
189189

190+
void visitBorrowedAttr(BorrowedAttr *attr) {
191+
// These criteria are the same preconditions laid out by
192+
// AbstractStorageDecl::requiresOpaqueModifyCoroutine().
193+
194+
assert(!D->hasClangNode() && "@_borrowed on imported declaration?");
195+
196+
if (D->getAttrs().hasAttribute<DynamicAttr>()) {
197+
TC.diagnose(attr->getLocation(), diag::borrowed_with_objc_dynamic,
198+
D->getDescriptiveKind())
199+
.fixItRemove(attr->getRange());
200+
D->getAttrs().removeAttribute(attr);
201+
return;
202+
}
203+
204+
auto dc = D->getDeclContext();
205+
auto protoDecl = dyn_cast<ProtocolDecl>(dc);
206+
if (protoDecl && protoDecl->isObjC()) {
207+
TC.diagnose(attr->getLocation(),
208+
diag::borrowed_on_objc_protocol_requirement,
209+
D->getDescriptiveKind())
210+
.fixItRemove(attr->getRange());
211+
D->getAttrs().removeAttribute(attr);
212+
return;
213+
}
214+
}
215+
190216
void visitTransparentAttr(TransparentAttr *attr);
191217
void visitMutationAttr(DeclAttribute *attr);
192218
void visitMutatingAttr(MutatingAttr *attr) { visitMutationAttr(attr); }
@@ -781,6 +807,7 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
781807
void visit##CLASS##Attr(CLASS##Attr *) {}
782808

783809
IGNORED_ATTR(Alignment)
810+
IGNORED_ATTR(Borrowed)
784811
IGNORED_ATTR(HasInitialValue)
785812
IGNORED_ATTR(ClangImporterSynthesizedType)
786813
IGNORED_ATTR(Consuming)

lib/Sema/TypeCheckDecl.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2341,8 +2341,16 @@ static bool computeIsSetterMutating(TypeChecker &TC,
23412341
llvm_unreachable("bad storage kind");
23422342
}
23432343

2344+
static bool shouldUseOpaqueReadAccessor(TypeChecker &TC,
2345+
AbstractStorageDecl *storage) {
2346+
return storage->getAttrs().hasAttribute<BorrowedAttr>();
2347+
}
2348+
23442349
static void validateAbstractStorageDecl(TypeChecker &TC,
23452350
AbstractStorageDecl *storage) {
2351+
if (shouldUseOpaqueReadAccessor(TC, storage))
2352+
storage->setOpaqueReadOwnership(OpaqueReadOwnership::Borrowed);
2353+
23462354
// isGetterMutating and isSetterMutating are part of the signature
23472355
// of a storage declaration and need to be validated immediately.
23482356
storage->setIsGetterMutating(computeIsGetterMutating(TC, storage));

lib/Sema/TypeCheckDeclOverride.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,6 +1166,7 @@ namespace {
11661166

11671167
UNINTERESTING_ATTR(AccessControl)
11681168
UNINTERESTING_ATTR(Alignment)
1169+
UNINTERESTING_ATTR(Borrowed)
11691170
UNINTERESTING_ATTR(CDecl)
11701171
UNINTERESTING_ATTR(Consuming)
11711172
UNINTERESTING_ATTR(Dynamic)

test/SILGen/read_accessor.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,3 +161,15 @@ struct TestKeyPath {
161161
// CHECK-NEXT: [[RET:%.*]] = tuple ()
162162
// CHECK-NEXT: return [[RET]] : $()
163163
// CHECK-LABEL: } // end sil function '$s13read_accessor11TestKeyPathV8readableSSvpACTK'
164+
165+
// Check that we emit a read coroutine but not a getter for this.
166+
// This test assumes that we emit accessors in a particular order.
167+
// CHECK-LABEL: sil [transparent] @$s13read_accessor20TestBorrowedPropertyV14borrowedStringSSvpfi
168+
// CHECK-NOT: sil [transparent] [serialized] @$s13read_accessor20TestBorrowedPropertyV14borrowedStringSSvg
169+
// CHECK: sil [transparent] [serialized] @$s13read_accessor20TestBorrowedPropertyV14borrowedStringSSvr
170+
// CHECK-NOT: sil [transparent] [serialized] @$s13read_accessor20TestBorrowedPropertyV14borrowedStringSSvg
171+
// CHECK-LABEL: sil [transparent] [serialized] @$s13read_accessor20TestBorrowedPropertyV14borrowedStringSSvs
172+
public struct TestBorrowedProperty {
173+
@_borrowed
174+
public var borrowedString = ""
175+
}

test/attr/attr_borrowed.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// RUN: %target-typecheck-verify-swift
2+
// REQUIRES: objc_interop
3+
4+
import Foundation
5+
6+
@_borrowed // expected-error {{'@_borrowed' attribute cannot be applied to this declaration}}
7+
func foo() -> String {}
8+
9+
@_borrowed
10+
var string = ""
11+
12+
@objc protocol P {
13+
@_borrowed // expected-error {{property cannot be '@_borrowed' if it is an @objc protocol requirement}}
14+
var title: String { get }
15+
}
16+
17+
@objc class A {
18+
@_borrowed // expected-error {{property cannot be '@_borrowed' if it is '@objc dynamic'}}
19+
@objc dynamic var title: String { return "" }
20+
}

0 commit comments

Comments
 (0)