Merge pull request #20213 from rjmccall/resilient-global-modify

Synthesize modify accessors for resilient global variables

Author: rjmccall

lib/AST/Decl.cpp

@@ -1699,13 +1699,8 @@ bool AbstractStorageDecl::requiresOpaqueModifyCoroutine() const {
   if (isDynamic())
     return false;
 
-  // We only need the modify coroutine in type contexts.
-  // TODO: resilient global variables?
-  auto *dc = getDeclContext();
-  if (!dc->isTypeContext())
-    return false;
-
   // Requirements of ObjC protocols don't support the modify coroutine.
+  auto *dc = getDeclContext();
   if (auto protoDecl = dyn_cast<ProtocolDecl>(dc))
     if (protoDecl->isObjC())
       return false;

lib/SILGen/SILGen.cpp

@@ -1205,15 +1205,40 @@ void SILGenModule::visitVarDecl(VarDecl *vd) {
   if (vd->hasStorage())
     addGlobalVariable(vd);
 
-  if (vd->getImplInfo().isSimpleStored()) {
-    // If the global variable has storage, it might also have synthesized
-    // accessors. Emit them here, since they won't appear anywhere else.
-    vd->visitExpectedOpaqueAccessors([&](AccessorKind kind) {
-      auto accessor = vd->getAccessor(kind);
-      if (accessor)
-        emitFunction(accessor);
-    });
-  }
+  // Emit the variable's opaque accessors.
+  vd->visitExpectedOpaqueAccessors([&](AccessorKind kind) {
+    auto accessor = vd->getAccessor(kind);
+    if (!accessor) return;
+
+    // Only eit the accessor if it wasn't added to the surrounding decl
+    // list by the parser.  We can test that easily by looking at the impl
+    // info, since all of these accessors have a corresponding access kind
+    // whose impl should definitely point at the accessor if it was parsed.
+    //
+    // This is an unfortunate formation rule, but it's easier than messing
+    // with the invariants for now.
+    bool shouldEmit = [&] {
+      auto impl = vd->getImplInfo();
+      switch (kind) {
+      case AccessorKind::Get:
+        return impl.getReadImpl() != ReadImplKind::Get;
+      case AccessorKind::Read:
+        return impl.getReadImpl() != ReadImplKind::Read;
+      case AccessorKind::Set:
+        return impl.getWriteImpl() != WriteImplKind::Set;
+      case AccessorKind::Modify:
+        return impl.getReadWriteImpl() != ReadWriteImplKind::Modify;
+#define ACCESSOR(ID) \
+      case AccessorKind::ID:
+#define OPAQUE_ACCESSOR(ID, KEYWORD)
+#include "swift/AST/AccessorKinds.def"
+        llvm_unreachable("not an opaque accessor");
+      }
+    }();
+    if (!shouldEmit) return;
+
+    emitFunction(accessor);
+  });
 
   tryEmitPropertyDescriptor(vd);
 }

test/Inputs/resilient_global.swift

@@ -4,6 +4,8 @@ public struct EmptyResilientStruct {
   public var computed: Int {
     return 1337
   }
+
+  public mutating func mutate() {}
 }
 
 public var emptyGlobal = EmptyResilientStruct()

test/SILGen/global_resilience.swift

@@ -21,7 +21,7 @@ public var myEmptyGlobal = MyEmptyStruct()
 // CHECK:         global_addr @$s17global_resilience13myEmptyGlobalAA02MyD6StructVv
 // CHECK:         return
 
-// Synthesized getter and setter for our resilient global variable
+// Synthesized accessors for our resilient global variable
 
 // CHECK-LABEL: sil @$s17global_resilience13myEmptyGlobalAA02MyD6StructVvg
 // CHECK:         function_ref @$s17global_resilience13myEmptyGlobalAA02MyD6StructVvau
@@ -31,6 +31,12 @@ public var myEmptyGlobal = MyEmptyStruct()
 // CHECK:         function_ref @$s17global_resilience13myEmptyGlobalAA02MyD6StructVvau
 // CHECK:         return
 
+// CHECK-LABEL: sil @$s17global_resilience13myEmptyGlobalAA02MyD6StructVvM
+// CHECK:         function_ref @$s17global_resilience13myEmptyGlobalAA02MyD6StructVvau
+// CHECK:         begin_access [modify] [dynamic]
+// CHECK:         yield
+// CHECK:         end_access
+
 // Mutable addressor for fixed-layout global
 
 // CHECK-LABEL: sil [global_init] @$s17global_resilience19myFixedLayoutGlobalAA13MyEmptyStructVvau
@@ -66,6 +72,19 @@ public func getEmptyGlobal() -> EmptyResilientStruct {
   return emptyGlobal
 }
 
+// CHECK-LABEL: sil @$s17global_resilience17modifyEmptyGlobalyyF
+// CHECK:         [[MODIFY:%.*]] = function_ref @$s16resilient_global11emptyGlobalAA20EmptyResilientStructVvM
+// CHECK-NEXT:    ([[ADDR:%.*]], [[TOKEN:%.*]]) = begin_apply [[MODIFY]]()
+// CHECK-NEXT:    // function_ref
+// CHECK-NEXT:    [[FN:%.*]] = function_ref @$s16resilient_global20EmptyResilientStructV6mutateyyF
+// CHECK-NEXT:    apply [[FN]]([[ADDR]])
+// CHECK-NEXT:    end_apply [[TOKEN]]
+// CHECK-NEXT:    tuple
+// CHECK-NEXT:    return
+public func modifyEmptyGlobal() {
+  emptyGlobal.mutate()
+}
+
 // Accessing fixed-layout global from a different resilience domain --
 // call the addressor directly
 

test/SILGen/properties.swift

@@ -648,6 +648,14 @@ var global_observing_property : Int = zero {
   // CHECK: properties.zero.unsafeMutableAddressor
   // CHECK: return
 
+  // global_observing_property's setter needs to call didSet.
+
+  // CHECK-LABEL: sil hidden @$s10properties25global_observing_property{{[_0-9a-zA-Z]*}}vs
+  // CHECK: function_ref properties.global_observing_property.unsafeMutableAddressor
+  // CHECK-NEXT:  function_ref @$s10properties25global_observing_property{{[_0-9a-zA-Z]*}}vau
+  // CHECK: function_ref properties.global_observing_property.didset
+  // CHECK-NEXT: function_ref @$s10properties25global_observing_property{{[_0-9a-zA-Z]*}}vW
+
   // CHECK-LABEL: sil private @$s10properties25global_observing_property{{[_0-9a-zA-Z]*}}vW
   didSet {
     // The didSet implementation needs to call takeInt.
@@ -683,15 +691,6 @@ func force_global_observing_property_setter() {
   global_observing_property = x
 }
 
-// global_observing_property's setter needs to call didSet.
-
-// CHECK-LABEL: sil hidden @$s10properties25global_observing_property{{[_0-9a-zA-Z]*}}vs
-// CHECK: function_ref properties.global_observing_property.unsafeMutableAddressor
-// CHECK-NEXT:  function_ref @$s10properties25global_observing_property{{[_0-9a-zA-Z]*}}vau
-// CHECK: function_ref properties.global_observing_property.didset
-// CHECK-NEXT: function_ref @$s10properties25global_observing_property{{[_0-9a-zA-Z]*}}vW
-
-
 // Test local observing properties.
 
 // CHECK-LABEL: sil hidden @$s10properties24local_observing_property{{[_0-9a-zA-Z]*}}SiF