Skip to content

Commit 1904960

Browse files
committed
Sema: Fix a couple of problems with capture analysis and TopLevelCodeDecls
A TopLevelCodeDecl is a local context and any declarations inside of one must be treated as captures. Furthermore, all the TopLevelCodeDecl children of a source file are peers, and can see each other's bindings, so don't perform an exact match on the DeclContext. Part of <rdar://problem/23051362> / <https://bugs.swift.org/browse/SR-3528>.
1 parent 1c2ac8e commit 1904960

File tree

2 files changed

+69
-34
lines changed

2 files changed

+69
-34
lines changed

lib/Sema/TypeCheckCaptures.cpp

Lines changed: 52 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "swift/AST/GenericSignature.h"
2626
#include "swift/AST/ParameterList.h"
2727
#include "swift/AST/PrettyStackTrace.h"
28+
#include "swift/AST/SourceFile.h"
2829
#include "swift/AST/TypeWalker.h"
2930
#include "swift/Basic/Defer.h"
3031
#include "llvm/ADT/SmallPtrSet.h"
@@ -219,10 +220,14 @@ class FindCapturedVars : public ASTWalker {
219220
if (D->getBaseName() == Context.Id_dollarInterpolation)
220221
return { false, DRE };
221222

223+
// DC is the DeclContext where D was defined
224+
// CurDC is the DeclContext where D was referenced
225+
auto DC = D->getDeclContext();
226+
222227
// Capture the generic parameters of the decl, unless it's a
223228
// local declaration in which case we will pick up generic
224229
// parameter references transitively.
225-
if (!D->getDeclContext()->isLocalContext()) {
230+
if (!DC->isLocalContext()) {
226231
if (!ObjC || !D->isObjC() || isa<ConstructorDecl>(D)) {
227232
if (auto subMap = DRE->getDeclRef().getSubstitutions()) {
228233
for (auto type : subMap.getReplacementTypes()) {
@@ -232,40 +237,57 @@ class FindCapturedVars : public ASTWalker {
232237
}
233238
}
234239

235-
// DC is the DeclContext where D was defined
236-
// CurDC is the DeclContext where D was referenced
237-
auto DC = D->getDeclContext();
240+
// Don't "capture" type definitions at all.
241+
if (isa<TypeDecl>(D))
242+
return { false, DRE };
238243

239244
// A local reference is not a capture.
240-
if (CurDC == DC)
245+
if (CurDC == DC || isa<TopLevelCodeDecl>(CurDC))
241246
return { false, DRE };
242247

243248
auto TmpDC = CurDC;
244-
245-
if (!isa<TopLevelCodeDecl>(DC)) {
246-
while (TmpDC != nullptr) {
247-
if (TmpDC == DC)
248-
break;
249-
250-
// The initializer of a lazy property will eventually get
251-
// recontextualized into it, so treat it as if it's already there.
252-
if (auto init = dyn_cast<PatternBindingInitializer>(TmpDC)) {
253-
if (auto lazyVar = init->getInitializedLazyVar()) {
254-
// If we have a getter with a body, we're already re-parented
255-
// everything so pretend we're inside the getter.
256-
if (auto getter = lazyVar->getAccessor(AccessorKind::Get)) {
257-
if (getter->getBody(/*canSynthesize=*/false)) {
258-
TmpDC = getter;
259-
continue;
260-
}
249+
while (TmpDC != nullptr) {
250+
// Variables defined inside TopLevelCodeDecls are semantically
251+
// local variables. If the reference is not from the top level,
252+
// we have a capture.
253+
if (isa<TopLevelCodeDecl>(DC) &&
254+
(isa<SourceFile>(TmpDC) || isa<TopLevelCodeDecl>(TmpDC)))
255+
break;
256+
257+
if (TmpDC == DC)
258+
break;
259+
260+
// The initializer of a lazy property will eventually get
261+
// recontextualized into it, so treat it as if it's already there.
262+
if (auto init = dyn_cast<PatternBindingInitializer>(TmpDC)) {
263+
if (auto lazyVar = init->getInitializedLazyVar()) {
264+
// If we have a getter with a body, we're already re-parented
265+
// everything so pretend we're inside the getter.
266+
if (auto getter = lazyVar->getAccessor(AccessorKind::Get)) {
267+
if (getter->getBody(/*canSynthesize=*/false)) {
268+
TmpDC = getter;
269+
continue;
261270
}
262271
}
263272
}
273+
}
264274

265-
// We have an intervening nominal type context that is not the
266-
// declaration context, and the declaration context is not global.
267-
// This is not supported since nominal types cannot capture values.
268-
if (auto NTD = dyn_cast<NominalTypeDecl>(TmpDC)) {
275+
// We have an intervening nominal type context that is not the
276+
// declaration context, and the declaration context is not global.
277+
// This is not supported since nominal types cannot capture values.
278+
if (auto NTD = dyn_cast<NominalTypeDecl>(TmpDC)) {
279+
// Allow references to local functions from inside methods of a
280+
// local type, because if the local function has captures, we'll
281+
// diagnose them in SILGen. It's a bit unfortunate that we can't
282+
// ban this outright, but people rely on code like this working:
283+
//
284+
// do {
285+
// func local() {}
286+
// class C {
287+
// func method() { local() }
288+
// }
289+
// }
290+
if (!isa<FuncDecl>(D)) {
269291
if (DC->isLocalContext()) {
270292
Context.Diags.diagnose(DRE->getLoc(), diag::capture_across_type_decl,
271293
NTD->getDescriptiveKind(),
@@ -278,18 +300,14 @@ class FindCapturedVars : public ASTWalker {
278300
return { false, DRE };
279301
}
280302
}
281-
282-
TmpDC = TmpDC->getParent();
283303
}
284304

285-
// We walked all the way up to the root without finding the declaration,
286-
// so this is not a capture.
287-
if (TmpDC == nullptr)
288-
return { false, DRE };
305+
TmpDC = TmpDC->getParent();
289306
}
290307

291-
// Don't "capture" type definitions at all.
292-
if (isa<TypeDecl>(D))
308+
// We walked all the way up to the root without finding the declaration,
309+
// so this is not a capture.
310+
if (TmpDC == nullptr)
293311
return { false, DRE };
294312

295313
// Only capture var decls at global scope. Other things can be captured

test/expr/capture/nested_class.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,20 @@ struct StructWithInnerStruct {
3636
}
3737
}
3838
}
39+
40+
// Types cannot close over top-level guard bindings
41+
guard let x: Int = nil else { fatalError() }
42+
// expected-note@-1 {{'x' declared here}}
43+
44+
func getX() -> Int { return x }
45+
46+
class ClosesOverGuard { // expected-note {{type declared here}}
47+
func foo() {
48+
_ = x
49+
// expected-error@-1 {{class declaration cannot close over value 'x' defined in outer scope}}
50+
}
51+
52+
func bar() {
53+
_ = getX() // This is diagnosed by SILGen.
54+
}
55+
}

0 commit comments

Comments
 (0)