[Typechecker] Fix _modify for wrapped properties with observers

include/swift/AST/Decl.h

@@ -4806,6 +4806,13 @@ class AbstractStorageDecl : public ValueDecl {
   /// Does this storage require a 'modify' accessor in its opaque-accessors set?
   bool requiresOpaqueModifyCoroutine() const;
 
+  /// Does this storage have any explicit observers (willSet or didSet) attached
+  /// to it?
+  bool hasObservers() const {
+    return getParsedAccessor(AccessorKind::WillSet) ||
+           getParsedAccessor(AccessorKind::DidSet);
+  }
+
   SourceRange getBracesRange() const {
     if (auto info = Accessors.getPointer())
       return info->getBracesRange();

lib/Sema/TypeCheckDeclOverride.cpp

@@ -1691,8 +1691,7 @@ static bool checkSingleOverride(ValueDecl *override, ValueDecl *base) {
     // Make sure that the overriding property doesn't have storage.
     if ((overrideASD->hasStorage() ||
          overrideASD->getAttrs().hasAttribute<LazyAttr>()) &&
-        !(overrideASD->getParsedAccessor(AccessorKind::WillSet) ||
-          overrideASD->getParsedAccessor(AccessorKind::DidSet))) {
+        !overrideASD->hasObservers()) {
       bool downgradeToWarning = false;
       if (!ctx.isSwiftVersionAtLeast(5) &&
           overrideASD->getAttrs().hasAttribute<LazyAttr>()) {

lib/Sema/TypeCheckStorage.cpp

@@ -1556,8 +1556,7 @@ synthesizeSetterBody(AccessorDecl *setter, ASTContext &ctx) {
     }
 
     if (var->hasAttachedPropertyWrapper()) {
-      if (var->getParsedAccessor(AccessorKind::WillSet) ||
-          var->getParsedAccessor(AccessorKind::DidSet)) {
+      if (var->hasObservers()) {
         return synthesizeObservedSetterBody(setter, TargetImpl::Wrapper, ctx);
       }
 
@@ -1670,14 +1669,17 @@ synthesizeCoroutineAccessorBody(AccessorDecl *accessor, ASTContext &ctx) {
 
   // If this is a variable with an attached property wrapper, then
   // the accessors need to yield the wrappedValue or projectedValue.
-  if (auto var = dyn_cast<VarDecl>(storage)) {
-    if (var->hasAttachedPropertyWrapper()) {
-      target = TargetImpl::Wrapper;
-    }
+  if (accessor->getAccessorKind() == AccessorKind::Read ||
+      storageReadWriteImpl == ReadWriteImplKind::Modify) {
+    if (auto var = dyn_cast<VarDecl>(storage)) {
+      if (var->hasAttachedPropertyWrapper()) {
+        target = TargetImpl::Wrapper;
+      }
 
-    if (var->getOriginalWrappedProperty(
-            PropertyWrapperSynthesizedPropertyKind::StorageWrapper)) {
-      target = TargetImpl::WrapperStorage;
+      if (var->getOriginalWrappedProperty(
+              PropertyWrapperSynthesizedPropertyKind::StorageWrapper)) {
+        target = TargetImpl::WrapperStorage;
+      }
     }
   }
 
@@ -2134,8 +2136,7 @@ IsAccessorTransparentRequest::evaluate(Evaluator &evaluator,
       // FIXME: This should be folded into the WriteImplKind below.
       if (auto var = dyn_cast<VarDecl>(storage)) {
         if (var->hasAttachedPropertyWrapper()) {
-          if (var->getParsedAccessor(AccessorKind::DidSet) ||
-              var->getParsedAccessor(AccessorKind::WillSet))
+          if (var->hasObservers())
             return false;
 
           break;
@@ -2683,11 +2684,17 @@ static void finishPropertyWrapperImplInfo(VarDecl *var,
     }
   }
 
-  if (wrapperSetterIsUsable)
+  if (!wrapperSetterIsUsable) {
+    info = StorageImplInfo::getImmutableComputed();
+    return;
+  }
+
+  if (var->hasObservers()) {
+    info = StorageImplInfo::getMutableComputed();
+  } else {
     info = StorageImplInfo(ReadImplKind::Get, WriteImplKind::Set,
                            ReadWriteImplKind::Modify);
-  else
-    info = StorageImplInfo::getImmutableComputed();
+  }
 }
 
 static void finishNSManagedImplInfo(VarDecl *var,

test/SILGen/property_wrapper_observers.swift

@@ -0,0 +1,71 @@
+// RUN: %target-swift-emit-silgen %s | %FileCheck %s
+
+// 1. Make sure the wrapped property setter calls the observers
+// 2. Make sure the synthesized _modify coroutine calls the wrapped property setter
+
+@propertyWrapper 
+struct Foo {
+  private var _storage: [Int] = []
+
+  init(wrappedValue value: [Int]) {
+    self._storage = value
+  }
+
+  var wrappedValue: [Int] {
+    get { _storage }
+    set { _storage = newValue }
+  }
+}
+
+class Bar {
+  @Foo var someArray = [1, 2, 3] {
+    willSet {}
+    didSet {}
+  }
+}
+
+// Bar.someArray.setter
+
+// CHECK-LABEL: sil hidden [ossa] @$s26property_wrapper_observers3BarC9someArraySaySiGvs : $@convention(method) (@owned Array<Int>, @guaranteed Bar) -> () {
+// CHECK: bb0([[VALUE:%.*]] : @owned $Array<Int>, [[BAR:%.*]] : @guaranteed $Bar):
+
+// CHECK: [[WILLSET:%.*]] = function_ref @$s26property_wrapper_observers3BarC9someArraySaySiGvw : $@convention(method) (@guaranteed Array<Int>, @guaranteed Bar) -> ()
+// CHECK-NEXT: [[RESULT_WS:%.*]] = apply [[WILLSET]](%{{[0-9]+}}, [[BAR]]) : $@convention(method) (@guaranteed Array<Int>, @guaranteed Bar) -> ()
+
+// CHECK: [[WRAPPED_VALUE_SETTER:%.*]] = function_ref @$s26property_wrapper_observers3FooV12wrappedValueSaySiGvs : $@convention(method) (@owned Array<Int>, @inout Foo) -> ()
+// CHECK-NEXT: [[RESULT_WVS:%.*]] = apply [[WRAPPED_VALUE_SETTER]](%{{[0-9]+}}, %{{[0-9]+}}) : $@convention(method) (@owned Array<Int>, @inout Foo) -> ()
+
+// CHECK: [[DIDSET:%.*]] = function_ref @$s26property_wrapper_observers3BarC9someArraySaySiGvW : $@convention(method) (@guaranteed Bar) -> ()
+// CHECK-NEXT: [[RESULT_DS:%.*]] = apply [[DIDSET]]([[BAR]]) : $@convention(method) (@guaranteed Bar) -> ()
+// CHECK: }
+
+// Bar.someArray.modify
+
+// CHECK-LABEL: sil hidden [ossa] @$s26property_wrapper_observers3BarC9someArraySaySiGvM : $@yield_once @convention(method) (@guaranteed Bar) -> @yields @inout Array<Int> {
+// CHECK: bb0([[BAR:%.*]] : @guaranteed $Bar):
+// CHECK-NEXT: debug_value [[BAR]] : $Bar, let, name "self", argno 1
+// CHECK-NEXT: [[ALLOC_STACK:%.*]] = alloc_stack $Array<Int>
+// CHECK-NEXT:  // function_ref Bar.someArray.getter
+// CHECK-NEXT: [[GETTER:%.*]] = function_ref @$s26property_wrapper_observers3BarC9someArraySaySiGvg : $@convention(method) (@guaranteed Bar) -> @owned Array<Int>
+// CHECK-NEXT:  [[RESULT:%.*]] = apply [[GETTER]]([[BAR]]) : $@convention(method) (@guaranteed Bar) -> @owned Array<Int>
+// CHECK-NEXT:  store [[RESULT]] to [init] [[ALLOC_STACK]] : $*Array<Int>
+// CHECK-NEXT:  yield [[ALLOC_STACK]] : $*Array<Int>, resume bb1, unwind bb2
+
+// CHECK: bb1:
+// CHECK-NEXT:  [[VALUE:%.*]] = load [take] [[ALLOC_STACK]] : $*Array<Int>
+// CHECK-NEXT:  // function_ref Bar.someArray.setter
+// CHECK-NEXT:  [[SETTER:%.*]] = function_ref @$s26property_wrapper_observers3BarC9someArraySaySiGvs : $@convention(method) (@owned Array<Int>, @guaranteed Bar) -> ()
+// CHECK-NEXT:  [[RESULT:%.*]] = apply [[SETTER]]([[VALUE]], [[BAR]]) : $@convention(method) (@owned Array<Int>, @guaranteed Bar) -> ()
+// CHECK-NEXT:  dealloc_stack [[ALLOC_STACK]] : $*Array<Int>
+// CHECK-NEXT:  [[TUPLE:%.*]] = tuple ()
+// CHECK-NEXT:  return [[TUPLE]] : $()
+
+// CHECK: bb2:
+// CHECK-NEXT:  [[NEWVALUE:%.*]] = load [copy] [[ALLOC_STACK]] : $*Array<Int>
+// CHECK-NEXT:  // function_ref Bar.someArray.setter
+// CHECK-NEXT:  [[SETTER:%.*]] = function_ref @$s26property_wrapper_observers3BarC9someArraySaySiGvs : $@convention(method) (@owned Array<Int>, @guaranteed Bar) -> ()
+// CHECK-NEXT:  [[RESULT:%.*]] = apply [[SETTER]]([[NEWVALUE]], [[BAR]]) : $@convention(method) (@owned Array<Int>, @guaranteed Bar) -> ()
+// CHECK-NEXT:  destroy_addr [[ALLOC_STACK]] : $*Array<Int>
+// CHECK-NEXT:  dealloc_stack [[ALLOC_STACK]] : $*Array<Int>
+// CHECK-NEXT:  unwind
+// CHECK-END: }

test/Sema/property_wrapper_property_with_observer.swift

@@ -0,0 +1,34 @@
+// RUN: %target-run-simple-swift
+
+// REQUIRES: executable_test
+
+@propertyWrapper 
+struct Foo {
+  private var _storage: [Int] = []
+
+  init(wrappedValue value: [Int]) {
+    self._storage = value
+  }
+
+  var wrappedValue: [Int] {
+    get { _storage }
+    set { _storage = newValue }
+  }
+}
+
+class Bar {
+  @Foo var someArray = [1, 2, 3] {
+    willSet {
+      print(newValue) 
+    }
+
+    didSet {
+      print(oldValue)
+    }
+  }
+}
+
+let bar = Bar()
+// CHECK: [4, 2, 3]
+// CHECK: [1, 2, 3]
+bar.someArray[0] = 4

tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp

@@ -1190,8 +1190,7 @@ static bool inferIsSettableSyntactically(const AbstractStorageDecl *D) {
   }
   if (D->hasParsedAccessors()) {
     return D->getParsedAccessor(AccessorKind::Set) != nullptr ||
-           D->getParsedAccessor(AccessorKind::WillSet) != nullptr ||
-           D->getParsedAccessor(AccessorKind::DidSet) != nullptr;
+           D->hasObservers();
   } else {
     return true;
   }