Merge pull request #64124 from kavon/moveonly-no-effectful-accessor

disallow effectful getters for borrowed reads

Author: swift-ci

include/swift/AST/DiagnosticsSema.def

@@ -5819,6 +5819,9 @@ ERROR(borrowed_with_objc_dynamic,none,
 ERROR(borrowed_on_objc_protocol_requirement,none,
       "%0 cannot be '@_borrowed' if it is an @objc protocol requirement",
       (DescriptiveDeclKind))
+ERROR(borrowed_with_effect,none,
+      "%0 cannot be '@_borrowed' if it is 'async' or 'throws'",
+      (DescriptiveDeclKind))
 
 //------------------------------------------------------------------------------
 // MARK: dynamic
@@ -6742,6 +6745,8 @@ ERROR(noimplicitcopy_attr_valid_only_on_local_let_params,
 ERROR(noimplicitcopy_attr_invalid_in_generic_context,
       none, "'@_noImplicitCopy' attribute cannot be applied to entities in generic contexts", ())
 ERROR(moveonly_generics, none, "move-only type %0 cannot be used with generics yet", (Type))
+ERROR(moveonly_effectful_getter,none,
+      "%0 of move-only type cannot be 'async' or 'throws'", (DescriptiveDeclKind))
 ERROR(noimplicitcopy_attr_not_allowed_on_moveonlytype,none,
       "'@_noImplicitCopy' has no effect when applied to a move only type", ())
 ERROR(moveonly_enums_do_not_support_indirect,none,

lib/Sema/TypeCheckStorage.cpp

@@ -610,14 +610,47 @@ IsSetterMutatingRequest::evaluate(Evaluator &evaluator,
   llvm_unreachable("bad storage kind");
 }
 
+/*
+ // An accessor that uses borrowed ownership cannot have effects, as the
+   // coroutine accessor doesn't support `async` or `throws`
+   if (auto accessor = dyn_cast<AccessorDecl>(FD)) {
+     if (accessor->hasThrows() || accessor->hasAsync())
+       if (accessor->)
+       if (accessor->getResultInterfaceType()->isPureMoveOnly())
+         accessor->diagnose(diag::moveonly_effectful_getter);
+   }
+ */
+
 OpaqueReadOwnership
 OpaqueReadOwnershipRequest::evaluate(Evaluator &evaluator,
                                      AbstractStorageDecl *storage) const {
-  if (storage->getAttrs().hasAttribute<BorrowedAttr>())
+  enum class DiagKind {
+    BorrowedAttr,
+    NoncopyableType
+  };
+
+  auto usesBorrowed = [&](DiagKind kind) -> OpaqueReadOwnership {
+    // Check for effects on the getter.
+    if (auto *getter = storage->getEffectfulGetAccessor()) {
+      switch (kind) {
+      case DiagKind::NoncopyableType:
+        getter->diagnose(diag::moveonly_effectful_getter,
+                         getter->getDescriptiveKind());
+        break;
+      case DiagKind::BorrowedAttr:
+        getter->diagnose(diag::borrowed_with_effect,
+                         getter->getDescriptiveKind());
+        break;
+      }
+    }
     return OpaqueReadOwnership::Borrowed;
+  };
+
+  if (storage->getAttrs().hasAttribute<BorrowedAttr>())
+    return usesBorrowed(DiagKind::BorrowedAttr);
 
   if (storage->getValueInterfaceType()->isPureMoveOnly())
-    return OpaqueReadOwnership::Borrowed;
+    return usesBorrowed(DiagKind::NoncopyableType);
 
   return OpaqueReadOwnership::Owned;
 }

test/Sema/forget.swift

@@ -135,12 +135,11 @@ enum E: Error { case err }
   }
 
   var validFile: File {
-    __consuming get throws {
+    __consuming get {
       if case let .valid(f) = self {
         return f
       }
       _forget self
-      throw E.err
     }
   }
 

test/Sema/moveonly_restrictions.swift

@@ -1,4 +1,6 @@
-// RUN: %target-typecheck-verify-swift -enable-experimental-move-only -enable-experimental-feature MoveOnlyClasses
+// RUN: %target-typecheck-verify-swift -disable-availability-checking -enable-experimental-move-only -enable-experimental-feature MoveOnlyClasses
+
+// REQUIRES: concurrency
 
 class CopyableKlass {}
 
@@ -226,6 +228,19 @@ enum StrengthLevel: Int { // ensure move-only raw enums do not conform to RawRep
     }
 }
 
+public class Holder {
+    var one: MoveOnlyStruct {
+        get async {  } // expected-error {{getter of move-only type cannot be 'async' or 'throws'}}
+    }
+    var two: MoveOnlyKlass {
+        get throws {  } // expected-error {{getter of move-only type cannot be 'async' or 'throws'}}
+    }
+}
 
+struct StructHolder {
+    var three: EMoveOnly {
+        get async throws {  } // expected-error {{getter of move-only type cannot be 'async' or 'throws'}}
+    }
+}
 
 

test/attr/attr_borrowed.swift

@@ -1,5 +1,6 @@
-// RUN: %target-typecheck-verify-swift
+// RUN: %target-typecheck-verify-swift -disable-availability-checking
 // REQUIRES: objc_interop
+// REQUIRES: concurrency
 
 import Foundation
 
@@ -18,3 +19,12 @@ var string = ""
   @_borrowed // expected-error {{property cannot be '@_borrowed' if it is '@objc dynamic'}}
   @objc dynamic var title: String { return "" }
 }
+
+public class Holder {
+  @_borrowed var one: String {
+    get async { "" } // expected-error {{getter cannot be '@_borrowed' if it is 'async' or 'throws'}}
+  }
+  @_borrowed var two: String {
+    get throws { "" } // expected-error {{getter cannot be '@_borrowed' if it is 'async' or 'throws'}}
+  }
+}