From a341c86f3ccbe064e79a94dfd5c18365d90a5315 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sat, 25 Oct 2025 15:07:11 +0100 Subject: [PATCH 1/2] [AST] NFC: Rename `VarDecl::visitAuxiliaryDecls` -> `visitAuxiliaryVars` Make it more obvious it's different to `Decl::visitAuxiliaryDecls`. --- include/swift/AST/Decl.h | 11 ++++++----- lib/AST/Decl.cpp | 5 +++-- lib/AST/UnqualifiedLookup.cpp | 4 ++-- lib/SILGen/SILGenDecl.cpp | 2 +- lib/SILGen/SILGenProlog.cpp | 2 +- lib/Sema/LookupVisibleDecls.cpp | 3 +-- lib/Sema/TypeCheckDeclPrimary.cpp | 7 +++---- lib/Sema/TypeCheckStmt.cpp | 5 ++--- 8 files changed, 19 insertions(+), 20 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 7c41541229f61..184df30ffe0e3 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -6725,13 +6725,14 @@ 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) const; + /// \note this function only visits auxiliary variables that are not part of + /// the AST. + void visitAuxiliaryVars(llvm::function_ref) const; /// Is this the synthesized storage for a 'lazy' property? bool isLazyStorageProperty() const { diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 4887071cee5e6..a0e801d494f65 100644 --- a/lib/AST/Decl.cpp +++ b/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, @@ -8767,7 +8767,8 @@ bool VarDecl::hasStorageOrWrapsStorage() const { return false; } -void VarDecl::visitAuxiliaryDecls(llvm::function_ref visit) const { +void VarDecl::visitAuxiliaryVars( + llvm::function_ref visit) const { if (getDeclContext()->isTypeContext() || (isImplicit() && !isa(this))) return; diff --git a/lib/AST/UnqualifiedLookup.cpp b/lib/AST/UnqualifiedLookup.cpp index 45f6ed4b3fcb1..bfdcf5c724f8f 100644 --- a/lib/AST/UnqualifiedLookup.cpp +++ b/lib/AST/UnqualifiedLookup.cpp @@ -708,7 +708,7 @@ bool ASTScopeDeclConsumerForUnqualifiedLookup::consume( bool foundMatch = false; if (auto *varDecl = dyn_cast(value)) { // Check if the name matches any auxiliary decls not in the AST - varDecl->visitAuxiliaryDecls([&](VarDecl *auxiliaryVar) { + varDecl->visitAuxiliaryVars([&](VarDecl *auxiliaryVar) { if (auxiliaryVar->ValueDecl::getName().matchesRef(fullName)) { value = auxiliaryVar; foundMatch = true; @@ -918,7 +918,7 @@ class ASTScopeDeclConsumerForLocalLookup bool foundMatch = false; if (auto *varDecl = dyn_cast(value)) { // Check if the name matches any auxiliary decls not in the AST - varDecl->visitAuxiliaryDecls([&](VarDecl *auxiliaryVar) { + varDecl->visitAuxiliaryVars([&](VarDecl *auxiliaryVar) { if (name.isSimpleName(auxiliaryVar->getName()) && hasCorrectABIRole(auxiliaryVar)) { results.push_back(auxiliaryVar); diff --git a/lib/SILGen/SILGenDecl.cpp b/lib/SILGen/SILGenDecl.cpp index 65332d83edbe0..89b5ab3e90c34 100644 --- a/lib/SILGen/SILGenDecl.cpp +++ b/lib/SILGen/SILGenDecl.cpp @@ -1716,7 +1716,7 @@ void SILGenFunction::visitVarDecl(VarDecl *D) { } // Emit lazy and property wrapper backing storage. - D->visitAuxiliaryDecls([&](VarDecl *var) { + D->visitAuxiliaryVars([&](VarDecl *var) { if (auto *patternBinding = var->getParentPatternBinding()) visitPatternBindingDecl(patternBinding); diff --git a/lib/SILGen/SILGenProlog.cpp b/lib/SILGen/SILGenProlog.cpp index cc1f802054211..39065470dc12e 100644 --- a/lib/SILGen/SILGenProlog.cpp +++ b/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([&](VarDecl *localVar) { SGF.LocalAuxiliaryDecls.push_back(localVar); }); diff --git a/lib/Sema/LookupVisibleDecls.cpp b/lib/Sema/LookupVisibleDecls.cpp index 1890c683b744e..d92f84c817ac8 100644 --- a/lib/Sema/LookupVisibleDecls.cpp +++ b/lib/Sema/LookupVisibleDecls.cpp @@ -1224,8 +1224,7 @@ class ASTScopeVisibleDeclConsumer // auxiliary variables (unless 'var' is a closure param). (void)var->getPropertyWrapperBackingPropertyType(); } - var->visitAuxiliaryDecls( - [&](VarDecl *auxVar) { foundDecl(auxVar); }); + var->visitAuxiliaryVars([&](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. diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index fa1a9f71ac021..7cb8ee6cdfff8 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -2382,9 +2382,8 @@ class DeclChecker : public DeclVisitor { checkGlobalIsolation(VD); // Visit auxiliary decls first - VD->visitAuxiliaryDecls([&](VarDecl *var) { - this->visitBoundVariable(var); - }); + VD->visitAuxiliaryVars( + [&](VarDecl *var) { this->visitBoundVariable(var); }); // Reject cases where this is a variable that has storage but it isn't // allowed. @@ -4119,7 +4118,7 @@ void TypeChecker::checkParameterList(ParameterList *params, if (!param->isInvalid()) { auto *SF = owner->getParentSourceFile(); - param->visitAuxiliaryDecls([&](VarDecl *auxiliaryDecl) { + param->visitAuxiliaryVars([&](VarDecl *auxiliaryDecl) { if (!isa(auxiliaryDecl)) DeclChecker(SF->getASTContext(), SF).visitBoundVariable(auxiliaryDecl); }); diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp index 0bad17c728222..32e40e7e1fdb3 100644 --- a/lib/Sema/TypeCheckStmt.cpp +++ b/lib/Sema/TypeCheckStmt.cpp @@ -153,9 +153,8 @@ namespace { // Auxiliary decls need to have their contexts adjusted too. if (auto *VD = dyn_cast(D)) { - VD->visitAuxiliaryDecls([&](VarDecl *D) { - D->setDeclContext(ParentDC); - }); + VD->visitAuxiliaryVars( + [&](VarDecl *D) { D->setDeclContext(ParentDC); }); } // We don't currently support peer macro declarations in local contexts, From 55ebd0774e6d710fa3d8a0cb79bc78518be5a86b Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sat, 25 Oct 2025 15:07:11 +0100 Subject: [PATCH 2/2] [AST] Avoid exposing `lazy` local storage var to name lookup We already reject attempts to reference this for `lazy` properties. For `lazy` locals let's just not expose it to name lookup to begin with. This ensures we don't attempt to prematurely kick the interface type computation for the var, fixing a couple of crashers. --- include/swift/AST/Decl.h | 6 +++- lib/AST/Decl.cpp | 15 +++++++--- lib/AST/UnqualifiedLookup.cpp | 28 ++++++++++--------- lib/SILGen/SILGenDecl.cpp | 2 +- lib/SILGen/SILGenProlog.cpp | 2 +- lib/Sema/LookupVisibleDecls.cpp | 3 +- lib/Sema/TypeCheckDeclPrimary.cpp | 12 ++++---- lib/Sema/TypeCheckStmt.cpp | 5 ++-- test/SILGen/lazy_locals.swift | 5 ++++ test/decl/var/lazy_properties.swift | 6 ++++ ...ageDecl-getValueInterfaceType-c9bb54.swift | 2 +- ...nstraintSystem-getClosureType-dc5f07.swift | 2 +- .../getParameterAt-0771c5.swift | 2 +- 13 files changed, 59 insertions(+), 31 deletions(-) rename validation-test/{compiler_crashers => compiler_crashers_fixed}/AbstractStorageDecl-getValueInterfaceType-c9bb54.swift (82%) rename validation-test/{compiler_crashers => compiler_crashers_fixed}/ConstraintSystem-getClosureType-dc5f07.swift (82%) rename validation-test/{compiler_crashers => compiler_crashers_fixed}/getParameterAt-0771c5.swift (71%) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 184df30ffe0e3..642b4e121f46f 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -6730,9 +6730,13 @@ class VarDecl : public AbstractStorageDecl { /// An auxiliary variable is one that is synthesized by the compiler to /// support this VarDecl, such as synthesized property wrapper variables. /// + /// \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(llvm::function_ref) const; + void visitAuxiliaryVars(bool forNameLookup, + llvm::function_ref) const; /// Is this the synthesized storage for a 'lazy' property? bool isLazyStorageProperty() const { diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index a0e801d494f65..b33eda32e9349 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -8768,14 +8768,21 @@ bool VarDecl::hasStorageOrWrapsStorage() const { } void VarDecl::visitAuxiliaryVars( - llvm::function_ref visit) const { + bool forNameLookup, llvm::function_ref visit) const { if (getDeclContext()->isTypeContext() || (isImplicit() && !isa(this))) return; - if (getAttrs().hasAttribute()) { - 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()) { + if (auto *backingVar = getLazyStorageProperty()) + visit(backingVar); + } } if (getAttrs().hasAttribute() || hasImplicitPropertyWrapper()) { diff --git a/lib/AST/UnqualifiedLookup.cpp b/lib/AST/UnqualifiedLookup.cpp index bfdcf5c724f8f..40d9dcabb074b 100644 --- a/lib/AST/UnqualifiedLookup.cpp +++ b/lib/AST/UnqualifiedLookup.cpp @@ -708,12 +708,13 @@ bool ASTScopeDeclConsumerForUnqualifiedLookup::consume( bool foundMatch = false; if (auto *varDecl = dyn_cast(value)) { // Check if the name matches any auxiliary decls not in the AST - varDecl->visitAuxiliaryVars([&](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) @@ -918,13 +919,14 @@ class ASTScopeDeclConsumerForLocalLookup bool foundMatch = false; if (auto *varDecl = dyn_cast(value)) { // Check if the name matches any auxiliary decls not in the AST - varDecl->visitAuxiliaryVars([&](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) diff --git a/lib/SILGen/SILGenDecl.cpp b/lib/SILGen/SILGenDecl.cpp index 89b5ab3e90c34..8f1bd9c90a835 100644 --- a/lib/SILGen/SILGenDecl.cpp +++ b/lib/SILGen/SILGenDecl.cpp @@ -1716,7 +1716,7 @@ void SILGenFunction::visitVarDecl(VarDecl *D) { } // Emit lazy and property wrapper backing storage. - D->visitAuxiliaryVars([&](VarDecl *var) { + D->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *var) { if (auto *patternBinding = var->getParentPatternBinding()) visitPatternBindingDecl(patternBinding); diff --git a/lib/SILGen/SILGenProlog.cpp b/lib/SILGen/SILGenProlog.cpp index 39065470dc12e..da0ea8e7ffad1 100644 --- a/lib/SILGen/SILGenProlog.cpp +++ b/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->visitAuxiliaryVars([&](VarDecl *localVar) { + PD->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *localVar) { SGF.LocalAuxiliaryDecls.push_back(localVar); }); diff --git a/lib/Sema/LookupVisibleDecls.cpp b/lib/Sema/LookupVisibleDecls.cpp index d92f84c817ac8..019505669720a 100644 --- a/lib/Sema/LookupVisibleDecls.cpp +++ b/lib/Sema/LookupVisibleDecls.cpp @@ -1224,7 +1224,8 @@ class ASTScopeVisibleDeclConsumer // auxiliary variables (unless 'var' is a closure param). (void)var->getPropertyWrapperBackingPropertyType(); } - var->visitAuxiliaryVars([&](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. diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index 7cb8ee6cdfff8..1ccd1e02e2ddb 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -2382,8 +2382,9 @@ class DeclChecker : public DeclVisitor { checkGlobalIsolation(VD); // Visit auxiliary decls first - VD->visitAuxiliaryVars( - [&](VarDecl *var) { this->visitBoundVariable(var); }); + VD->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *var) { + this->visitBoundVariable(var); + }); // Reject cases where this is a variable that has storage but it isn't // allowed. @@ -4118,9 +4119,10 @@ void TypeChecker::checkParameterList(ParameterList *params, if (!param->isInvalid()) { auto *SF = owner->getParentSourceFile(); - param->visitAuxiliaryVars([&](VarDecl *auxiliaryDecl) { - if (!isa(auxiliaryDecl)) - DeclChecker(SF->getASTContext(), SF).visitBoundVariable(auxiliaryDecl); + auto &ctx = SF->getASTContext(); + param->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *auxVar) { + if (!isa(auxVar)) + DeclChecker(ctx, SF).visitBoundVariable(auxVar); }); } diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp index 32e40e7e1fdb3..f2b2222a65644 100644 --- a/lib/Sema/TypeCheckStmt.cpp +++ b/lib/Sema/TypeCheckStmt.cpp @@ -153,8 +153,9 @@ namespace { // Auxiliary decls need to have their contexts adjusted too. if (auto *VD = dyn_cast(D)) { - VD->visitAuxiliaryVars( - [&](VarDecl *D) { D->setDeclContext(ParentDC); }); + VD->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *D) { + D->setDeclContext(ParentDC); + }); } // We don't currently support peer macro declarations in local contexts, diff --git a/test/SILGen/lazy_locals.swift b/test/SILGen/lazy_locals.swift index 5e5251371ca5f..2de3b73e4b273 100644 --- a/test/SILGen/lazy_locals.swift +++ b/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 + } } diff --git a/test/decl/var/lazy_properties.swift b/test/decl/var/lazy_properties.swift index 7e1e7e445986d..1691a10e369b3 100644 --- a/test/decl/var/lazy_properties.swift +++ b/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}'. diff --git a/validation-test/compiler_crashers/AbstractStorageDecl-getValueInterfaceType-c9bb54.swift b/validation-test/compiler_crashers_fixed/AbstractStorageDecl-getValueInterfaceType-c9bb54.swift similarity index 82% rename from validation-test/compiler_crashers/AbstractStorageDecl-getValueInterfaceType-c9bb54.swift rename to validation-test/compiler_crashers_fixed/AbstractStorageDecl-getValueInterfaceType-c9bb54.swift index 47ca8db7f5224..3ff0cf614f5b5 100644 --- a/validation-test/compiler_crashers/AbstractStorageDecl-getValueInterfaceType-c9bb54.swift +++ b/validation-test/compiler_crashers_fixed/AbstractStorageDecl-getValueInterfaceType-c9bb54.swift @@ -1,5 +1,5 @@ // {"kind":"typecheck","signature":"swift::AbstractStorageDecl::getValueInterfaceType() const","signatureAssert":"Assertion failed: (detail::isPresent(Val) && \"dyn_cast on a non-existent value\"), function dyn_cast"} -// RUN: not --crash %target-swift-frontend -typecheck %s +// RUN: not %target-swift-frontend -typecheck %s class a { } { diff --git a/validation-test/compiler_crashers/ConstraintSystem-getClosureType-dc5f07.swift b/validation-test/compiler_crashers_fixed/ConstraintSystem-getClosureType-dc5f07.swift similarity index 82% rename from validation-test/compiler_crashers/ConstraintSystem-getClosureType-dc5f07.swift rename to validation-test/compiler_crashers_fixed/ConstraintSystem-getClosureType-dc5f07.swift index 9ebe731aea572..a64e9b548545f 100644 --- a/validation-test/compiler_crashers/ConstraintSystem-getClosureType-dc5f07.swift +++ b/validation-test/compiler_crashers_fixed/ConstraintSystem-getClosureType-dc5f07.swift @@ -1,5 +1,5 @@ // {"kind":"typecheck","signature":"swift::constraints::ConstraintSystem::getClosureType(swift::ClosureExpr const*) const","signatureAssert":"Assertion failed: (result), function getClosureType"} -// RUN: not --crash %target-swift-frontend -typecheck %s +// RUN: not %target-swift-frontend -typecheck %s { lazy var a = if <#expression#> { diff --git a/validation-test/compiler_crashers/getParameterAt-0771c5.swift b/validation-test/compiler_crashers_fixed/getParameterAt-0771c5.swift similarity index 71% rename from validation-test/compiler_crashers/getParameterAt-0771c5.swift rename to validation-test/compiler_crashers_fixed/getParameterAt-0771c5.swift index bbc745a206bc2..8437ba9de3ffd 100644 --- a/validation-test/compiler_crashers/getParameterAt-0771c5.swift +++ b/validation-test/compiler_crashers_fixed/getParameterAt-0771c5.swift @@ -1,3 +1,3 @@ // {"kind":"typecheck","signature":"swift::getParameterAt(swift::ConcreteDeclRef, unsigned int)"} -// RUN: not --crash %target-swift-frontend -typecheck %s +// RUN: not %target-swift-frontend -typecheck %s func a func b [{ lazy var c = a(d var e = b