Skip to content

Commit 2ef101c

Browse files
slavapestovslavapestov
committed
Sema: Don't add local functions to TC.definedFunctions
Instead, check them and their error handling right away. In addition to fixing the crash in the radar, this also causes us to emit unused variable warnings in functions containing local functions. Eventually, TC.definedFunctions should go away altogether. Fixes <rdar://problem/53956342>.
1 parent beb883f commit 2ef101c

19 files changed

+84
-18
lines changed

‎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/TypeCheckDecl.cpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3062,6 +3062,9 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
30623062
if (requiresDefinition(FD) && !FD->hasBody()) {
30633063
// Complain if we should have a body.
30643064
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);
30653068
} else {
30663069
// Record the body.
30673070
TC.definedFunctions.push_back(FD);
@@ -3283,6 +3286,9 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
32833286
if (requiresDefinition(CD) && !CD->hasBody()) {
32843287
// Complain if we should have a body.
32853288
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);
32863292
} else {
32873293
TC.definedFunctions.push_back(CD);
32883294
}
@@ -3295,7 +3301,13 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
32953301
void visitDestructorDecl(DestructorDecl *DD) {
32963302
TC.validateDecl(DD);
32973303
TC.checkDeclAttributes(DD);
3298-
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+
}
32993311
}
33003312
};
33013313
} // 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: 1 addition & 7 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
}
@@ -366,13 +367,6 @@ static void typeCheckFunctionsAndExternalDecls(SourceFile &SF, TypeChecker &TC)
366367
TypeChecker::computeCaptures(FD);
367368
}
368369

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);
374-
}
375-
376370
TC.definedFunctions.clear();
377371
}
378372

‎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

‎test/attr/attr_noescape.swift

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,11 @@ class SomeClass {
6565

6666
func plain() { foo() }
6767
let plain2 = { foo() } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{20-20=self.}}
68+
_ = plain2
6869

6970
func multi() -> Int { foo(); return 0 }
7071
let mulit2: () -> Int = { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{31-31=self.}}
72+
_ = mulit2
7173

7274
doesEscape { foo() } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{18-18=self.}}
7375
takesNoEscapeClosure { foo() } // okay
@@ -78,6 +80,7 @@ class SomeClass {
7880
func outer() {
7981
func inner() { foo() }
8082
let inner2 = { foo() } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{22-22=self.}}
83+
_ = inner2
8184
func multi() -> Int { foo(); return 0 }
8285
let _: () -> Int = { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{28-28=self.}}
8386
doesEscape { foo() } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{20-20=self.}}
@@ -86,11 +89,13 @@ class SomeClass {
8689
takesNoEscapeClosure { foo(); return 0 }
8790
}
8891

