Sema: Add @_hasInitialValue and @_hasStorage in typeCheckDecl() not validateDecl()

We only ever use this in TBDGen and interface printing, and we only ever
generate TBD files or print interfaces for declarations in primary files,
so let's avoid unnecessary work in validateDecl().

Eventually we want validateDecl() to be replaced with a getInterfaceType()
request, so adding new attributes in this path (or other side effects in
general) is a big no-no.

Author: slavapestov

lib/Sema/TypeCheckConstraints.cpp

@@ -2484,7 +2484,6 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
 
 bool TypeChecker::typeCheckPatternBinding(PatternBindingDecl *PBD,
                                           unsigned patternNumber) {
-  auto &ctx = PBD->getASTContext();
   const auto &pbe = PBD->getPatternList()[patternNumber];
   Pattern *pattern = PBD->getPattern(patternNumber);
   Expr *init = PBD->getInit(patternNumber);
@@ -2507,13 +2506,6 @@ bool TypeChecker::typeCheckPatternBinding(PatternBindingDecl *PBD,
   PBD->setPattern(patternNumber, pattern, initContext);
   PBD->setInit(patternNumber, init);
 
-  // Add the attribute that preserves the "has an initializer" value across
-  // module generation, as required for TBDGen.
-  PBD->getPattern(patternNumber)->forEachVariable([&](VarDecl *VD) {
-    if (VD->hasStorage() && !VD->getAttrs().hasAttribute<HasInitialValueAttr>())
-      VD->getAttrs().add(new (ctx) HasInitialValueAttr(/*IsImplicit=*/true));
-  });
-
   if (!hadError) {
     // If we're performing an binding to a weak or unowned variable from a
     // constructor call, emit a warning that the instance will be immediately

lib/Sema/TypeCheckDecl.cpp

@@ -2120,14 +2120,17 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
   }
 
   void visitBoundVariable(VarDecl *VD) {
+    // WARNING: Anything you put in this function will only be run when the
+    // VarDecl is fully type-checked within its own file. It will NOT be run
+    // when the VarDecl is merely used from another file.
     TC.validateDecl(VD);
 
-    // Set up accessors.
+    // Set up accessors, also lowering lazy and @NSManaged properties.
     maybeAddAccessorsToStorage(VD);
 
-    // WARNING: Anything you put in this function will only be run when the
-    // VarDecl is fully type-checked within its own file. It will NOT be run
-    // when the VarDecl is merely used from another file.
+    // Add the '@_hasStorage' attribute if this property is stored.
+    if (VD->hasStorage() && !VD->getAttrs().hasAttribute<HasStorageAttr>())
+      VD->getAttrs().add(new (TC.Context) HasStorageAttr(/*isImplicit=*/true));
 
     // Reject cases where this is a variable that has storage but it isn't
     // allowed.
@@ -2277,6 +2280,19 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
           PBD->setInit(i, defaultInit);
         }
       }
+
+      if (PBD->getInit(i)) {
+        // Add the attribute that preserves the "has an initializer" value across
+        // module generation, as required for TBDGen.
+        PBD->getPattern(i)->forEachVariable([&](VarDecl *VD) {
+          if (VD->hasStorage() &&
+              !VD->getAttrs().hasAttribute<HasInitialValueAttr>()) {
+            auto *attr = new (TC.Context) HasInitialValueAttr(
+                /*IsImplicit=*/true);
+            VD->getAttrs().add(attr);
+          }
+        });
+      }
     }
 
     bool isInSILMode = false;
@@ -3687,10 +3703,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
     auto *VD = cast<VarDecl>(D);
     auto *PBD = VD->getParentPatternBinding();
 
-    // Add the '@_hasStorage' attribute if this property is stored.
-    if (VD->hasStorage() && !VD->getAttrs().hasAttribute<HasStorageAttr>())
-      VD->getAttrs().add(new (Context) HasStorageAttr(/*isImplicit=*/true));
-
     // Note that we need to handle the fact that some VarDecls don't
     // have a PatternBindingDecl, for example the iterator in a
     // 'for ... in ...' loop.

test/SIL/Parser/final.swift

@@ -1,7 +1,7 @@
 // RUN: %target-swift-frontend %s -emit-silgen | %FileCheck %s
 
 // CHECK: final class Rect
-// CHECK: @_hasInitialValue @_hasStorage final var orgx: Double
+// CHECK: @_hasStorage @_hasInitialValue final var orgx: Double
 final class Rect {
   var orgx = 0.0
 }

test/attr/attr_objc.swift

@@ -839,7 +839,7 @@ class infer_instanceVar1 {
   }
 
   var observingAccessorsVar1: Int {
-  // CHECK: @_hasStorage @objc var observingAccessorsVar1: Int {
+  // CHECK: @objc @_hasStorage var observingAccessorsVar1: Int {
     willSet {}
     // CHECK-NEXT: {{^}} @objc get
     didSet {}
@@ -1709,7 +1709,7 @@ class HasNSManaged {
 
   @NSManaged
   var goodManaged: Class_ObjC1
-  // CHECK-LABEL: {{^}}  @objc @NSManaged @_hasStorage dynamic var goodManaged: Class_ObjC1 {
+  // CHECK-LABEL: {{^}}  @objc @NSManaged dynamic var goodManaged: Class_ObjC1 {
   // CHECK-NEXT: {{^}} @objc get
   // CHECK-NEXT: {{^}} @objc set
   // CHECK-NEXT: {{^}} }
@@ -1719,7 +1719,7 @@ class HasNSManaged {
   // expected-error@-1 {{property cannot be marked @NSManaged because its type cannot be represented in Objective-C}}
   // expected-note@-2 {{Swift structs cannot be represented in Objective-C}}
   // expected-error@-3{{'dynamic' property 'badManaged' must also be '@objc'}}
-  // CHECK-LABEL: {{^}}  @NSManaged @_hasStorage var badManaged: PlainStruct {
+  // CHECK-LABEL: {{^}}  @NSManaged var badManaged: PlainStruct {
   // CHECK-NEXT: {{^}} get
   // CHECK-NEXT: {{^}} set
   // CHECK-NEXT: {{^}} }

test/attr/hasInitialValue.swift

@@ -12,7 +12,7 @@ class C {
     // CHECK: {{^}}  @_implicitly_unwrapped_optional @_hasInitialValue var iuo: Int!
     var iuo: Int!
 
-    // CHECK: {{^}}  @_hasStorage lazy var lazyIsntARealInit: Int
+    // CHECK: {{^}}  lazy var lazyIsntARealInit: Int
     lazy var lazyIsntARealInit: Int = 0
 
     init() {