Skip to content

Commit 55ebd07

Browse files
committed
[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.
1 parent a341c86 commit 55ebd07

File tree

13 files changed

+59
-31
lines changed

13 files changed

+59
-31
lines changed

include/swift/AST/Decl.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6730,9 +6730,13 @@ class VarDecl : public AbstractStorageDecl {
67306730
/// An auxiliary variable is one that is synthesized by the compiler to
67316731
/// support this VarDecl, such as synthesized property wrapper variables.
67326732
///
6733+
/// \param forNameLookup If \c true, will only visit auxiliary variables that
6734+
/// may appear in name lookup results.
6735+
///
67336736
/// \note this function only visits auxiliary variables that are not part of
67346737
/// the AST.
6735-
void visitAuxiliaryVars(llvm::function_ref<void(VarDecl *)>) const;
6738+
void visitAuxiliaryVars(bool forNameLookup,
6739+
llvm::function_ref<void(VarDecl *)>) const;
67366740

67376741
/// Is this the synthesized storage for a 'lazy' property?
67386742
bool isLazyStorageProperty() const {

lib/AST/Decl.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8768,14 +8768,21 @@ bool VarDecl::hasStorageOrWrapsStorage() const {
87688768
}
87698769

87708770
void VarDecl::visitAuxiliaryVars(
8771-
llvm::function_ref<void(VarDecl *)> visit) const {
8771+
bool forNameLookup, llvm::function_ref<void(VarDecl *)> visit) const {
87728772
if (getDeclContext()->isTypeContext() ||
87738773
(isImplicit() && !isa<ParamDecl>(this)))
87748774
return;
87758775

8776-
if (getAttrs().hasAttribute<LazyAttr>()) {
8777-
if (auto *backingVar = getLazyStorageProperty())
8778-
visit(backingVar);
8776+
// The backing storage for a lazy property is not accessible to name lookup.
8777+
// This is not only a matter of hiding implementation details, but also
8778+
// correctness since triggering LazyStoragePropertyRequest currently eagerly
8779+
// requests the interface type for the var, which could result in incorrectly
8780+
// type-checking a lazy local independently of its surrounding closure.
8781+
if (!forNameLookup) {
8782+
if (getAttrs().hasAttribute<LazyAttr>()) {
8783+
if (auto *backingVar = getLazyStorageProperty())
8784+
visit(backingVar);
8785+
}
87798786
}
87808787

87818788
if (getAttrs().hasAttribute<CustomAttr>() || hasImplicitPropertyWrapper()) {

lib/AST/UnqualifiedLookup.cpp

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -708,12 +708,13 @@ bool ASTScopeDeclConsumerForUnqualifiedLookup::consume(
708708
bool foundMatch = false;
709709
if (auto *varDecl = dyn_cast<VarDecl>(value)) {
710710
// Check if the name matches any auxiliary decls not in the AST
711-
varDecl->visitAuxiliaryVars([&](VarDecl *auxiliaryVar) {
712-
if (auxiliaryVar->ValueDecl::getName().matchesRef(fullName)) {
713-
value = auxiliaryVar;
714-
foundMatch = true;
715-
}
716-
});
711+
varDecl->visitAuxiliaryVars(
712+
/*forNameLookup*/ true, [&](VarDecl *auxiliaryVar) {
713+
if (auxiliaryVar->ValueDecl::getName().matchesRef(fullName)) {
714+
value = auxiliaryVar;
715+
foundMatch = true;
716+
}
717+
});
717718
}
718719

719720
if (!foundMatch)
@@ -918,13 +919,14 @@ class ASTScopeDeclConsumerForLocalLookup
918919
bool foundMatch = false;
919920
if (auto *varDecl = dyn_cast<VarDecl>(value)) {
920921
// Check if the name matches any auxiliary decls not in the AST
921-
varDecl->visitAuxiliaryVars([&](VarDecl *auxiliaryVar) {
922-
if (name.isSimpleName(auxiliaryVar->getName())
923-
&& hasCorrectABIRole(auxiliaryVar)) {
924-
results.push_back(auxiliaryVar);
925-
foundMatch = true;
926-
}
927-
});
922+
varDecl->visitAuxiliaryVars(
923+
/*forNameLookup*/ true, [&](VarDecl *auxiliaryVar) {
924+
if (name.isSimpleName(auxiliaryVar->getName()) &&
925+
hasCorrectABIRole(auxiliaryVar)) {
926+
results.push_back(auxiliaryVar);
927+
foundMatch = true;
928+
}
929+
});
928930
}
929931

930932
if (!foundMatch && value->getName().matchesRef(name)

lib/SILGen/SILGenDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1716,7 +1716,7 @@ void SILGenFunction::visitVarDecl(VarDecl *D) {
17161716
}
17171717

17181718
// Emit lazy and property wrapper backing storage.
1719-
D->visitAuxiliaryVars([&](VarDecl *var) {
1719+
D->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *var) {
17201720
if (auto *patternBinding = var->getParentPatternBinding())
17211721
visitPatternBindingDecl(patternBinding);
17221722

lib/SILGen/SILGenProlog.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1064,7 +1064,7 @@ class ArgumentInitHelper {
10641064
void emitParam(ParamDecl *PD) {
10651065
// Register any auxiliary declarations for the parameter to be
10661066
// visited later.
1067-
PD->visitAuxiliaryVars([&](VarDecl *localVar) {
1067+
PD->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *localVar) {
10681068
SGF.LocalAuxiliaryDecls.push_back(localVar);
10691069
});
10701070

lib/Sema/LookupVisibleDecls.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1224,7 +1224,8 @@ class ASTScopeVisibleDeclConsumer
12241224
// auxiliary variables (unless 'var' is a closure param).
12251225
(void)var->getPropertyWrapperBackingPropertyType();
12261226
}
1227-
var->visitAuxiliaryVars([&](VarDecl *auxVar) { foundDecl(auxVar); });
1227+
var->visitAuxiliaryVars(/*forNameLookup*/ true,
1228+
[&](VarDecl *auxVar) { foundDecl(auxVar); });
12281229
}
12291230
// NOTE: We don't call Decl::visitAuxiliaryDecls here since peer decls of
12301231
// local decls should not show up in lookup results.

lib/Sema/TypeCheckDeclPrimary.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2382,8 +2382,9 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
23822382
checkGlobalIsolation(VD);
23832383

23842384
// Visit auxiliary decls first
2385-
VD->visitAuxiliaryVars(
2386-
[&](VarDecl *var) { this->visitBoundVariable(var); });
2385+
VD->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *var) {
2386+
this->visitBoundVariable(var);
2387+
});
23872388

23882389
// Reject cases where this is a variable that has storage but it isn't
23892390
// allowed.
@@ -4118,9 +4119,10 @@ void TypeChecker::checkParameterList(ParameterList *params,
41184119

41194120
if (!param->isInvalid()) {
41204121
auto *SF = owner->getParentSourceFile();
4121-
param->visitAuxiliaryVars([&](VarDecl *auxiliaryDecl) {
4122-
if (!isa<ParamDecl>(auxiliaryDecl))
4123-
DeclChecker(SF->getASTContext(), SF).visitBoundVariable(auxiliaryDecl);
4122+
auto &ctx = SF->getASTContext();
4123+
param->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *auxVar) {
4124+
if (!isa<ParamDecl>(auxVar))
4125+
DeclChecker(ctx, SF).visitBoundVariable(auxVar);
41244126
});
41254127
}
41264128

lib/Sema/TypeCheckStmt.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,9 @@ namespace {
153153

154154
// Auxiliary decls need to have their contexts adjusted too.
155155
if (auto *VD = dyn_cast<VarDecl>(D)) {
156-
VD->visitAuxiliaryVars(
157-
[&](VarDecl *D) { D->setDeclContext(ParentDC); });
156+
VD->visitAuxiliaryVars(/*forNameLookup*/ false, [&](VarDecl *D) {
157+
D->setDeclContext(ParentDC);
158+
});
158159
}
159160

160161
// We don't currently support peer macro declarations in local contexts,

test/SILGen/lazy_locals.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,9 @@ func lazyLocalInClosure() {
5050
lazy var x = 0
5151
return x
5252
}
53+
// https://github.com/swiftlang/swift/issues/83627
54+
let _: (Int) -> Void = { (arg: _) in
55+
lazy var val = arg
56+
_ = val
57+
}
5358
}

test/decl/var/lazy_properties.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,12 @@ class LazyVarContainer {
204204
}
205205
}
206206

207+
// The backing storage for lazy locals just isn't exposed through name lookup.
208+
func testLazyLocal() {
209+
lazy var foo = 0
210+
print($__lazy_storage_$_foo) // expected-error {{cannot find '$__lazy_storage_$_foo' in scope}}
211+
}
212+
207213
// Make sure we can still access a synthesized variable with the same name as a lazy storage variable
208214
// i.e. $__lazy_storage_$_{property_name} when using property wrapper where the property name is
209215
// '__lazy_storage_$_{property_name}'.

0 commit comments

Comments
 (0)