AST: Fix name lookup from within lazy property initializers

Allow instance properties and methods to be referenced from
within a lazy property initializer, with or without explicit
'self.' qualification.

The old behavior in Swift 3 was an incredible combination
of odd quirks:

- If the lazy property had an explicitly-written type, it was
  possible to reference instance members from the initializer
  expression by explicitly prefixing 'self.'.

- However, if the lazy property type is inferred, it would
  first be type checked in the initializer context, which
  has no 'self' available.

- Unqualified references to instance members did not work
  at all, because name lookup thought the "location" of the
  lookup was outside of the body of the getter.

- Unqualified references to static properties worked, however
  unqualified references to static methods did not, and
  produced a bogus diagnostic, because one part of the name
  lookup code thought that initializers were "instance
  context" and another thought they were "static context".

This patch improves on the old behavior with the following
fixes:

- Give PatternBindingInitializers associated with lazy
  properties an implicit 'self' declaration for use by
  name lookup.

- In order to allow "re-parenting" the initializer after it
  has been type checked into the body of the getter, "steal"
  the initializer's 'self' when buiding the getter.

- Fix up name lookup and make it aware of the implicit
  'self' decl of a PatternBindingInitializer.

This improves upon an earlier fix for this issue by Doug Gregor
which only worked with ASTScope enabled; the new fix is more
general and shares logic between the two name lookup
implementations.

Fixes <rdar://problem/16888679>, <https://bugs.swift.org/browse/SR-48>,
<https://bugs.swift.org/browse/SR-2203>,
<https://bugs.swift.org/browse/SR-4663>, and the countless other
dupes of this issue.

Author: slavapestov

include/swift/AST/Initializer.h

