Merge pull request #85135 from hamishknight/lazy-fix

[AST] Avoid exposing `lazy` local storage var to name lookup

Author: hamishknight

include/swift/AST/Decl.h

@@ -6724,13 +6724,18 @@ class VarDecl : public AbstractStorageDecl {
     Bits.VarDecl.IsDebuggerVar = IsDebuggerVar;
   }
 
-  /// Visit all auxiliary declarations to this VarDecl.
+  /// Visit all auxiliary variables for this VarDecl.
   ///
-  /// An auxiliary declaration is a declaration synthesized by the compiler to support
-  /// this VarDecl, such as synthesized property wrapper variables.
+  /// An auxiliary variable is one that is synthesized by the compiler to
+  /// support this VarDecl, such as synthesized property wrapper variables.
   ///
-  /// \note this function only visits auxiliary decls that are not part of the AST.
-  void visitAuxiliaryDecls(llvm::function_ref<void(VarDecl *)>) const;
+  /// \param forNameLookup If \c true, will only visit auxiliary variables that
+  /// may appear in name lookup results.
+  ///
+  /// \note this function only visits auxiliary variables that are not part of
+  /// the AST.
+  void visitAuxiliaryVars(bool forNameLookup,
+                          llvm::function_ref<void(VarDecl *)>) const;
 
   /// Is this the synthesized storage for a 'lazy' property?
   bool isLazyStorageProperty() const {

lib/AST/Decl.cpp

@@ -507,7 +507,7 @@ void Decl::visitAuxiliaryDecls(
     }
   }
 
