Skip to content

Commit 25ac8c0

Browse files
slavapestovslavapestov
authored
Merge pull request #26529 from slavapestov/silly-captures
Fix recent regression with pattern binding capture checking
2 parents 038cc60 + 2ef101c commit 25ac8c0

22 files changed

+113
-52
lines changed

‎lib/Sema/DebuggerTestingTransform.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ class DebuggerTestingTransform : public ASTWalker {
242242

243243
// Captures have to be computed after the closure is type-checked. This
244244
// ensures that the type checker can infer <noescape> for captured values.
245-
TC.computeCaptures(Closure);
245+
TypeChecker::computeCaptures(Closure);
246246

247247
return {false, FinalExpr};
248248
}

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2135,6 +2135,7 @@ class VarDeclUsageChecker : public ASTWalker {
21352135
// If this is a nested function with a capture list, mark any captured
21362136
// variables.
21372137
if (afd->isBodyTypeChecked()) {
2138+
TypeChecker::computeCaptures(afd);
21382139
for (const auto &capture : afd->getCaptureInfo().getCaptures())
21392140
addMark(capture.getDecl(), RK_Read|RK_Written);
21402141
} else {

‎lib/Sema/TypeCheckCaptures.cpp

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ using namespace swift;
3232
namespace {
3333

3434
class FindCapturedVars : public ASTWalker {
35-
TypeChecker &TC;
35+
ASTContext &Context;
3636
SmallVector<CapturedValue, 4> Captures;
3737
llvm::SmallDenseMap<ValueDecl*, unsigned, 4> captureEntryNumber;
3838
SourceLoc GenericParamCaptureLoc;
@@ -44,12 +44,12 @@ class FindCapturedVars : public ASTWalker {
4444
bool NoEscape, ObjC;
4545

4646
public:
47-
FindCapturedVars(TypeChecker &tc,
47+
FindCapturedVars(ASTContext &Context,
4848
SourceLoc CaptureLoc,
4949
DeclContext *CurDC,
5050
bool NoEscape,
5151
bool ObjC)
52-
: TC(tc), CaptureLoc(CaptureLoc), CurDC(CurDC),
52+
: Context(Context), CaptureLoc(CaptureLoc), CurDC(CurDC),
5353
NoEscape(NoEscape), ObjC(ObjC) {}
5454

5555
CaptureInfo getCaptureInfo() const {
@@ -71,7 +71,7 @@ class FindCapturedVars : public ASTWalker {
7171
if (Captures.empty())
7272
result.setCaptures(None);
7373
else
74-
result.setCaptures(TC.Context.AllocateCopy(Captures));
74+
result.setCaptures(Context.AllocateCopy(Captures));
7575

7676
return result;
7777
}
@@ -219,7 +219,7 @@ class FindCapturedVars : public ASTWalker {
219219
// can safely ignore them.
220220
// FIXME(TapExpr): This is probably caused by the scoping
221221
// algorithm's ignorance of TapExpr. We should fix that.
222-
if (D->getBaseName() == D->getASTContext().Id_dollarInterpolation)
222+
if (D->getBaseName() == Context.Id_dollarInterpolation)
223223
return { false, DRE };
224224

225225
// Capture the generic parameters of the decl, unless it's a
@@ -273,14 +273,14 @@ class FindCapturedVars : public ASTWalker {
273273
// This is not supported since nominal types cannot capture values.
274274
if (auto NTD = dyn_cast<NominalTypeDecl>(TmpDC)) {
275275
if (DC->isLocalContext()) {
276-
TC.diagnose(DRE->getLoc(), diag::capture_across_type_decl,
277-
NTD->getDescriptiveKind(),
278-
D->getBaseName().getIdentifier());
276+
Context.Diags.diagnose(DRE->getLoc(), diag::capture_across_type_decl,
277+
NTD->getDescriptiveKind(),
278+
D->getBaseName().getIdentifier());
279279

280-
TC.diagnose(NTD->getLoc(), diag::kind_declared_here,
281-
DescriptiveDeclKind::Type);
280+
NTD->diagnose(diag::kind_declared_here,
281+
DescriptiveDeclKind::Type);
282282

283-
TC.diagnose(D, diag::decl_declared_here, D->getFullName());
283+
D->diagnose(diag::decl_declared_here, D->getFullName());
284284
return { false, DRE };
285285
}
286286
}
@@ -328,7 +328,7 @@ class FindCapturedVars : public ASTWalker {
328328
}
329329

330330
void propagateCaptures(AnyFunctionRef innerClosure, SourceLoc captureLoc) {
331-
TC.computeCaptures(innerClosure);
331+
TypeChecker::computeCaptures(innerClosure);
332332

333333
auto &captureInfo = innerClosure.getCaptureInfo();
334334

@@ -597,7 +597,8 @@ void TypeChecker::computeCaptures(AnyFunctionRef AFR) {
597597
if (!AFR.getBody())
598598
return;
599599

600-
FindCapturedVars finder(*this,
600+
auto &Context = AFR.getAsDeclContext()->getASTContext();
601+
FindCapturedVars finder(Context,
601602
AFR.getLoc(),
602603
AFR.getAsDeclContext(),
603604
AFR.isKnownNoEscape(),
@@ -624,27 +625,29 @@ void TypeChecker::computeCaptures(AnyFunctionRef AFR) {
624625
if (AFD && finder.getGenericParamCaptureLoc().isValid()) {
625626
if (auto Clas = AFD->getParent()->getSelfClassDecl()) {
626627
if (Clas->usesObjCGenericsModel()) {
627-
diagnose(AFD->getLoc(),
628-
diag::objc_generic_extension_using_type_parameter);
628+
AFD->diagnose(diag::objc_generic_extension_using_type_parameter);
629629

630630
// If it's possible, suggest adding @objc.
631631
Optional<ForeignErrorConvention> errorConvention;
632632
if (!AFD->isObjC() &&
633633
isRepresentableInObjC(AFD, ObjCReason::MemberOfObjCMembersClass,
634634
errorConvention)) {
635-
diagnose(AFD->getLoc(),
635+
AFD->diagnose(
636636
diag::objc_generic_extension_using_type_parameter_try_objc)
637637
.fixItInsert(AFD->getAttributeInsertionLoc(false), "@objc ");
638638
}
639639

640-
diagnose(finder.getGenericParamCaptureLoc(),
641-
diag::objc_generic_extension_using_type_parameter_here);
640+
Context.Diags.diagnose(
641+
finder.getGenericParamCaptureLoc(),
642+
diag::objc_generic_extension_using_type_parameter_here);
642643
}
643644
}
644645
}
645646
}
646647

647648
void TypeChecker::checkPatternBindingCaptures(NominalTypeDecl *typeDecl) {
649+
auto &ctx = typeDecl->getASTContext();
650+
648651
for (auto member : typeDecl->getMembers()) {
649652
// Ignore everything other than PBDs.
650653
auto *PBD = dyn_cast<PatternBindingDecl>(member);
@@ -659,16 +662,16 @@ void TypeChecker::checkPatternBindingCaptures(NominalTypeDecl *typeDecl) {
659662
if (init == nullptr)
660663
continue;
661664

662-
FindCapturedVars finder(*this,
665+
FindCapturedVars finder(ctx,
663666
init->getLoc(),
664667
PBD->getInitContext(i),
665668
/*NoEscape=*/false,
666669
/*ObjC=*/false);
667670
init->walk(finder);
668671

669672
if (finder.getDynamicSelfCaptureLoc().isValid()) {
670-
diagnose(finder.getDynamicSelfCaptureLoc(),
671-
diag::dynamic_self_stored_property_init);
673+
ctx.Diags.diagnose(finder.getDynamicSelfCaptureLoc(),
674+
diag::dynamic_self_stored_property_init);
672675
}
673676

674677
auto captures = finder.getCaptureInfo();

‎lib/Sema/TypeCheckDecl.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2248,14 +2248,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
22482248
}
22492249
}
22502250

2251-
// FIXME: Temporary hack until capture computation has been request-ified.
2252-
if (VD->getDeclContext()->isLocalContext()) {
2253-
VD->visitOpaqueAccessors([&](AccessorDecl *accessor) {
2254-
if (accessor->isImplicit())
2255-
TC.definedFunctions.push_back(accessor);
2256-
});
2257-
}
2258-
22592251
// Under the Swift 3 inference rules, if we have @IBInspectable or
22602252
// @GKInspectable but did not infer @objc, warn that the attribute is
22612253
if (!VD->isObjC() && TC.Context.LangOpts.EnableSwift3ObjCInference) {
@@ -3070,6 +3062,9 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
30703062
if (requiresDefinition(FD) && !FD->hasBody()) {
30713063
// Complain if we should have a body.
30723064
TC.diagnose(FD->getLoc(), diag::func_decl_without_brace);
3065+
} else if (FD->getDeclContext()->isLocalContext()) {
3066+
// Check local function bodies right away.
3067+
TC.typeCheckAbstractFunctionBody(FD);
30733068
} else {
30743069
// Record the body.
30753070
TC.definedFunctions.push_back(FD);
@@ -3291,6 +3286,9 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
32913286
if (requiresDefinition(CD) && !CD->hasBody()) {
32923287
// Complain if we should have a body.
32933288
TC.diagnose(CD->getLoc(), diag::missing_initializer_def);
3289+
} else if (CD->getDeclContext()->isLocalContext()) {
3290+
// Check local function bodies right away.
3291+
TC.typeCheckAbstractFunctionBody(CD);
32943292
} else {
32953293
TC.definedFunctions.push_back(CD);
32963294
}
@@ -3303,7 +3301,13 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
33033301
void visitDestructorDecl(DestructorDecl *DD) {
33043302
TC.validateDecl(DD);
33053303
TC.checkDeclAttributes(DD);
3306-
TC.definedFunctions.push_back(DD);
3304+
3305+
if (DD->getDeclContext()->isLocalContext()) {
3306+
// Check local function bodies right away.
3307+
TC.typeCheckAbstractFunctionBody(DD);
3308+
} else {
3309+
TC.definedFunctions.push_back(DD);
3310+
}
33073311
}
33083312
};
33093313
} // end anonymous namespace

‎lib/Sema/TypeCheckStmt.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1926,7 +1926,9 @@ static Type getFunctionBuilderType(FuncDecl *FD) {
19261926
}
19271927

19281928
bool TypeChecker::typeCheckAbstractFunctionBody(AbstractFunctionDecl *AFD) {
1929-
return typeCheckAbstractFunctionBodyUntil(AFD, SourceLoc());
1929+
auto result = typeCheckAbstractFunctionBodyUntil(AFD, SourceLoc());
1930+
checkFunctionErrorHandling(AFD);
1931+
return result;
19301932
}
19311933

19321934
static Expr* constructCallToSuperInit(ConstructorDecl *ctor,

‎lib/Sema/TypeChecker.cpp

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ static void typeCheckFunctionsAndExternalDecls(SourceFile &SF, TypeChecker &TC)
318318
for (unsigned n = TC.definedFunctions.size(); currentFunctionIdx != n;
319319
++currentFunctionIdx) {
320320
auto *AFD = TC.definedFunctions[currentFunctionIdx];
321+
assert(!AFD->getDeclContext()->isLocalContext());
321322

322323
TC.typeCheckAbstractFunctionBody(AFD);
323324
}
@@ -358,19 +359,12 @@ static void typeCheckFunctionsAndExternalDecls(SourceFile &SF, TypeChecker &TC)
358359

359360
// Compute captures for functions and closures we visited.
360361
for (auto *closure : TC.ClosuresWithUncomputedCaptures) {
361-
TC.computeCaptures(closure);
362+
TypeChecker::computeCaptures(closure);
362363
}
363364
TC.ClosuresWithUncomputedCaptures.clear();
364365

365366
for (AbstractFunctionDecl *FD : reversed(TC.definedFunctions)) {
366-
TC.computeCaptures(FD);
367-
}
368-
369-
// Check error-handling correctness for all the functions defined in
370-
// this file. This can depend on all of their interior function
371-
// bodies having been type-checked.
372-
for (AbstractFunctionDecl *FD : TC.definedFunctions) {
373-
TC.checkFunctionErrorHandling(FD);
367+
TypeChecker::computeCaptures(FD);
374368
}
375369

376370
TC.definedFunctions.clear();

‎lib/Sema/TypeChecker.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,10 +1430,10 @@ class TypeChecker final : public LazyResolver {
14301430
bool typeCheckForEachBinding(DeclContext *dc, ForEachStmt *stmt);
14311431

14321432
/// Compute the set of captures for the given function or closure.
1433-
void computeCaptures(AnyFunctionRef AFR);
1433+
static void computeCaptures(AnyFunctionRef AFR);
14341434

14351435
/// Check for invalid captures from stored property initializers.
1436-
void checkPatternBindingCaptures(NominalTypeDecl *typeDecl);
1436+
static void checkPatternBindingCaptures(NominalTypeDecl *typeDecl);
14371437

14381438
/// Change the context of closures in the given initializer
14391439
/// expression to the given context.

‎test/NameBinding/name-binding.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ func overloadtest(x: Int) {
142142
func localtest() {
143143
func shadowbug() {
144144
var Foo = 10
145+
// expected-warning@-1 {{initialization of variable 'Foo' was never used; consider replacing with assignment to '_' or removing it}}
145146
func g() {
146147
struct S {
147148
// FIXME: Swap these two lines to crash our broken lookup.

‎test/NameBinding/name_lookup.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -410,13 +410,14 @@ extension ThisDerived1 {
410410

411411
// <rdar://problem/11554141>
412412
func shadowbug() {
413-
var Foo = 10
413+
let Foo = 10
414414
func g() {
415415
struct S {
416416
var x : Foo
417417
typealias Foo = Int
418418
}
419419
}
420+
_ = Foo
420421
}
421422
func scopebug() {
422423
let Foo = 10
@@ -633,4 +634,4 @@ struct PatternBindingWithTwoVars3 { var x = y, y = x }
633634
func sr9015() {
634635
let closure1 = { closure2() } // expected-error {{let 'closure1' references itself}}
635636
let closure2 = { closure1() }
636-
}
637+
}

‎test/Parse/dollar_identifier.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ func escapedDollarAnd() {
6464

6565
func $declareWithDollar() { // expected-error{{cannot declare entity named '$declareWithDollar'}}
6666
var $foo = 17 // expected-error{{cannot declare entity named '$foo'}}
67+
// expected-warning@-1 {{initialization of variable '$foo' was never used; consider replacing with assignment to '_' or removing it}}
6768
func $bar() { } // expected-error{{cannot declare entity named '$bar'}}
6869
func wibble(
6970
$a: Int, // expected-error{{cannot declare entity named '$a'}}

‎test/Parse/init_deinit.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,13 @@ func fooFunc() {
9898

9999
func barFunc() {
100100
var x : () = { () -> () in
101+
// expected-warning@-1 {{variable 'x' was never used; consider replacing with '_' or removing it}}
101102
init() {} // expected-error {{initializers may only be declared within a type}}
102103
return
103104
} ()
104105

105106
var y : () = { () -> () in
107+
// expected-warning@-1 {{variable 'y' was never used; consider replacing with '_' or removing it}}
106108
deinit {} // expected-error {{deinitializers may only be declared within a class}}
107109
return
108110
} ()

‎test/SILGen/capture_order.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ class SR4812 {
158158
let bar = { [weak self] in
159159
// expected-error@-1 {{closure captures 'bar' before it is declared}}
160160
// expected-note@-2 {{captured value declared here}}
161+
// expected-warning@-3 {{variable 'self' was written to, but never read}}
161162
bar2()
162163
}
163164
func bar2() {
@@ -198,4 +199,4 @@ class rdar40600800 {
198199
}
199200
}
200201
}
201-
}
202+
}

‎test/SILGen/inlinable_attribute.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public func bas() {
142142
}
143143

144144
// CHECK-LABEL: sil shared [serialized] [ossa] @$s19inlinable_attribute3bas{{[_0-9a-zA-Z]*}}U_
145-
let zung = {
145+
let _ = {
146146
// CHECK-LABEL: sil shared [serializable] [ossa] @$s19inlinable_attribute3basyyFyycfU_7zippityL_yyF
147147
func zippity() { }
148148
}

‎test/SILGen/lazy_properties.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,20 @@ class LazyClass {
4646
// CHECK-LABEL: sil hidden [ossa] @$s15lazy_properties9LazyClassC1xSivs : $@convention(method) (Int, @guaranteed LazyClass) -> ()
4747
// CHECK: ref_element_addr %1 : $LazyClass, #LazyClass.$__lazy_storage_$_x
4848
// CHECK: return
49+
50+
// rdar://problem/53956342
51+
class Butt {
52+
func foo() -> Int { return 0 }
53+
54+
lazy var butt: Int = {
55+
func bar() -> Int{
56+
return foo()
57+
}
58+
return bar()
59+
}()
60+
}
61+
62+
// Both the closure and the local function inside of it should capture 'self':
63+
64+
// CHECK-LABEL: sil private [ossa] @$s15lazy_properties4ButtC4buttSivgSiyXEfU_ : $@convention(thin) (@guaranteed Butt) -> Int
65+
// CHECK-LABEL: sil private [ossa] @$s15lazy_properties4ButtC4buttSivgSiyXEfU_3barL_SiyF : $@convention(thin) (@guaranteed Butt) -> Int

‎test/Sema/availability_versions.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -482,13 +482,15 @@ func accessUnavailableProperties(_ o: ClassWithUnavailableProperties) {
482482

483483
// Nesting in source of assignment
484484
var v: Int
485-
485+
486486
v = o.propWithGetterOnlyAvailableOn10_51 // expected-error {{getter for 'propWithGetterOnlyAvailableOn10_51' is only available in macOS 10.51 or newer}}
487487
// expected-note@-1 {{add 'if #available' version check}}
488488

489489
v = (o.propWithGetterOnlyAvailableOn10_51) // expected-error {{getter for 'propWithGetterOnlyAvailableOn10_51' is only available in macOS 10.51 or newer}}
490490
// expected-note@-1 {{add 'if #available' version check}}
491-
491+
492+
_ = v // muffle warning
493+
492494
// Inout requires access to both getter and setter
493495

494496
func takesInout(_ i : inout Int) { }

‎test/Sema/diag_defer_block_end.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ if (x > y) {
1111
func sr7307(_ value: Bool) {
1212
let negated = !value
1313
defer { // expected-warning {{'defer' statement at end of scope always executes immediately}}{{5-10=do}}
14-
print("negated value is {negated}")
14+
print("negated value is \(negated)")
1515
}
1616
}
1717

0 commit comments

Comments
 (0)