89-
let outer2: () -> Void = {
92+
let _: () -> Void = {
9093
func inner() { foo() } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{22-22=self.}}
9194
let inner2 = { foo() } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{22-22=self.}}
95+
_ = inner2
9296
func multi() -> Int { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{29-29=self.}}
9397
let mulit2: () -> Int = { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{33-33=self.}}
98+
_ = mulit2
9499
doesEscape { foo() } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{20-20=self.}}
95100
takesNoEscapeClosure { foo() } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{30-30=self.}}
96101
doesEscape { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{20-20=self.}}
@@ -100,8 +105,10 @@ class SomeClass {
100105
doesEscape {
101106
func inner() { foo() } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{22-22=self.}}
102107
let inner2 = { foo() } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{22-22=self.}}
108+
_ = inner2
103109
func multi() -> Int { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{29-29=self.}}
104110
let mulit2: () -> Int = { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{33-33=self.}}
111+
_ = mulit2
105112
doesEscape { foo() } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{20-20=self.}}
106113
takesNoEscapeClosure { foo() } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{30-30=self.}}
107114
doesEscape { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{20-20=self.}}
@@ -111,8 +118,10 @@ class SomeClass {
111118
takesNoEscapeClosure {
112119
func inner() { foo() }
113120
let inner2 = { foo() } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{22-22=self.}}
121+
_ = inner2
114122
func multi() -> Int { foo(); return 0 }
115123
let mulit2: () -> Int = { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{33-33=self.}}
124+
_ = mulit2
116125
doesEscape { foo() } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{20-20=self.}}
117126
takesNoEscapeClosure { foo() } // okay
118127
doesEscape { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit}} {{20-20=self.}}
@@ -127,6 +136,7 @@ class SomeClass {
127136

128137
func plain() { bar() }
129138
let plain2 = { bar() } // expected-error {{call to method 'bar' in closure requires explicit 'self.' to make capture semantics explicit}} {{24-24=self.}}
139+
_ = plain2
130140

131141
func multi() -> Int { bar(); return 0 }
132142
let _: () -> Int = { bar(); return 0 } // expected-error {{call to method 'bar' in closure requires explicit 'self.' to make capture semantics explicit}} {{30-30=self.}}
@@ -149,6 +159,7 @@ class SomeClass {
149159

150160
func plain() { bar() }
151161
let plain2 = { bar() } // expected-error {{call to method 'bar' in closure requires explicit 'self.' to make capture semantics explicit}} {{26-26=self.}}
162+
_ = plain2
152163

153164
func multi() -> Int { bar(); return 0 }
154165
let _: () -> Int = { bar(); return 0 } // expected-error {{call to method 'bar' in closure requires explicit 'self.' to make capture semantics explicit}} {{32-32=self.}}

‎test/attr/attr_objc.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ class subject_staticVar1 {
106106
func subject_freeFunc() { // expected-error {{@objc can only be used with members of classes, @objc protocols, and concrete extensions of classes}} {{1-6=}}
107107
@objc
108108
var subject_localVar: Int // expected-error {{@objc can only be used with members of classes, @objc protocols, and concrete extensions of classes}} {{3-8=}}
109+
// expected-warning@-1 {{variable 'subject_localVar' was never used; consider replacing with '_' or removing it}}
109110

110111
@objc
111112
func subject_nestedFreeFunc() { // expected-error {{@objc can only be used with members of classes, @objc protocols, and concrete extensions of classes}} {{3-8=}}
@@ -116,6 +117,7 @@ func subject_freeFunc() { // expected-error {{@objc can only be used with member
116117
func subject_genericFunc<T>(t: T) { // expected-error {{@objc can only be used with members of classes, @objc protocols, and concrete extensions of classes}} {{1-6=}}
117118
@objc
118119
var subject_localVar: Int // expected-error {{@objc can only be used with members of classes, @objc protocols, and concrete extensions of classes}} {{3-8=}}
120+
// expected-warning@-1 {{variable 'subject_localVar' was never used; consider replacing with '_' or removing it}}
119121

120122
@objc
121123
func subject_instanceFunc() {} // expected-error {{@objc can only be used with members of classes, @objc protocols, and concrete extensions of classes}} {{3-8=}}

‎test/decl/var/properties.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ var implicitGet1: X {
9696

9797
var implicitGet2: Int {
9898
var zzz = 0
99+
// expected-warning@-1 {{initialization of variable 'zzz' was never used; consider replacing with assignment to '_' or removing it}}
99100
// For the purpose of this test, any other function attribute work as well.
100101
@inline(__always)
101102
func foo() {}
@@ -191,6 +192,7 @@ func disambiguateGetSet4() {
191192
func disambiguateGetSet4Attr() {
192193
func set(_ x: Int, fn: () -> ()) {}
193194
var newValue: Int = 0
195+
// expected-warning@-1 {{variable 'newValue' was never mutated; consider changing to 'let' constant}}
194196
var a: Int = takeTrailingClosure {
195197
@inline(__always)
196198
func foo() {}

‎test/decl/var/usage.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,3 +420,18 @@ extension MemberGetterExtension {
420420
}
421421
}
422422
}
423+
424+
func testLocalFunc() {
425+
var unusedVar = 0
426+
// expected-warning@-1 {{initialization of variable 'unusedVar' was never used; consider replacing with assignment to '_' or removing it}}
427+
428+
var notMutatedVar = 0
429+
// FIXME: All captures are read/write for the purposes of this analysis.
430+
431+
var mutatedVar = 0
432+
433+
func localFunc() {
434+
_ = notMutatedVar
435+
mutatedVar = 1
436+
}
437+
}

‎test/expr/closure/closures.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ extension SomeClass {
229229

230230
// <rdar://problem/16955318> Observed variable in a closure triggers an assertion
231231
var closureWithObservedProperty: () -> () = {
232-
var a: Int = 42 {
232+
var a: Int = 42 { // expected-warning {{variable 'a' was never used; consider replacing with '_' or removing it}}
233233
willSet {
234234
_ = "Will set a to \(newValue)"
235235
}

‎test/stmt/statements.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,13 +158,15 @@ func tuple_assign() {
158158
(a,b) = (1,2)
159159
func f() -> (Int,Int) { return (1,2) }
160160
((a,b), (c,d)) = (f(), f())
161+
_ = (a,b,c,d)
161162
}
162163

163164
func missing_semicolons() {
164165
var w = 321
165166
func g() {}
166167
g() w += 1 // expected-error{{consecutive statements}} {{6-6=;}}
167168
var z = w"hello" // expected-error{{consecutive statements}} {{12-12=;}} expected-warning {{string literal is unused}}
169+
// expected-warning@-1 {{initialization of variable 'z' was never used; consider replacing with assignment to '_' or removing it}}
168170
class C {}class C2 {} // expected-error{{consecutive statements}} {{14-14=;}}
169171
struct S {}struct S2 {} // expected-error{{consecutive statements}} {{14-14=;}}
170172
func j() {}func k() {} // expected-error{{consecutive statements}} {{14-14=;}}

0 commit comments

Comments
 (0)