@@ -70,17 +70,21 @@ class Initializer : public DeclContext {
 class PatternBindingInitializer : public Initializer {
   PatternBindingDecl *Binding;
 
+  // created lazily for 'self' lookup from lazy property initializer
+  ParamDecl *SelfParam;
+
   friend class ASTContext; // calls reset on unused contexts
 
   void reset(DeclContext *parent) {
     setParent(parent);
     Binding = nullptr;
+    SelfParam = nullptr;
   }
 
 public:
   explicit PatternBindingInitializer(DeclContext *parent)
     : Initializer(InitializerKind::PatternBinding, parent),
-      Binding(nullptr) {
+      Binding(nullptr), SelfParam(nullptr) {
     SpareBits = 0;
   }
 
@@ -95,6 +99,8 @@ class PatternBindingInitializer : public Initializer {
 
   unsigned getBindingIndex() const { return SpareBits; }
 
+  ParamDecl *getImplicitSelfDecl();
+
   static bool classof(const DeclContext *DC) {
     if (auto init = dyn_cast<Initializer>(DC))
       return classof(init);

lib/AST/ASTScope.cpp

@@ -1903,28 +1903,17 @@ SmallVector<ValueDecl *, 4> ASTScope::getLocalBindings() const {
     }
     break;
 
-  case ASTScopeKind::PatternInitializer:
-    // FIXME: This causes recursion that we cannot yet handle.
-#if false
+  case ASTScopeKind::PatternInitializer: {
     // 'self' is available within the pattern initializer of a 'lazy' variable.
-    if (auto singleVar = patternBinding.decl->getSingleVar()) {
-      if (singleVar->getAttrs().hasAttribute<LazyAttr>() &&
-          singleVar->getDeclContext()->isTypeContext()) {
-        // If there is no getter (yet), add them.
-        if (!singleVar->getGetter()) {
-          ASTContext &ctx = singleVar->getASTContext();
-          if (auto resolver = ctx.getLazyResolver())
-            resolver->introduceLazyVarAccessors(singleVar);
-        }
-
-        // Add the getter's 'self'.
-        if (auto getter = singleVar->getGetter())
-          if (auto self = getter->getImplicitSelfDecl())
-            result.push_back(self);
-      }
+    auto *initContext = cast_or_null<PatternBindingInitializer>(
+      patternBinding.decl->getPatternList()[0].getInitContext());
+    if (initContext) {
+      if (auto *selfParam = initContext->getImplicitSelfDecl())
+        result.push_back(selfParam);
     }
-#endif
+
     break;
+  }
 
   case ASTScopeKind::Closure:
     // Note: Parameters all at once is different from functions, but it's not

lib/AST/Decl.cpp

@@ -986,6 +986,25 @@ PatternBindingDecl *PatternBindingDecl::createDeserialized(
   return PBD;
 }
 
+ParamDecl *PatternBindingInitializer::getImplicitSelfDecl() {
+  if (SelfParam)
+    return SelfParam;
+
+  if (auto singleVar = getBinding()->getSingleVar()) {
+    auto *DC = singleVar->getDeclContext();
+    if (singleVar->getAttrs().hasAttribute<LazyAttr>() &&
+        DC->isTypeContext()) {
+      bool isInOut = !DC->getDeclaredTypeOfContext()->hasReferenceSemantics();
+      SelfParam = ParamDecl::createSelf(SourceLoc(), DC,
+                                        singleVar->isStatic(),
+                                        isInOut);
+      SelfParam->setDeclContext(this);
+    }
+  }
+
+  return SelfParam;
+}
+
 static bool patternContainsVarDeclBinding(const Pattern *P, const VarDecl *VD) {
   bool Result = false;
   P->forEachVariable([&](VarDecl *FoundVD) {
@@ -3916,9 +3935,12 @@ Pattern *VarDecl::getParentPattern() const {
 }
 
 bool VarDecl::isSelfParameter() const {
-  if (isa<ParamDecl>(this))
+  if (isa<ParamDecl>(this)) {
     if (auto *AFD = dyn_cast<AbstractFunctionDecl>(getDeclContext()))
       return AFD->getImplicitSelfDecl() == this;
+    if (auto *PBI = dyn_cast<PatternBindingInitializer>(getDeclContext()))
+      return PBI->getImplicitSelfDecl() == this;
+  }
 
   return false;
 }

lib/AST/NameLookup.cpp

@@ -510,29 +510,11 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
         // Pattern binding initializers are only interesting insofar as they
         // affect lookup in an enclosing nominal type or extension thereof.
         if (auto *bindingInit = dyn_cast<PatternBindingInitializer>(dc)) {
-          if (auto binding = bindingInit->getBinding()) {
-            // Look for 'self' for a lazy variable initializer.
-            if (auto singleVar = binding->getSingleVar())
-              // We only care about lazy variables.
-              if (singleVar->getAttrs().hasAttribute<LazyAttr>()) {
-
-              // 'self' will be listed in the local bindings.
-              for (auto local : localBindings) {
-                auto param = dyn_cast<ParamDecl>(local);
-                if (!param) continue;
-
-
-                // If we have a variable that's the implicit self of its enclosing
-                // context, mark it as 'self'.
-                if (auto func = dyn_cast<FuncDecl>(param->getDeclContext())) {
-                  if (param == func->getImplicitSelfDecl()) {
-                    selfDecl = param;
-                    break;
-                  }
-                }
-              }
-            }
-          }
+          // Lazy variable initializer contexts have a 'self' parameter for
+          // instance member lookup.
+          if (auto *selfParam = bindingInit->getImplicitSelfDecl())
+            selfDecl = selfParam;
+
           continue;
         }
 
@@ -646,17 +628,46 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
         Type ExtendedType;
         bool isTypeLookup = false;
 
-        // If this declcontext is an initializer for a static property, then we're
-        // implicitly doing a static lookup into the parent declcontext.
-        if (auto *PBI = dyn_cast<PatternBindingInitializer>(DC))
-          if (!DC->getParent()->isModuleScopeContext()) {
-            if (auto *PBD = PBI->getBinding()) {
-              isTypeLookup = PBD->isStatic();
-              DC = DC->getParent();
-            }
+        if (auto *PBI = dyn_cast<PatternBindingInitializer>(DC)) {
+          auto *PBD = PBI->getBinding();
+          assert(PBD);
+
+          // Lazy variable initializer contexts have a 'self' parameter for
+          // instance member lookup.
+          if (auto *selfParam = PBI->getImplicitSelfDecl()) {
+            Consumer.foundDecl(selfParam,
+                               DeclVisibilityKind::FunctionParameter);
+            if (!Results.empty())
+              return;
+
+            DC = DC->getParent();
+
+            BaseDecl = selfParam;
+            ExtendedType = DC->getSelfTypeInContext();
+            MetaBaseDecl = DC->getAsNominalTypeOrNominalTypeExtensionContext();
+
+            isTypeLookup = PBD->isStatic();
+          }
+          // Initializers for stored properties of types perform static
+          // lookup into the surrounding context.
+          else if (PBD->getDeclContext()->isTypeContext()) {
+            DC = DC->getParent();
+
+            ExtendedType = DC->getSelfTypeInContext();
+            MetaBaseDecl = DC->getAsNominalTypeOrNominalTypeExtensionContext();
+            BaseDecl = MetaBaseDecl;
+
+            isTypeLookup = PBD->isStatic(); // FIXME
+
+            isCascadingUse = DC->isCascadingContextForLookup(false);
           }
-        
-        if (auto *AFD = dyn_cast<AbstractFunctionDecl>(DC)) {
+          // Otherwise, we have an initializer for a global or local property.
+          // There's not much to find here, we'll keep going up to a parent
+          // context.
+
+          if (!isCascadingUse.hasValue())
+            isCascadingUse = DC->isCascadingContextForLookup(false);
+        } else if (auto *AFD = dyn_cast<AbstractFunctionDecl>(DC)) {
           // Look for local variables; normally, the parser resolves these
           // for us, but it can't do the right thing inside local types.
           // FIXME: when we can parse and typecheck the function body partially
@@ -690,9 +701,12 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
               if (FD->isStatic())
                 isTypeLookup = true;
 
-            // If we're not in the body of the function, the base declaration
+            // If we're not in the body of the function (for example, we
+            // might be type checking a default argument expression and
+            // performing name lookup from there), the base declaration
             // is the nominal type, not 'self'.
-            if (Loc.isValid() &&
+            if (!AFD->isImplicit() &&
+                Loc.isValid() &&
                 AFD->getBodySourceRange().isValid() &&
                 !SM.rangeContainsTokenLoc(AFD->getBodySourceRange(), Loc)) {
               BaseDecl = MetaBaseDecl;

lib/Sema/CodeSynthesis.cpp

@@ -22,6 +22,7 @@
 #include "swift/AST/Availability.h"
 #include "swift/AST/Expr.h"
 #include "swift/AST/GenericEnvironment.h"
+#include "swift/AST/Initializer.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/Basic/Defer.h"
@@ -143,10 +144,25 @@ static FuncDecl *createGetterPrototype(AbstractStorageDecl *storage,
   SmallVector<ParameterList*, 2> getterParams;
 
   // The implicit 'self' argument if in a type context.
-  if (storage->getDeclContext()->isTypeContext())
-    getterParams.push_back(ParameterList::createSelf(loc,
-                                                     storage->getDeclContext(),
-                                                     /*isStatic*/false));
+  if (storage->getDeclContext()->isTypeContext()) {
+    ParamDecl *selfDecl;
+
+    // For lazy properties, steal the 'self' from the initializer context.
+    if (storage->getAttrs().hasAttribute<LazyAttr>()) {
+      auto *varDecl = cast<VarDecl>(storage);
+      auto *bindingDecl = varDecl->getParentPatternBinding();
+      auto *bindingInit = cast<PatternBindingInitializer>(
+        bindingDecl->getPatternEntryForVarDecl(varDecl).getInitContext());
+
+      selfDecl = bindingInit->getImplicitSelfDecl();
+    } else {
+      selfDecl = ParamDecl::createSelf(loc,
+                                       storage->getDeclContext(),
+                                       /*isStatic*/false);
+    }
+
+    getterParams.push_back(ParameterList::create(TC.Context, selfDecl));
+  }
 
   // Add an index-forwarding clause.
   getterParams.push_back(buildIndexForwardingParamList(storage, {}));
@@ -1146,7 +1162,6 @@ static FuncDecl *completeLazyPropertyGetter(VarDecl *VD, VarDecl *Storage,
   // realize that they are in the getter function.
   InitValue->walk(RecontextualizeClosures(Get));
 
-
   Pattern *Tmp2PBDPattern = new (Ctx) NamedPattern(Tmp2VD, /*implicit*/true);
   Tmp2PBDPattern = new (Ctx) TypedPattern(Tmp2PBDPattern,
                                           TypeLoc::withoutLoc(VD->getType()),

test/NameBinding/name_lookup.swift

@@ -528,7 +528,7 @@ class r21677702 {
 // <rdar://problem/16954496> lazy properties must use "self." in their body, and can weirdly refer to class variables directly
 class r16954496 {
   func bar() {}
-  lazy var x: Array<(r16954496) -> () -> Void> = [bar]
+  lazy var x: Array<() -> Void> = [bar]
 }
 
 

test/NameBinding/scope_map.swift

@@ -460,8 +460,7 @@ class LazyProperties {
 // CHECK-SEARCHES-NEXT:     ClassDecl name=LazyProperties
 // CHECK-SEARCHES-NEXT:       Initializer PatternBinding {{.*}} #0
 
-// FIXME: Re-enable the binding below
-// CHECK-SEARCHES-NOT: Local bindings: self
+// CHECK-SEARCHES-NEXT: Local bindings: self
 
 // CHECK-SEARCHES-LABEL: ***Complete scope map***
 // CHECK-SEARCHES-NEXT: SourceFile {{.*}} '{{.*}}scope_map.swift' [1:1 - [[EOF:[0-9]+:[0-9]+]]] unexpanded

test/decl/var/lazy_properties.swift

@@ -1,4 +1,5 @@
-// RUN: %target-typecheck-verify-swift -parse-as-library
+// RUN: %target-typecheck-verify-swift -parse-as-library -swift-version 3
+// RUN: %target-typecheck-verify-swift -parse-as-library -swift-version 4
 
 lazy func lazy_func() {} // expected-error {{'lazy' may only be used on 'var' declarations}} {{1-6=}}
 
@@ -72,7 +73,8 @@ struct StructTest {
 
 // <rdar://problem/16889110> capture lists in lazy member properties cannot use self
 class CaptureListInLazyProperty {
-  lazy var closure: () -> Int = { [weak self] in return self!.i }
+  lazy var closure1 = { [weak self] in return self!.i }
+  lazy var closure2: () -> Int = { [weak self] in return self!.i }
   var i = 42
 }
 
@@ -119,3 +121,35 @@ struct Construction {
 class Constructor {
   lazy var myQ = Construction(x: 3)
 }
+
+
+// Problems with self references
+class BaseClass {
+  var baseInstanceProp = 42
+  static var baseStaticProp = 42
+}
+
+class ReferenceSelfInLazyProperty : BaseClass {
+  lazy var refs = (i, f())
+  lazy var trefs: (Int, Int) = (i, f())
+
+  lazy var qrefs = (self.i, self.f())
+  lazy var qtrefs: (Int, Int) = (self.i, self.f())
+
+  lazy var crefs = { (i, f()) }()
+  lazy var ctrefs: (Int, Int) = { (i, f()) }()
+
+  lazy var cqrefs = { (self.i, self.f()) }()
+  lazy var cqtrefs: (Int, Int) = { (self.i, self.f()) }()
+
+  lazy var mrefs = { () -> (Int, Int) in return (i, f()) }()
+  // expected-error@-1 {{call to method 'f' in closure requires explicit 'self.' to make capture semantics explicit}}
+  // expected-error@-2 {{reference to property 'i' in closure requires explicit 'self.' to make capture semantics explicit}}
+  lazy var mtrefs: (Int, Int) = { return (i, f()) }()
+
+  lazy var mqrefs = { () -> (Int, Int) in (self.i, self.f()) }()
+  lazy var mqtrefs: (Int, Int) = { return (self.i, self.f()) }()
+
+  var i = 42
+  func f() -> Int { return 0 }
+}