Merge pull request #20403 from rjmccall/borrowed-attr-read-accessor

Add a @_borrowed attribute to force the use of an opaque read accessor

Author: rjmccall

include/swift/AST/Attr.def

@@ -381,6 +381,9 @@ SIMPLE_DECL_ATTR(_nonoverride, NonOverride,
 DECL_ATTR(_dynamicReplacement, DynamicReplacement,
   OnAbstractFunction | OnVar | OnSubscript | UserInaccessible,
   80)
+SIMPLE_DECL_ATTR(_borrowed, Borrowed,
+  OnVar | OnSubscript | UserInaccessible |
+  NotSerialized, 81)
 
 #undef TYPE_ATTR
 #undef DECL_ATTR_ALIAS

include/swift/AST/DiagnosticsSema.def

@@ -3866,6 +3866,16 @@ ERROR(nonobjc_not_allowed,none,
 #undef OBJC_DIAG_SELECT
 #undef OBJC_ATTR_SELECT
 
+//------------------------------------------------------------------------------
+// MARK: @_borrowed
+//------------------------------------------------------------------------------
+ERROR(borrowed_with_objc_dynamic,none,
+      "%0 cannot be '@_borrowed' if it is '@objc dynamic'",
+      (DescriptiveDeclKind))
+ERROR(borrowed_on_objc_protocol_requirement,none,
+      "%0 cannot be '@_borrowed' if it is an @objc protocol requirement",
+      (DescriptiveDeclKind))
+
 //------------------------------------------------------------------------------
 // MARK: dynamic
 //------------------------------------------------------------------------------

lib/SILGen/SILGenExpr.cpp

@@ -2617,6 +2617,14 @@ loadIndexValuesForKeyPathComponent(SILGenFunction &SGF, SILLocation loc,
   return indexValues;
 }
 
+static AccessorDecl *
+getRepresentativeAccessorForKeyPath(AbstractStorageDecl *storage) {
+  if (storage->requiresOpaqueGetter())
+    return storage->getGetter();
+  assert(storage->requiresOpaqueReadCoroutine());
+  return storage->getReadCoroutine();
+}
+
 static SILFunction *getOrCreateKeyPathGetter(SILGenModule &SGM,
                          SILLocation loc,
                          AbstractStorageDecl *property,
@@ -2629,16 +2637,16 @@ static SILFunction *getOrCreateKeyPathGetter(SILGenModule &SGM,
   // back to the declaration whose getter introduced the witness table
   // entry.
   if (isa<ProtocolDecl>(property->getDeclContext())) {
-    auto getter = property->getGetter();
-    if (!SILDeclRef::requiresNewWitnessTableEntry(getter)) {
+    auto accessor = getRepresentativeAccessorForKeyPath(property);
+    if (!SILDeclRef::requiresNewWitnessTableEntry(accessor)) {
       // Find the getter that does have a witness table entry.
-      auto wtableGetter =
-        cast<AccessorDecl>(SILDeclRef::getOverriddenWitnessTableEntry(getter));
+      auto wtableAccessor =
+        cast<AccessorDecl>(SILDeclRef::getOverriddenWitnessTableEntry(accessor));
 
       // Substitute the 'Self' type of the base protocol.
       subs = SILGenModule::mapSubstitutionsForWitnessOverride(
-              getter, wtableGetter, subs);
-      property = wtableGetter->getStorage();
+              accessor, wtableAccessor, subs);
+      property = wtableAccessor->getStorage();
     }
   }
 
@@ -3239,7 +3247,7 @@ getIdForKeyPathComponentComputedProperty(SILGenModule &SGM,
     // observed property into a stored property.
     strategy = strategy.getReadStrategy();
     if (strategy.getKind() != AccessStrategy::Storage ||
-        !storage->getGetter()) {
+        !getRepresentativeAccessorForKeyPath(storage)) {
       return getIdForKeyPathComponentComputedProperty(SGM, storage, strategy);
     }
     LLVM_FALLTHROUGH;
@@ -3250,12 +3258,13 @@ getIdForKeyPathComponentComputedProperty(SILGenModule &SGM,
     // TODO: If the getter has shared linkage (say it's synthesized for a
     // Clang-imported thing), we'll need some other sort of
     // stable identifier.
-    auto getterRef = SILDeclRef(storage->getGetter(), SILDeclRef::Kind::Func);
+    auto getterRef = SILDeclRef(getRepresentativeAccessorForKeyPath(storage),
+                                SILDeclRef::Kind::Func);
     return SGM.getFunction(getterRef, NotForDefinition);
   }
   case AccessStrategy::DispatchToAccessor: {
     // Identify the property by its vtable or wtable slot.
-    return SGM.getAccessorDeclRef(storage->getGetter());
+    return SGM.getAccessorDeclRef(getRepresentativeAccessorForKeyPath(storage));
   }
   case AccessStrategy::BehaviorStorage:
     llvm_unreachable("unpossible");
@@ -3336,7 +3345,8 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc,
         // Properties that only dispatch via ObjC lookup do not have nor need
         // property descriptors, since the selector identifies the storage.
         && (!storage->hasAnyAccessors()
-            || !getAccessorDeclRef(storage->getGetter()).isForeign);
+            || !getAccessorDeclRef(getRepresentativeAccessorForKeyPath(storage))
+                  .isForeign);
     };
 
   auto strategy = storage->getAccessStrategy(AccessSemantics::Ordinary,

lib/Sema/TypeCheckAttr.cpp

@@ -187,6 +187,32 @@ class AttributeEarlyChecker : public AttributeVisitor<AttributeEarlyChecker> {
       TC.diagnose(attr->getLocation(), diag::alignment_not_power_of_two);
   }
 
+  void visitBorrowedAttr(BorrowedAttr *attr) {
+    // These criteria are the same preconditions laid out by
+    // AbstractStorageDecl::requiresOpaqueModifyCoroutine().
+
+    assert(!D->hasClangNode() && "@_borrowed on imported declaration?");
+
+    if (D->getAttrs().hasAttribute<DynamicAttr>()) {
+      TC.diagnose(attr->getLocation(), diag::borrowed_with_objc_dynamic,
+                  D->getDescriptiveKind())
+        .fixItRemove(attr->getRange());
+      D->getAttrs().removeAttribute(attr);
+      return;
+    }
+
+    auto dc = D->getDeclContext();
+    auto protoDecl = dyn_cast<ProtocolDecl>(dc);
+    if (protoDecl && protoDecl->isObjC()) {
+      TC.diagnose(attr->getLocation(),
+                  diag::borrowed_on_objc_protocol_requirement,
+                  D->getDescriptiveKind())
+        .fixItRemove(attr->getRange());
+      D->getAttrs().removeAttribute(attr);
+      return;
+    }
+  }
+
   void visitTransparentAttr(TransparentAttr *attr);
   void visitMutationAttr(DeclAttribute *attr);
   void visitMutatingAttr(MutatingAttr *attr) { visitMutationAttr(attr); }
@@ -781,6 +807,7 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
     void visit##CLASS##Attr(CLASS##Attr *) {}
 
     IGNORED_ATTR(Alignment)
+    IGNORED_ATTR(Borrowed)
     IGNORED_ATTR(HasInitialValue)
     IGNORED_ATTR(ClangImporterSynthesizedType)
     IGNORED_ATTR(Consuming)

lib/Sema/TypeCheckDecl.cpp

@@ -2341,8 +2341,16 @@ static bool computeIsSetterMutating(TypeChecker &TC,
   llvm_unreachable("bad storage kind");
 }
 
+static bool shouldUseOpaqueReadAccessor(TypeChecker &TC,
+                                        AbstractStorageDecl *storage) {
+  return storage->getAttrs().hasAttribute<BorrowedAttr>();
+}
+
 static void validateAbstractStorageDecl(TypeChecker &TC,
                                         AbstractStorageDecl *storage) {
+  if (shouldUseOpaqueReadAccessor(TC, storage))
+    storage->setOpaqueReadOwnership(OpaqueReadOwnership::Borrowed);
+
   // isGetterMutating and isSetterMutating are part of the signature
   // of a storage declaration and need to be validated immediately.
   storage->setIsGetterMutating(computeIsGetterMutating(TC, storage));

lib/Sema/TypeCheckDeclOverride.cpp

@@ -1166,6 +1166,7 @@ namespace  {
 
     UNINTERESTING_ATTR(AccessControl)
     UNINTERESTING_ATTR(Alignment)
+    UNINTERESTING_ATTR(Borrowed)
     UNINTERESTING_ATTR(CDecl)
     UNINTERESTING_ATTR(Consuming)
     UNINTERESTING_ATTR(Dynamic)

test/SILGen/read_accessor.swift

@@ -161,3 +161,15 @@ struct TestKeyPath {
 // CHECK-NEXT:    [[RET:%.*]] = tuple ()
 // CHECK-NEXT:    return [[RET]] : $()
 // CHECK-LABEL: } // end sil function '$s13read_accessor11TestKeyPathV8readableSSvpACTK'
+
+//   Check that we emit a read coroutine but not a getter for this.
+//   This test assumes that we emit accessors in a particular order.
+// CHECK-LABEL: sil [transparent] @$s13read_accessor20TestBorrowedPropertyV14borrowedStringSSvpfi
+// CHECK-NOT:   sil [transparent] [serialized] @$s13read_accessor20TestBorrowedPropertyV14borrowedStringSSvg
+// CHECK:       sil [transparent] [serialized] @$s13read_accessor20TestBorrowedPropertyV14borrowedStringSSvr
+// CHECK-NOT:   sil [transparent] [serialized] @$s13read_accessor20TestBorrowedPropertyV14borrowedStringSSvg
+// CHECK-LABEL: sil [transparent] [serialized] @$s13read_accessor20TestBorrowedPropertyV14borrowedStringSSvs
+public struct TestBorrowedProperty {
+  @_borrowed
+  public var borrowedString = ""
+}

test/attr/attr_borrowed.swift

@@ -0,0 +1,20 @@
+// RUN: %target-typecheck-verify-swift
+// REQUIRES: objc_interop
+
+import Foundation
+
+@_borrowed // expected-error {{'@_borrowed' attribute cannot be applied to this declaration}}
+func foo() -> String {}
+
+@_borrowed
+var string = ""
+
+@objc protocol P {
+  @_borrowed // expected-error {{property cannot be '@_borrowed' if it is an @objc protocol requirement}}
+  var title: String { get }
+}
+
+@objc class A {
+  @_borrowed // expected-error {{property cannot be '@_borrowed' if it is '@objc dynamic'}}
+  @objc dynamic var title: String { return "" }
+}