Minor fixes to borrow accessors

SwiftCompilerSources/Sources/Optimizer/Utilities/EscapeUtils.swift

@@ -725,7 +725,7 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
         }
       case .escapingToReturn(let toPath, let exclusive):
         if effect.matches(calleeArgIdx, argPath.projectionPath) {
-          guard let fas = apply as? FullApplySite, let result = fas.singleDirectResult else {
+          guard let fas = apply as? FullApplySite, let result = fas.singleDirectResult, result.type.isObject else {
             return isEscaping
           }
 

include/swift/AST/DiagnosticsCommon.def

@@ -280,8 +280,8 @@ ERROR(lookup_outputs_dont_match,none,
 // MARK: Accessor diagnostics
 //------------------------------------------------------------------------------
 
-ERROR(accessor_not_supported_in_decl,none,
-      "%0 is supported only on a struct or enum", (StringRef))      
+ERROR(borrow_mutate_accessor_not_supported_in_decl, none,
+      "%0 is supported only on a struct", (StringRef))      
 
 //------------------------------------------------------------------------------
 // MARK: bridged diagnostics

lib/Parse/ParseDecl.cpp

@@ -8131,9 +8131,8 @@ bool Parser::parseAccessorAfterIntroducer(
   }
 
   if (Kind == AccessorKind::Borrow || Kind == AccessorKind::Mutate) {
-    if (!Flags.contains(PD_InStruct) && !Flags.contains(PD_InEnum) &&
-        !Flags.contains(PD_InExtension)) {
-      diagnose(Tok, diag::accessor_not_supported_in_decl,
+    if (!Flags.contains(PD_InStruct) && !Flags.contains(PD_InExtension)) {
+      diagnose(Tok, diag::borrow_mutate_accessor_not_supported_in_decl,
                getAccessorNameForDiagnostic(Kind, /*article*/ true,
                                             /*underscored*/ false));
     }

lib/SILGen/SILGenApply.cpp

@@ -5457,9 +5457,20 @@ ManagedValue CallEmission::applyBorrowMutateAccessor() {
   // begin_borrow instructions added for move-only self argument.
   if (selfArgMV.getValue()->getType().isMoveOnly() &&
       selfArgMV.getValue()->getType().isObject()) {
-    uncurriedArgs[uncurriedArgs.size() - 1] =
-        ManagedValue::forBorrowedObjectRValue(
-            lookThroughMoveOnlyCheckerPattern(selfArgMV.getValue()));
+    uncurriedArgs.back() = ManagedValue::forBorrowedObjectRValue(
+        lookThroughMoveOnlyCheckerPattern(selfArgMV.getValue()));
+  }
+
+  if (fnValue.getFunction()->getConventions().hasGuaranteedResult()) {
+    if (isa<LoadBorrowInst>(selfArgMV)) {
+      // unchecked_ownership is used to silence the ownership verifier for
+      // returning a value produced within a load_borrow scope. SILGenCleanup
+      // eliminates it and introduces return_borrow appropriately.
+      uncurriedArgs.back() =
+          ManagedValue::forForwardedRValue(
+              SGF, SGF.B.createUncheckedOwnership(uncurriedLoc.value(),
+                                                  selfArgMV.getValue()));
+    }
   }
 
   auto value = SGF.applyBorrowMutateAccessor(
@@ -5475,22 +5486,13 @@ ManagedValue SILGenFunction::applyBorrowMutateAccessor(
     ApplyOptions options) {
   // Emit the call.
   SmallVector<SILValue, 4> rawResults;
+
   emitRawApply(*this, loc, fn, subs, args, substFnType, options,
                /*indirect results*/ {}, /*indirect errors*/ {}, rawResults,
                ExecutorBreadcrumb());
   assert(rawResults.size() == 1);
   auto rawResult = rawResults[0];
 
-  if (fn.getFunction()->getConventions().hasGuaranteedResult()) {
-    auto selfArg = args.back().getValue();
-    if (isa<LoadBorrowInst>(selfArg)) {
-      // unchecked_ownership is used to silence the ownership verifier for
-      // returning a value produced within a load_borrow scope. SILGenCleanup
-      // eliminates it and introduces return_borrow appropriately.
-      rawResult = B.createUncheckedOwnership(loc, rawResult);
-    }
-  }
-
   if (rawResult->getType().isMoveOnly()) {
     if (rawResult->getType().isAddress()) {
       SILFunctionConventions substFnConv(substFnType, SGM.M);

lib/Sema/TypeCheckStorage.cpp

@@ -4324,16 +4324,22 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator,
   if (borrow || mutate) {
     if (auto *extDecl = dyn_cast<ExtensionDecl>(DC)) {
       auto extNominal = extDecl->getExtendedNominal();
-      if (!isa<StructDecl>(extNominal) && !isa<EnumDecl>(extNominal)) {
+      if (!isa<StructDecl>(extNominal)) {
         if (borrow) {
           storage->getASTContext().Diags.diagnose(
-              borrow->getLoc(), diag::accessor_not_supported_in_decl,
-              "a borrow accessor");
+              borrow->getLoc(),
+              diag::borrow_mutate_accessor_not_supported_in_decl,
+              getAccessorNameForDiagnostic(borrow->getAccessorKind(),
+                                           /*article*/ true,
+                                           /*underscored*/ false));
         }
         if (mutate) {
           storage->getASTContext().Diags.diagnose(
-              mutate->getLoc(), diag::accessor_not_supported_in_decl,
-              "a mutate accessor");
+              mutate->getLoc(),
+              diag::borrow_mutate_accessor_not_supported_in_decl,
+              getAccessorNameForDiagnostic(mutate->getAccessorKind(),
+                                           /*article*/ true,
+                                           /*underscored*/ false));
         }
       }
     }

test/Parse/borrow_and_mutate_accessors.swift

@@ -80,31 +80,14 @@ struct Wrapper {
 var i: Int
 
 var i_accessor: Int {
-  borrow { // expected-error{{a 'borrow' accessor is supported only on a struct or enum}}
+  borrow { // expected-error{{a 'borrow' accessor is supported only on a struct}}
     fatalError()
   }
-  mutate { // expected-error{{a 'mutate' accessor is supported only on a struct or enum}}
+  mutate { // expected-error{{a 'mutate' accessor is supported only on a struct}}
     return &i // expected-error{{'&' may only be used to pass an argument to inout parameter}}
   }
 }
 
-var _count: Int = 0
-
-enum Color {
-  case red
-  case green
-  case blue
-
-  var count: Int {
-    borrow {
-      return _count
-    }
-    mutate {
-      return &_count
-    }
-  }
-}
-
 class KlassWrapper {
   var _k: Klass
 
@@ -113,10 +96,10 @@ class KlassWrapper {
   }
 
   var k: Klass {
-    borrow {// expected-error{{a 'borrow' accessor is supported only on a struct or enum}}
+    borrow {// expected-error{{a 'borrow' accessor is supported only on a struct}}
       return _k
     }
-    mutate {// expected-error{{a 'mutate' accessor is supported only on a struct or enum}}
+    mutate {// expected-error{{a 'mutate' accessor is supported only on a struct}}
       return &_k
     }
   }

test/SILGen/borrow_accessor.swift

@@ -1,4 +1,5 @@
 // RUN:%target-swift-frontend -emit-silgen %s -enable-experimental-feature BorrowAndMutateAccessors | %FileCheck %s
+// RUN:%target-swift-frontend -c %s -enable-experimental-feature BorrowAndMutateAccessors -Xllvm -sil-print-after=SILGenCleanup 2>&1 | %FileCheck %s --check-prefixes=CHECK-SIL
 
 // REQUIRES: swift_feature_BorrowAndMutateAccessors
 
@@ -496,6 +497,14 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
 // CHECK:   return [[REG4]]
 // CHECK: }
 
+// CHECK-SIL: sil hidden [ossa] @$s15borrow_accessor10GenWrapperV1sAA1SVvb : $@convention(method) <T> (@in_guaranteed GenWrapper<T>) -> @guaranteed S {
+// CHECK-SIL: bb0([[REG0:%.*]] : $*GenWrapper<T>):
+// CHECK-SIL:   debug_value [[REG0]], let, name "self", argno 1, expr op_deref
+// CHECK-SIL:   [[REG2:%.*]] = struct_element_addr [[REG0]], #GenWrapper._s
+// CHECK-SIL:   [[REG3:%.*]] = load_borrow [[REG2]]
+// CHECK-SIL:   return_borrow [[REG3]] from_scopes ([[REG3]])
+// CHECK-SIL: }
+
 // CHECK: sil hidden [ossa] @$s15borrow_accessor10GenWrapperV1sAA1SVvz : $@convention(method) <T> (@inout GenWrapper<T>) -> @inout S {
 // CHECK:bb0([[REG0:%.*]] : $*GenWrapper<T>):
 // CHECK:  debug_value [[REG0]], var, name "self", argno 1, expr op_deref
@@ -513,6 +522,14 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
 // CHECK:   return [[REG4]]
 // CHECK: }
 
+// CHECK-SIL: sil hidden [ossa] @$s15borrow_accessor10GenWrapperV1kAA5KlassCvb : $@convention(method) <T> (@in_guaranteed GenWrapper<T>) -> @guaranteed Klass {
+// CHECK-SIL: bb0([[REG0:%.*]] : $*GenWrapper<T>):
+// CHECK-SIL:   debug_value [[REG0]], let, name "self", argno 1, expr op_deref
+// CHECK-SIL:   [[REG2:%.*]] = struct_element_addr [[REG0]], #GenWrapper._k
+// CHECK-SIL:   [[REG3:%.*]] = load_borrow [[REG2]]
+// CHECK-SIL:   return_borrow [[REG3]] from_scopes ([[REG3]])
+// CHECK-SIL: }
+
 // CHECK: sil hidden [ossa] @$s15borrow_accessor10GenWrapperV1kAA5KlassCvz : $@convention(method) <T> (@inout GenWrapper<T>) -> @inout Klass {
 // CHECK:bb0([[REG0:%.*]] : $*GenWrapper<T>):
 // CHECK:  debug_value [[REG0]], var, name "self", argno 1, expr op_deref
@@ -552,12 +569,22 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
 // CHECK:   [[REG2:%.*]] = struct_element_addr [[REG0]], #GenWrapper._s
 // CHECK:   [[REG3:%.*]] = load_borrow [[REG2]]
 // CHECK:   [[REG4:%.*]] = function_ref @$s15borrow_accessor1SV1kAA5KlassCvb : $@convention(method) (@guaranteed S) -> @guaranteed Klass
-// CHECK:   [[REG5:%.*]] = apply [[REG4]]([[REG3]]) : $@convention(method) (@guaranteed S) -> @guaranteed Klass
-// CHECK:   [[REG6:%.*]] = unchecked_ownership [[REG5]]
+// CHECK:   [[REG5:%.*]] = unchecked_ownership [[REG3]]
+// CHECK:   [[REG6:%.*]] = apply [[REG4]]([[REG5]]) : $@convention(method) (@guaranteed S) -> @guaranteed Klass
 // CHECK:   end_borrow [[REG3]]
 // CHECK:   return [[REG6]]
 // CHECK: }
 
+// CHECK-SIL: sil hidden [ossa] @$s15borrow_accessor10GenWrapperV9nested_k1AA5KlassCvb : $@convention(method) <T> (@in_guaranteed GenWrapper<T>) -> @guaranteed Klass {
+// CHECK-SIL: bb0([[REG0:%.*]] : $*GenWrapper<T>):
+// CHECK-SIL:   debug_value [[REG0]], let, name "self", argno 1, expr op_deref
+// CHECK-SIL:   [[REG2:%.*]] = struct_element_addr [[REG0]], #GenWrapper._s
+// CHECK-SIL:   [[REG3:%.*]] = load_borrow [[REG2]]
+// CHECK-SIL:   [[REG4:%.*]] = function_ref @$s15borrow_accessor1SV1kAA5KlassCvb : $@convention(method) (@guaranteed S) -> @guaranteed Klass
+// CHECK-SIL:   [[REG5:%.*]] = apply [[REG4]]([[REG3]]) : $@convention(method) (@guaranteed S) -> @guaranteed Klass
+// CHECK-SIL:   return_borrow [[REG5]] from_scopes ([[REG3]])
+// CHECK-SIL: }
+
 // CHECK: sil hidden [ossa] @$s15borrow_accessor10GenWrapperV9nested_k1AA5KlassCvz : $@convention(method) <T> (@inout GenWrapper<T>) -> @inout Klass {
 // CHECK:bb0([[REG0:%.*]] : $*GenWrapper<T>):
 // CHECK:  debug_value [[REG0]], var, name "self", argno 1, expr op_deref
@@ -654,6 +681,15 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
 // CHECK:   return [[REG5]]
 // CHECK: }
 
+// CHECK-SIL: sil hidden [ossa] @$s15borrow_accessor12GenNCWrapperVAARi_zrlE2ncAA2NCVvb : $@convention(method) <T where T : ~Copyable> (@in_guaranteed GenNCWrapper<T>) -> @guaranteed NC {
+// CHECK-SIL: bb0([[REG0:%.*]] : $*GenNCWrapper<T>):
+// CHECK-SIL:   debug_value [[REG0]], let, name "self", argno 1, expr op_deref
+// CHECK-SIL:   [[REG2:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG0]]
+// CHECK-SIL:   [[REG3:%.*]] = struct_element_addr [[REG2]], #GenNCWrapper._nc
+// CHECK-SIL:   [[REG4:%.*]] = load_borrow [[REG3]]
+// CHECK-SIL:   return_borrow [[REG4]] from_scopes ([[REG4]])
+// CHECK-SIL: }
+
 // CHECK: sil hidden [ossa] @$s15borrow_accessor12GenNCWrapperVAARi_zrlE2ncAA2NCVvz : $@convention(method) <T where T : ~Copyable> (@inout GenNCWrapper<T>) -> @inout NC {
 // CHECK:bb0([[REG0:%.*]] : $*GenNCWrapper<T>):
 // CHECK:  debug_value [[REG0]], var, name "self", argno 1, expr op_deref
@@ -673,6 +709,15 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
 // CHECK:   return [[REG5]]
 // CHECK: }
 
+// CHECK-SIL: sil hidden [ossa] @$s15borrow_accessor12GenNCWrapperVAARi_zrlE3ncwAA0D0Vvb : $@convention(method) <T where T : ~Copyable> (@in_guaranteed GenNCWrapper<T>) -> @guaranteed NCWrapper {
+// CHECK-SIL: bb0([[REG0:%.*]] : $*GenNCWrapper<T>):
+// CHECK-SIL:   debug_value [[REG0]], let, name "self", argno 1, expr op_deref
+// CHECK-SIL:   [[REG2:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG0]]
+// CHECK-SIL:   [[REG3:%.*]] = struct_element_addr [[REG2]], #GenNCWrapper._ncw
+// CHECK-SIL:   [[REG4:%.*]] = load_borrow [[REG3]]
+// CHECK-SIL:   return_borrow [[REG4]] from_scopes ([[REG4]])
+// CHECK-SIL: }
+
 // CHECK: sil hidden [ossa] @$s15borrow_accessor12GenNCWrapperVAARi_zrlE3ncwAA0D0Vvz : $@convention(method) <T where T : ~Copyable> (@inout GenNCWrapper<T>) -> @inout NCWrapper {
 // CHECK:bb0([[REG0:%.*]] : $*GenNCWrapper<T>):
 // CHECK:  debug_value [[REG0]], var, name "self", argno 1, expr op_deref
@@ -730,8 +775,8 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
 // CHECK:   [[REG3:%.*]] = struct_element_addr [[REG2]], #GenNCWrapper._ncw
 // CHECK:   [[REG4:%.*]] = load_borrow [[REG3]]
 // CHECK:   [[REG5:%.*]] = function_ref @$s15borrow_accessor9NCWrapperV2ncAA2NCVvb : $@convention(method) (@guaranteed NCWrapper) -> @guaranteed NC
-// CHECK:   [[REG6:%.*]] = apply [[REG5]]([[REG4]]) : $@convention(method) (@guaranteed NCWrapper) -> @guaranteed NC
-// CHECK:   [[REG7:%.*]] = unchecked_ownership [[REG6]]
+// CHECK:   [[REG6:%.*]] = unchecked_ownership [[REG4]]
+// CHECK:   [[REG7:%.*]] = apply [[REG5]]([[REG6]]) : $@convention(method) (@guaranteed NCWrapper) -> @guaranteed NC
 // CHECK:   [[REG9:%.*]] = copy_value [[REG7]]
 // CHECK:   [[REG10:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG9]]
 // CHECK:   [[REG11:%.*]] = begin_borrow [[REG10]]
@@ -741,6 +786,20 @@ public struct GenNCWrapper<T : ~Copyable> : ~Copyable {
 // CHECK:   return [[REG7]]
 // CHECK: }
 
+// CHECK-SIL: sil hidden [ossa] @$s15borrow_accessor12GenNCWrapperVAARi_zrlE10nested_nc1AA2NCVvb : $@convention(method) <T where T : ~Copyable> (@in_guaranteed GenNCWrapper<T>) -> @guaranteed NC {
+// CHECK-SIL: bb0([[REG0:%.*]] : $*GenNCWrapper<T>):
+// CHECK-SIL:   debug_value [[REG0]], let, name "self", argno 1, expr op_deref
+// CHECK-SIL:   [[REG2:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG0]]
+// CHECK-SIL:   [[REG3:%.*]] = struct_element_addr [[REG2]], #GenNCWrapper._ncw
+// CHECK-SIL:   [[REG4:%.*]] = load_borrow [[REG3]]
+// CHECK-SIL:   [[REG5:%.*]] = function_ref @$s15borrow_accessor9NCWrapperV2ncAA2NCVvb : $@convention(method) (@guaranteed NCWrapper) -> @guaranteed NC
+// CHECK-SIL:   [[REG6:%.*]] = apply [[REG5]]([[REG4]]) : $@convention(method) (@guaranteed NCWrapper) -> @guaranteed NC
+// CHECK-SIL:   [[REG7:%.*]] = copy_value [[REG6]]
+// CHECK-SIL:   [[REG8:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[REG7]]
+// CHECK-SIL:   destroy_value [[REG8]]
+// CHECK-SIL:   return_borrow [[REG6]] from_scopes ([[REG4]])
+// CHECK-SIL: }
+
 // CHECK: sil hidden [ossa] @$s15borrow_accessor12GenNCWrapperVAARi_zrlE10nested_nc1AA2NCVvz : $@convention(method) <T where T : ~Copyable> (@inout GenNCWrapper<T>) -> @inout NC {
 // CHECK:bb0([[REG0:%.*]] : $*GenNCWrapper<T>):
 // CHECK:  debug_value [[REG0]], var, name "self", argno 1, expr op_deref

test/SILOptimizer/escape_effects.sil

@@ -414,3 +414,12 @@ bb0(%0 : $*Wrapper):
   %2 = struct_element_addr %0, #Wrapper._k
   return %2
 }
+
+sil [ossa] @call_mutate_accessor : $@convention(thin) (@inout Wrapper, @owned Klass) -> () {
+bb0(%0 : $*Wrapper, %1 : @owned $Klass):
+  %func = function_ref @mutate_accessor : $@convention(thin) (@inout Wrapper) -> @inout Klass
+  %addr = apply %func(%0) : $@convention(thin) (@inout Wrapper) -> @inout Klass
+  store %1 to [assign] %addr
+  %r = tuple ()
+  return %r
+}

test/Sema/borrow_and_mutate_accessors.swift

@@ -41,10 +41,10 @@ struct Struct {
 
 extension Klass {
   var i: Int {
-    borrow { // expected-error{{a borrow accessor is supported only on a struct or enum}}
+    borrow { // expected-error{{a 'borrow' accessor is supported only on a struct}}
       return 0
     }
-    mutate { // expected-error{{a mutate accessor is supported only on a struct or enum}}
+    mutate { // expected-error{{a 'mutate' accessor is supported only on a struct}}
       return &_i
     }
   }
@@ -67,3 +67,19 @@ protocol P {
   var phone: String { mutate } // expected-error{{property in protocol must have explicit { get } or { get set } specifier}} // expected-error{{expected get, read, or set in a protocol property}}
 }
 
+enum OrderStatus: ~Copyable {
+  case processing(trackingNumber: String)
+  case cancelled(reason: String)
+
+  var description: String {
+    borrow { // expected-error{{a 'borrow' accessor is supported only on a struct}}
+      switch self {
+        case .processing(let trackingNumber):
+          return trackingNumber
+        case .cancelled(let reason):
+          return reason
+      }
+    }
+  }
+}
+