-  // FIXME: fold VarDecl::visitAuxiliaryDecls into this.
+  // FIXME: fold VarDecl::visitAuxiliaryVars into this.
 }
 
 void Decl::forEachAttachedMacro(MacroRole role,
@@ -8783,14 +8783,22 @@ bool VarDecl::hasStorageOrWrapsStorage() const {
   return false;
 }
 
-void VarDecl::visitAuxiliaryDecls(llvm::function_ref<void(VarDecl *)> visit) const {
+void VarDecl::visitAuxiliaryVars(
+    bool forNameLookup, llvm::function_ref<void(VarDecl *)> visit) const {
   if (getDeclContext()->isTypeContext() ||
       (isImplicit() && !isa<ParamDecl>(this)))
     return;
 
-  if (getAttrs().hasAttribute<LazyAttr>()) {
-    if (auto *backingVar = getLazyStorageProperty())
-      visit(backingVar);
+  // The backing storage for a lazy property is not accessible to name lookup.
+  // This is not only a matter of hiding implementation details, but also
+  // correctness since triggering LazyStoragePropertyRequest currently eagerly
+  // requests the interface type for the var, which could result in incorrectly
+  // type-checking a lazy local independently of its surrounding closure.
+  if (!forNameLookup) {
+    if (getAttrs().hasAttribute<LazyAttr>()) {
+      if (auto *backingVar = getLazyStorageProperty())
+        visit(backingVar);
+    }
   }
 
   if (getAttrs().hasAttribute<CustomAttr>() || hasImplicitPropertyWrapper()) {

lib/AST/UnqualifiedLookup.cpp

@@ -726,12 +726,13 @@ bool ASTScopeDeclConsumerForUnqualifiedLookup::consume(
       bool foundMatch = false;
       if (auto *varDecl = dyn_cast<VarDecl>(value)) {
         // Check if the name matches any auxiliary decls not in the AST
-        varDecl->visitAuxiliaryDecls([&](VarDecl *auxiliaryVar) {
-          if (auxiliaryVar->ValueDecl::getName().matchesRef(fullName)) {
-            value = auxiliaryVar;
-            foundMatch = true;
-          }
-        });
+        varDecl->visitAuxiliaryVars(
+            /*forNameLookup*/ true, [&](VarDecl *auxiliaryVar) {
+              if (auxiliaryVar->ValueDecl::getName().matchesRef(fullName)) {
+                value = auxiliaryVar;
+                foundMatch = true;
+              }
+            });
       }
 
       if (!foundMatch)
@@ -938,13 +939,14 @@ class ASTScopeDeclConsumerForLocalLookup
       bool foundMatch = false;
       if (auto *varDecl = dyn_cast<VarDecl>(value)) {
         // Check if the name matches any auxiliary decls not in the AST
-        varDecl->visitAuxiliaryDecls([&](VarDecl *auxiliaryVar) {
-          if (name.isSimpleName(auxiliaryVar->getName())
-                && hasCorrectABIRole(auxiliaryVar)) {
-            results.push_back(auxiliaryVar);
-            foundMatch = true;
-          }
-        });
+        varDecl->visitAuxiliaryVars(
+            /*forNameLookup*/ true, [&](VarDecl *auxiliaryVar) {
+              if (name.isSimpleName(auxiliaryVar->getName()) &&
+                  hasCorrectABIRole(auxiliaryVar)) {
+                results.push_back(auxiliaryVar);
+                foundMatch = true;
+              }
+            });
       }
 
       if (!foundMatch && value->getName().matchesRef(name.getFullName())

lib/SILGen/SILGenDecl.cpp

@@ -1727,7 +1727,7 @@ void SILGenFunction::visitVarDecl(VarDecl *D) {
   }
 
   // Emit lazy and property wrapper backing storage.
-  D->visitAuxiliaryDecls([&](VarDecl *var) {
+  D->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *var) {
     if (auto *patternBinding = var->getParentPatternBinding())
       visitPatternBindingDecl(patternBinding);
 

lib/SILGen/SILGenProlog.cpp

@@ -1064,7 +1064,7 @@ class ArgumentInitHelper {
   void emitParam(ParamDecl *PD) {
     // Register any auxiliary declarations for the parameter to be
     // visited later.
-    PD->visitAuxiliaryDecls([&](VarDecl *localVar) {
+    PD->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *localVar) {
       SGF.LocalAuxiliaryDecls.push_back(localVar);
     });
 

lib/Sema/LookupVisibleDecls.cpp

@@ -1224,8 +1224,8 @@ class ASTScopeVisibleDeclConsumer
           // auxiliary variables (unless 'var' is a closure param).
           (void)var->getPropertyWrapperBackingPropertyType();
         }
-        var->visitAuxiliaryDecls(
-            [&](VarDecl *auxVar) { foundDecl(auxVar); });
+        var->visitAuxiliaryVars(/*forNameLookup*/ true,
+                                [&](VarDecl *auxVar) { foundDecl(auxVar); });
       }
       // NOTE: We don't call Decl::visitAuxiliaryDecls here since peer decls of
       // local decls should not show up in lookup results.

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -2388,7 +2388,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     checkGlobalIsolation(VD);
 
     // Visit auxiliary decls first
-    VD->visitAuxiliaryDecls([&](VarDecl *var) {
+    VD->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *var) {
       this->visitBoundVariable(var);
     });
 
@@ -4125,9 +4125,10 @@ void TypeChecker::checkParameterList(ParameterList *params,
 
     if (!param->isInvalid()) {
       auto *SF = owner->getParentSourceFile();
-      param->visitAuxiliaryDecls([&](VarDecl *auxiliaryDecl) {
-        if (!isa<ParamDecl>(auxiliaryDecl))
-          DeclChecker(SF->getASTContext(), SF).visitBoundVariable(auxiliaryDecl);
+      auto &ctx = SF->getASTContext();
+      param->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *auxVar) {
+        if (!isa<ParamDecl>(auxVar))
+          DeclChecker(ctx, SF).visitBoundVariable(auxVar);
       });
     }
 

lib/Sema/TypeCheckStmt.cpp

@@ -153,7 +153,7 @@ namespace {
 
       // Auxiliary decls need to have their contexts adjusted too.
       if (auto *VD = dyn_cast<VarDecl>(D)) {
-        VD->visitAuxiliaryDecls([&](VarDecl *D) {
+        VD->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *D) {
           D->setDeclContext(ParentDC);
         });
       }

test/SILGen/lazy_locals.swift

@@ -50,4 +50,9 @@ func lazyLocalInClosure() {
     lazy var x = 0
     return x
   }
+  // https://github.com/swiftlang/swift/issues/83627
+  let _: (Int) -> Void = { (arg: _) in
+    lazy var val = arg
+    _ = val
+  }
 }

test/decl/var/lazy_properties.swift

@@ -204,6 +204,12 @@ class LazyVarContainer {
   }
 }
 
+// The backing storage for lazy locals just isn't exposed through name lookup.
+func testLazyLocal() {
+  lazy var foo = 0
+  print($__lazy_storage_$_foo) // expected-error {{cannot find '$__lazy_storage_$_foo' in scope}}
+}
+
 // Make sure we can still access a synthesized variable with the same name as a lazy storage variable
 // i.e. $__lazy_storage_$_{property_name} when using property wrapper where the property name is 
 // '__lazy_storage_$_{property_name}'.