Sema: Don't compute isSimpleDidSet() from getStorageImplInfo()

Author: slavapestov

include/swift/AST/StorageImpl.h

@@ -223,11 +223,11 @@ enum class ReadWriteImplKind {
   /// There's a modify coroutine.
   Modify,
 
-  /// We have a didSet which doesn't use the oldValue
-  StoredWithSimpleDidSet,
-
-  /// We have a didSet which doesn't use the oldValue
-  InheritedWithSimpleDidSet,
+  /// We have a didSet, so we're either going to use
+  /// MaterializeOrTemporary or the "simple didSet"
+  // access pattern.
+  StoredWithDidSet,
+  InheritedWithDidSet,
 };
 enum { NumReadWriteImplKindBits = 4 };
 
@@ -273,13 +273,13 @@ class StorageImplInfo {
     case WriteImplKind::StoredWithObservers:
       assert(readImpl == ReadImplKind::Stored);
       assert(readWriteImpl == ReadWriteImplKind::MaterializeToTemporary ||
-             readWriteImpl == ReadWriteImplKind::StoredWithSimpleDidSet);
+             readWriteImpl == ReadWriteImplKind::StoredWithDidSet);
       return;
 
     case WriteImplKind::InheritedWithObservers:
       assert(readImpl == ReadImplKind::Inherited);
       assert(readWriteImpl == ReadWriteImplKind::MaterializeToTemporary ||
-             readWriteImpl == ReadWriteImplKind::InheritedWithSimpleDidSet);
+             readWriteImpl == ReadWriteImplKind::InheritedWithDidSet);
       return;
 
     case WriteImplKind::Set:

lib/AST/ASTDumper.cpp

@@ -286,10 +286,10 @@ StringRef swift::getReadWriteImplKindName(ReadWriteImplKind kind) {
     return "materialize_to_temporary";
   case ReadWriteImplKind::Modify:
     return "modify_coroutine";
-  case ReadWriteImplKind::StoredWithSimpleDidSet:
-    return "stored_simple_didset";
-  case ReadWriteImplKind::InheritedWithSimpleDidSet:
-    return "inherited_simple_didset";
+  case ReadWriteImplKind::StoredWithDidSet:
+    return "stored_with_didset";
+  case ReadWriteImplKind::InheritedWithDidSet:
+    return "inherited_with_didset";
   }
   llvm_unreachable("bad kind");
 }

lib/AST/Decl.cpp

