Merge pull request #25426 from slavapestov/final-request-cleanup

Small IsFinalRequest cleanup

Author: slavapestov

include/swift/AST/Decl.h

@@ -387,9 +387,8 @@ class alignas(1 << DeclAlignInBits) Decl {
     /// It is up to the debugger to instruct SIL how to access this variable.
     IsDebuggerVar : 1,
 
-    /// Whether this is a property defined in the debugger's REPL.
-    /// FIXME: Remove this once LLDB has proper support for resilience.
-    IsREPLVar : 1,
+    /// Whether this is the backing storage for a lazy property.
+    IsLazyStorageProperty : 1,
 
     /// Whether this is the backing storage for a property wrapper.
     IsPropertyWrapperBackingProperty : 1
@@ -4806,7 +4805,7 @@ class VarDecl : public AbstractStorageDecl {
     Bits.VarDecl.Specifier = static_cast<unsigned>(Sp);
     Bits.VarDecl.IsCaptureList = IsCaptureList;
     Bits.VarDecl.IsDebuggerVar = false;
-    Bits.VarDecl.IsREPLVar = false;
+    Bits.VarDecl.IsLazyStorageProperty = false;
     Bits.VarDecl.HasNonPatternBindingInit = false;
     Bits.VarDecl.IsPropertyWrapperBackingProperty = false;
   }
@@ -5095,12 +5094,13 @@ class VarDecl : public AbstractStorageDecl {
   void setDebuggerVar(bool IsDebuggerVar) {
     Bits.VarDecl.IsDebuggerVar = IsDebuggerVar;
   }
-  
-  /// Is this a special debugger REPL variable?
-  /// FIXME: Remove this once LLDB has proper support for resilience.
-  bool isREPLVar() const { return Bits.VarDecl.IsREPLVar; }
-  void setREPLVar(bool IsREPLVar) {
-    Bits.VarDecl.IsREPLVar = IsREPLVar;
+
+  /// Is this the synthesized storage for a 'lazy' property?
+  bool isLazyStorageProperty() const {
+    return Bits.VarDecl.IsLazyStorageProperty;
+  }
+  void setLazyStorageProperty(bool IsLazyStorage) {
+    Bits.VarDecl.IsLazyStorageProperty = IsLazyStorage;
   }
 
   /// Retrieve the custom attribute that attaches a property wrapper to this

lib/Sema/CodeSynthesis.cpp

@@ -1625,7 +1625,9 @@ void swift::completeLazyVarImplementation(VarDecl *VD) {
                                         StorageName,
                                         VD->getDeclContext());
   Storage->setInterfaceType(StorageInterfaceTy);
+  Storage->setLazyStorageProperty(true);
   Storage->setUserAccessible(false);
+
   addMemberToContextIfNeeded(Storage, VD->getDeclContext(), VD);
 
   // Create the pattern binding decl for the storage decl.  This will get
@@ -1642,12 +1644,7 @@ void swift::completeLazyVarImplementation(VarDecl *VD) {
   VD->getGetter()->setBodySynthesizer(&synthesizeLazyGetterBody, Storage);
   VD->getSetter()->setBodySynthesizer(&synthesizeLazySetterBody, Storage);
 
-  // Mark the vardecl to be final, implicit, and private.  In a class, this
-  // prevents it from being dynamically dispatched.  Note that we do this after
-  // the accessors are set up, because we don't want the setter for the lazy
-  // property to inherit these properties from the storage.
-  if (VD->getDeclContext()->getSelfClassDecl())
-    makeFinal(Context, Storage);
+  // The storage is implicit and private.
   Storage->setImplicit();
   Storage->overwriteAccess(AccessLevel::Private);
   Storage->overwriteSetterAccess(AccessLevel::Private);
@@ -1796,6 +1793,11 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
   if (isInvalid)
     backingVar->setInvalid();
   backingVar->setOriginalWrappedProperty(var);
+
+  // The backing storage is 'private'.
+  backingVar->overwriteAccess(AccessLevel::Private);
+  backingVar->overwriteSetterAccess(AccessLevel::Private);
+
   addMemberToContextIfNeeded(backingVar, dc, var);
 
   // Create the pattern binding declaration for the backing property.
@@ -1819,15 +1821,6 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
 
     tc.typeCheckPatternBinding(parentPBD, patternNumber);
   }
-  // Mark the backing property as 'final'. There's no sensible way to override.
-  if (dc->getSelfClassDecl())
-    makeFinal(ctx, backingVar);
-
-  // The backing storage is 'private'.
-  backingVar->overwriteAccess(AccessLevel::Private);
-
-  // Determine setter access.
-  backingVar->overwriteSetterAccess(AccessLevel::Private);
 
   Expr *originalInitialValue = nullptr;
   if (Expr *init = parentPBD->getInit(patternNumber)) {

lib/Sema/CodeSynthesis.h

@@ -42,9 +42,6 @@ class TypeChecker;
 
 class ObjCReason;
 
-// These are implemented in TypeCheckDecl.cpp.
-void makeFinal(ASTContext &ctx, ValueDecl *D);
-
 // Implemented in TypeCheckerOverride.cpp
 bool checkOverrides(ValueDecl *decl);
 

lib/Sema/TypeCheckDecl.cpp

@@ -1050,13 +1050,6 @@ static void validatePatternBindingEntries(TypeChecker &tc,
     validatePatternBindingEntry(tc, binding, i);
 }
 
-void swift::makeFinal(ASTContext &ctx, ValueDecl *D) {
-  if (D && !D->isFinal()) {
-    assert(isa<ClassDecl>(D) || D->isPotentiallyOverridable());
-    D->getAttrs().add(new (ctx) FinalAttr(/*IsImplicit=*/true));
-  }
-}
-
 namespace {
 // The raw values of this enum must be kept in sync with
 // diag::implicitly_final_cannot_be_open.
@@ -1070,12 +1063,8 @@ enum class ImplicitlyFinalReason : unsigned {
 };
 }
 
-static bool inferFinalAndDiagnoseIfNeeded(ValueDecl *D,
+static bool inferFinalAndDiagnoseIfNeeded(ValueDecl *D, ClassDecl *cls,
                                           StaticSpellingKind staticSpelling) {
-  auto cls = D->getDeclContext()->getSelfClassDecl();
-  if (!cls)
-    return false;
-
   // Are there any reasons to infer 'final'? Prefer 'static' over the class
   // being final for the purposes of diagnostics.
   Optional<ImplicitlyFinalReason> reason;
@@ -1186,15 +1175,28 @@ doesAccessorNeedDynamicAttribute(AccessorDecl *accessor, Evaluator &evaluator) {
 
 llvm::Expected<bool>
 IsFinalRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
+  if (isa<ClassDecl>(decl))
+    return decl->getAttrs().hasAttribute<FinalAttr>();
+
+  auto cls = decl->getDeclContext()->getSelfClassDecl();
+  if (!cls)
+    return false;
+
   switch (decl->getKind()) {
     case DeclKind::Var: {
       // Properties are final if they are declared 'static' or a 'let'
       auto *VD = cast<VarDecl>(decl);
+
+      // Backing storage for 'lazy' or property wrappers is always final.
+      if (VD->isLazyStorageProperty() ||
+          VD->getOriginalWrappedProperty())
+        return true;
+
       if (auto *nominalDecl = VD->getDeclContext()->getSelfClassDecl()) {
         // If this variable is a class member, mark it final if the
         // class is final, or if it was declared with 'let'.
         auto *PBD = VD->getParentPatternBinding();
-        if (PBD && inferFinalAndDiagnoseIfNeeded(decl, PBD->getStaticSpelling()))
+        if (PBD && inferFinalAndDiagnoseIfNeeded(decl, cls, PBD->getStaticSpelling()))
           return true;
 
         if (VD->isLet()) {
@@ -1219,7 +1221,7 @@ IsFinalRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
     case DeclKind::Func: {
       // Methods declared 'static' are final.
       auto staticSpelling = cast<FuncDecl>(decl)->getStaticSpelling();
-      if (inferFinalAndDiagnoseIfNeeded(decl, staticSpelling))
+      if (inferFinalAndDiagnoseIfNeeded(decl, cls, staticSpelling))
         return true;
       break;
     }
@@ -1230,9 +1232,7 @@ IsFinalRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
           case AccessorKind::DidSet:
           case AccessorKind::WillSet:
             // Observing accessors are marked final if in a class.
-            if (accessor->getDeclContext()->getSelfClassDecl())
-              return true;
-            break;
+            return true;
 
           case AccessorKind::Read:
           case AccessorKind::Modify:
@@ -1254,7 +1254,7 @@ IsFinalRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
     case DeclKind::Subscript: {
       // Member subscripts.
       auto staticSpelling = cast<SubscriptDecl>(decl)->getStaticSpelling();
-      if (inferFinalAndDiagnoseIfNeeded(decl, staticSpelling))
+      if (inferFinalAndDiagnoseIfNeeded(decl, cls, staticSpelling))
         return true;
       break;
     }

test/SILGen/lazy_properties.swift

@@ -34,3 +34,15 @@ func test21057425() {
 struct HasAnonymousParameters {
   lazy var x = { $0 }(0)
 }
+
+class LazyClass {
+  lazy var x = 0
+}
+
+// CHECK-LABEL: sil hidden [ossa] @$s15lazy_properties9LazyClassC1xSivg : $@convention(method) (@guaranteed LazyClass) -> Int
+// CHECK: ref_element_addr %0 : $LazyClass, #LazyClass.$__lazy_storage_$_x
+// CHECK: return
+
+// CHECK-LABEL: sil hidden [ossa] @$s15lazy_properties9LazyClassC1xSivs : $@convention(method) (Int, @guaranteed LazyClass) -> ()
+// CHECK: ref_element_addr %1 : $LazyClass, #LazyClass.$__lazy_storage_$_x
+// CHECK: return

test/SILGen/property_wrappers.swift

@@ -264,7 +264,7 @@ class ClassUsingWrapper {
 extension ClassUsingWrapper {
   // CHECK-LABEL: sil hidden [ossa] @$s17property_wrappers17ClassUsingWrapperC04testcdE01cyAC_tF : $@convention(method) (@guaranteed ClassUsingWrapper, @guaranteed ClassUsingWrapper) -> () {
   func testClassUsingWrapper(c: ClassUsingWrapper) {
-    // CHECK: class_method [[GETTER:%.*]] : $ClassUsingWrapper, #ClassUsingWrapper.$x!getter.1
+    // CHECK: ref_element_addr %1 : $ClassUsingWrapper, #ClassUsingWrapper.$x
     self.$x.test()
   }
 }
@@ -298,9 +298,9 @@ class UseWrapperWithDefaultInit {
 
 
 // CHECK-LABEL: sil_vtable ClassUsingWrapper {
-// CHECK:  #ClassUsingWrapper.x!getter.1: (ClassUsingWrapper) -> () -> Int : @$s17property_wrappers17ClassUsingWrapperC1xSivg   // ClassUsingWrapper.x.getter
-// CHECK:  #ClassUsingWrapper.x!setter.1: (ClassUsingWrapper) -> (Int) -> () : @$s17property_wrappers17ClassUsingWrapperC1xSivs // ClassUsingWrapper.x.setter
-// CHECK:  #ClassUsingWrapper.x!modify.1: (ClassUsingWrapper) -> () -> () : @$s17property_wrappers17ClassUsingWrapperC1xSivM    // ClassUsingWrapper.x.modify
-// CHECK:  #ClassUsingWrapper.$x!getter.1: (ClassUsingWrapper) -> () -> WrapperWithInitialValue<Int> : @$s17property_wrappers17ClassUsingWrapperC2$x33_{{.*}}16WithInitialValueVySiGvg       // ClassUsingWrapper.$x.getter
-// CHECK:  #ClassUsingWrapper.$x!setter.1: (ClassUsingWrapper) -> (WrapperWithInitialValue<Int>) -> () : @$s17property_wrappers17ClassUsingWrapperC2$x33_{{.*}}16WithInitialValueVySiGvs     // ClassUsingWrapper.$x.setter
-// CHECK:  #ClassUsingWrapper.$x!modify.1: (ClassUsingWrapper) -> () -> () : @$s17property_wrappers17ClassUsingWrapperC2$x33_{{.*}}16WithInitialValueVySiGvM // ClassUsingWrapper.$x.modify
+// CHECK-NEXT:  #ClassUsingWrapper.x!getter.1: (ClassUsingWrapper) -> () -> Int : @$s17property_wrappers17ClassUsingWrapperC1xSivg   // ClassUsingWrapper.x.getter
+// CHECK-NEXT:  #ClassUsingWrapper.x!setter.1: (ClassUsingWrapper) -> (Int) -> () : @$s17property_wrappers17ClassUsingWrapperC1xSivs // ClassUsingWrapper.x.setter
+// CHECK-NEXT:  #ClassUsingWrapper.x!modify.1: (ClassUsingWrapper) -> () -> () : @$s17property_wrappers17ClassUsingWrapperC1xSivM    // ClassUsingWrapper.x.modify
+// CHECK-NEXT:  #ClassUsingWrapper.init!allocator.1: (ClassUsingWrapper.Type) -> () -> ClassUsingWrapper : @$s17property_wrappers17ClassUsingWrapperCACycfC
+// CHECK-NEXT: #ClassUsingWrapper.deinit!deallocator.1: @$s17property_wrappers17ClassUsingWrapperCfD
+// CHECK-NEXT:  }