Sema: Correctly mark backing storage for lazy properties and property wrappers as 'final'

By calling isFinal() first in makeFinal(), we were triggering
computation of IsFinalRequest and caching a value of 'false'.

With lazy properties, we did direct storage access, and no
accessors were synthesized anyway, but add a test for that.

With property wrappers this now means the backing storage
property is final, as intended (?).

Author: slavapestov

lib/Sema/CodeSynthesis.cpp

@@ -1626,6 +1626,11 @@ void swift::completeLazyVarImplementation(VarDecl *VD) {
                                         VD->getDeclContext());
   Storage->setInterfaceType(StorageInterfaceTy);
   Storage->setUserAccessible(false);
+
+  // Mark the backing property as 'final'. There's no sensible way to override.
+  if (VD->getDeclContext()->getSelfClassDecl())
+    makeFinal(Context, Storage);
+
   addMemberToContextIfNeeded(Storage, VD->getDeclContext(), VD);
 
   // Create the pattern binding decl for the storage decl.  This will get
@@ -1642,12 +1647,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 +1796,15 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
   if (isInvalid)
     backingVar->setInvalid();
   backingVar->setOriginalWrappedProperty(var);
+
+  // 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);
+  backingVar->overwriteSetterAccess(AccessLevel::Private);
+
   addMemberToContextIfNeeded(backingVar, dc, var);
 
   // Create the pattern binding declaration for the backing property.
@@ -1819,15 +1828,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/TypeCheckDecl.cpp

@@ -1051,10 +1051,8 @@ static void validatePatternBindingEntries(TypeChecker &tc,
 }
 
 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));
-  }
+  assert(isa<ClassDecl>(D) || D->isPotentiallyOverridable());
+  D->getAttrs().add(new (ctx) FinalAttr(/*IsImplicit=*/true));
 }
 
 namespace {

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:  }