@@ -2019,9 +2019,10 @@ getDirectReadWriteAccessStrategy(const AbstractStorageDecl *storage) {
   case ReadWriteImplKind::Modify:
     return AccessStrategy::getAccessor(AccessorKind::Modify,
                                        /*dispatch*/ false);
-  case ReadWriteImplKind::StoredWithSimpleDidSet:
-  case ReadWriteImplKind::InheritedWithSimpleDidSet:
-    if (storage->requiresOpaqueModifyCoroutine()) {
+  case ReadWriteImplKind::StoredWithDidSet:
+  case ReadWriteImplKind::InheritedWithDidSet:
+    if (storage->requiresOpaqueModifyCoroutine() &&
+        storage->getParsedAccessor(AccessorKind::DidSet)->isSimpleDidSet()) {
       return AccessStrategy::getAccessor(AccessorKind::Modify,
                                          /*dispatch*/ false);
     } else {

lib/SIL/IR/TypeLowering.cpp

@@ -2684,10 +2684,13 @@ TypeConverter::getLoweredLocalCaptures(SILDeclRef fn) {
             collectAccessorCaptures(AccessorKind::MutableAddress);
             break;
           case ReadWriteImplKind::Modify:
-          case ReadWriteImplKind::StoredWithSimpleDidSet:
-          case ReadWriteImplKind::InheritedWithSimpleDidSet:
             collectAccessorCaptures(AccessorKind::Modify);
             break;
+          case ReadWriteImplKind::StoredWithDidSet:
+            // We've already processed the didSet operation.
+            break;
+          case ReadWriteImplKind::InheritedWithDidSet:
+            llvm_unreachable("inherited local variable");
           }
         }
 

lib/Sema/TypeCheckStorage.cpp

@@ -1761,9 +1761,10 @@ synthesizeCoroutineAccessorBody(AccessorDecl *accessor, ASTContext &ctx) {
   // serialized, which prevents them from being able to directly reference
   // didSet/willSet accessors, which are private.
   if (isModify &&
-      (storageReadWriteImpl == ReadWriteImplKind::StoredWithSimpleDidSet ||
-       storageReadWriteImpl == ReadWriteImplKind::InheritedWithSimpleDidSet) &&
-      !accessor->hasForcedStaticDispatch()) {
+      !accessor->hasForcedStaticDispatch() &&
+      (storageReadWriteImpl == ReadWriteImplKind::StoredWithDidSet ||
+       storageReadWriteImpl == ReadWriteImplKind::InheritedWithDidSet) &&
+      storage->getParsedAccessor(AccessorKind::DidSet)->isSimpleDidSet()) {
     return synthesizeModifyCoroutineBodyWithSimpleDidSet(accessor, ctx);
   }
 
@@ -2321,9 +2322,11 @@ IsAccessorTransparentRequest::evaluate(Evaluator &evaluator,
   }
 
   switch (storage->getReadWriteImpl()) {
-  case ReadWriteImplKind::StoredWithSimpleDidSet:
-  case ReadWriteImplKind::InheritedWithSimpleDidSet:
-    return false;
+  case ReadWriteImplKind::StoredWithDidSet:
+  case ReadWriteImplKind::InheritedWithDidSet:
+    if (storage->getAccessor(AccessorKind::DidSet)->isSimpleDidSet())
+      return false;
+    break;
   default:
     break;
   }
@@ -3078,10 +3081,6 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator,
   bool hasSetter = storage->getParsedAccessor(AccessorKind::Set);
   bool hasModify = storage->getParsedAccessor(AccessorKind::Modify);
   bool hasMutableAddress = storage->getParsedAccessor(AccessorKind::MutableAddress);
-  bool hasSimpleDidSet =
-      hasDidSet && const_cast<AccessorDecl *>(
-                       storage->getParsedAccessor(AccessorKind::DidSet))
-                       ->isSimpleDidSet();
 
   // 'get', 'read', and a non-mutable addressor are all exclusive.
   ReadImplKind readImpl;
@@ -3143,22 +3142,22 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator,
   // Check if we have observers.
   } else if (readImpl == ReadImplKind::Inherited) {
     writeImpl = WriteImplKind::InheritedWithObservers;
-    readWriteImpl = ReadWriteImplKind::MaterializeToTemporary;
 
-    if (!hasWillSet && hasSimpleDidSet) {
-      readWriteImpl = ReadWriteImplKind::InheritedWithSimpleDidSet;
-    }
+    if (hasWillSet)
+      readWriteImpl = ReadWriteImplKind::MaterializeToTemporary;
+    else
+      readWriteImpl = ReadWriteImplKind::InheritedWithDidSet;
 
   // Otherwise, it's stored.
   } else if (readImpl == ReadImplKind::Stored &&
              !cast<VarDecl>(storage)->isLet()) {
     if (hasWillSet || hasDidSet) {
       writeImpl = WriteImplKind::StoredWithObservers;
-      readWriteImpl = ReadWriteImplKind::MaterializeToTemporary;
 
-      if (!hasWillSet && hasSimpleDidSet) {
-        readWriteImpl = ReadWriteImplKind::StoredWithSimpleDidSet;
-      }
+      if (hasWillSet)
+        readWriteImpl = ReadWriteImplKind::MaterializeToTemporary;
+      else
+        readWriteImpl = ReadWriteImplKind::StoredWithDidSet;
     } else {
       writeImpl = WriteImplKind::Stored;
       readWriteImpl = ReadWriteImplKind::Stored;

lib/Serialization/Deserialization.cpp

@@ -2199,8 +2199,8 @@ getActualReadWriteImplKind(unsigned rawKind) {
   CASE(MutableAddress)
   CASE(MaterializeToTemporary)
   CASE(Modify)
-  CASE(StoredWithSimpleDidSet)
-  CASE(InheritedWithSimpleDidSet)
+  CASE(StoredWithDidSet)
+  CASE(InheritedWithDidSet)
 #undef CASE
   }
   return None;

lib/Serialization/ModuleFormat.h

@@ -55,7 +55,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
 /// Don't worry about adhering to the 80-column limit for this line.
-const uint16_t SWIFTMODULE_VERSION_MINOR = 576; // hasCReferences
+const uint16_t SWIFTMODULE_VERSION_MINOR = 577; // isSimpleDidSet() change
 
 /// A standard hash seed used for all string hashes in a serialized module.
 ///
@@ -208,8 +208,8 @@ enum class ReadWriteImplKind : uint8_t {
   MutableAddress,
   MaterializeToTemporary,
   Modify,
-  StoredWithSimpleDidSet,
-  InheritedWithSimpleDidSet,
+  StoredWithDidSet,
+  InheritedWithDidSet,
 };
 using ReadWriteImplKindField = BCFixed<3>;
 

lib/Serialization/Serialization.cpp

@@ -514,8 +514,8 @@ static unsigned getRawReadWriteImplKind(swift::ReadWriteImplKind kind) {
   CASE(MutableAddress)
   CASE(MaterializeToTemporary)
   CASE(Modify)
-  CASE(StoredWithSimpleDidSet)
-  CASE(InheritedWithSimpleDidSet)
+  CASE(StoredWithDidSet)
+  CASE(InheritedWithDidSet)
 #undef CASE
   }
   llvm_unreachable("bad kind");

test/attr/attr_native_dynamic.swift

@@ -70,7 +70,7 @@ struct Strukt {
     }
   }
 
-  // CHECK: (var_decl {{.*}} "storedWithObserver" type='Int' interface type='Int' access=internal dynamic readImpl=stored writeImpl=stored_with_observers readWriteImpl=stored_simple_didset
+  // CHECK: (var_decl {{.*}} "storedWithObserver" type='Int' interface type='Int' access=internal dynamic readImpl=stored writeImpl=stored_with_observers readWriteImpl=stored_with_didset
   // CHECK: (accessor_decl {{.*}}access=private dynamic didSet_for=storedWithObserver
   // CHECK: (accessor_decl {{.*}}access=internal dynamic get_for=storedWithObserver
   // CHECK: (accessor_decl {{.*}}access=internal set_for=storedWithObserver