Skip to content

Commit 93f0faf

Browse files
committed
Sema: Fix false-positive var => let warning in VarUsageDeclChecker
Fixes <rdar://problem/60563962>.
1 parent 7b4786e commit 93f0faf

File tree

3 files changed

+32
-39
lines changed

3 files changed

+32
-39
lines changed

lib/Sema/MiscDiagnostics.cpp

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2236,10 +2236,11 @@ class VarDeclUsageChecker : public ASTWalker {
22362236
DiagnosticEngine &Diags;
22372237
// Keep track of some information about a variable.
22382238
enum {
2239-
RK_Read = 1, ///< Whether it was ever read.
2240-
RK_Written = 2, ///< Whether it was ever written or passed inout.
2239+
RK_Defined = 1, ///< Whether it was ever defined in this scope.
2240+
RK_Read = 2, ///< Whether it was ever read.
2241+
RK_Written = 4, ///< Whether it was ever written or passed inout.
22412242

2242-
RK_CaptureList = 4 ///< Var is an entry in a capture list.
2243+
RK_CaptureList = 8 ///< Var is an entry in a capture list.
22432244
};
22442245

22452246
/// These are all of the variables that we are tracking. VarDecls get added
@@ -2280,7 +2281,7 @@ class VarDeclUsageChecker : public ASTWalker {
22802281
if (FD->getAccessorKind() == AccessorKind::Set) {
22812282
if (auto getter = dyn_cast<VarDecl>(FD->getStorage())) {
22822283
auto arguments = FD->getParameters();
2283-
VarDecls[arguments->get(0)] = 0;
2284+
VarDecls[arguments->get(0)] = RK_Defined;
22842285
AssociatedGetter = getter;
22852286
}
22862287
}
@@ -2291,9 +2292,9 @@ class VarDeclUsageChecker : public ASTWalker {
22912292

22922293
VarDeclUsageChecker(VarDecl *vd) : Diags(vd->getASTContext().Diags) {
22932294
// Track a specific VarDecl
2294-
VarDecls[vd] = 0;
2295+
VarDecls[vd] = RK_Defined;
22952296
if (auto *childVd = vd->getCorrespondingCaseBodyVariable().getPtrOrNull()) {
2296-
VarDecls[childVd] = 0;
2297+
VarDecls[childVd] = RK_Defined;
22972298
}
22982299
}
22992300

@@ -2323,10 +2324,6 @@ class VarDeclUsageChecker : public ASTWalker {
23232324
}
23242325
return sawMutation;
23252326
}
2326-
2327-
bool isVarDeclEverWritten(VarDecl *VD) {
2328-
return (VarDecls[VD] & RK_Written) != 0;
2329-
}
23302327

23312328
bool shouldTrackVarDecl(VarDecl *VD) {
23322329
// If the variable is implicit, ignore it.
@@ -2355,9 +2352,7 @@ class VarDeclUsageChecker : public ASTWalker {
23552352
auto *vd = dyn_cast<VarDecl>(D);
23562353
if (!vd) return;
23572354

2358-
auto vdi = VarDecls.find(vd);
2359-
if (vdi != VarDecls.end())
2360-
vdi->second |= Flag;
2355+
VarDecls[vd] |= Flag;
23612356
}
23622357

23632358
void markBaseOfStorageUse(Expr *E, ConcreteDeclRef decl, unsigned flags);
@@ -2385,17 +2380,17 @@ class VarDeclUsageChecker : public ASTWalker {
23852380
// that fact for better diagnostics.
23862381
auto parentAsExpr = Parent.getAsExpr();
23872382
if (parentAsExpr && isa<CaptureListExpr>(parentAsExpr))
2388-
return RK_CaptureList;
2383+
return RK_CaptureList | RK_Defined;
23892384
// Otherwise, return none.
2390-
return 0;
2385+
return RK_Defined;
23912386
}();
23922387

23932388
if (!vd->isImplicit()) {
23942389
if (auto *childVd =
23952390
vd->getCorrespondingCaseBodyVariable().getPtrOrNull()) {
23962391
// Child vars are never in capture lists.
2397-
assert(defaultFlags == 0);
2398-
VarDecls[childVd] |= 0;
2392+
assert(defaultFlags == RK_Defined);
2393+
VarDecls[childVd] |= RK_Defined;
23992394
}
24002395
}
24012396
VarDecls[vd] |= defaultFlags;
@@ -2437,7 +2432,7 @@ class VarDeclUsageChecker : public ASTWalker {
24372432
if (!PBD) continue;
24382433
for (auto idx : range(PBD->getNumPatternEntries())) {
24392434
PBD->getPattern(idx)->forEachVariable([&](VarDecl *VD) {
2440-
VarDecls[VD] = RK_Read|RK_Written;
2435+
VarDecls[VD] = RK_Read|RK_Written|RK_Defined;
24412436
});
24422437
}
24432438
} else if (node.is<Stmt *>()) {
@@ -2448,7 +2443,7 @@ class VarDeclUsageChecker : public ASTWalker {
24482443
for (StmtConditionElement SCE : GS->getCond()) {
24492444
if (auto pattern = SCE.getPatternOrNull()) {
24502445
pattern->forEachVariable([&](VarDecl *VD) {
2451-
VarDecls[VD] = RK_Read|RK_Written;
2446+
VarDecls[VD] = RK_Read|RK_Written|RK_Defined;
24522447
});
24532448
}
24542449
}
@@ -2507,7 +2502,7 @@ class VarDeclUsageChecker : public ASTWalker {
25072502
// Make sure that we setup our case body variables.
25082503
if (auto *caseStmt = dyn_cast<CaseStmt>(S)) {
25092504
for (auto *vd : caseStmt->getCaseBodyVariablesOrEmptyArray()) {
2510-
VarDecls[vd] |= 0;
2505+
VarDecls[vd] |= RK_Defined;
25112506
}
25122507
}
25132508

@@ -2661,6 +2656,10 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
26612656
unsigned access;
26622657
std::tie(var, access) = p;
26632658

2659+
// If the variable was not defined in this scope, we can safely ignore it.
2660+
if (!(access & RK_Defined))
2661+
continue;
2662+
26642663
if (auto *caseStmt =
26652664
dyn_cast_or_null<CaseStmt>(var->getRecursiveParentPatternStmt())) {
26662665
// Only diagnose VarDecls from the first CaseLabelItem in CaseStmts, as

test/SILGen/capture_order.swift

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ public func captureBeforeDefLet(amount: Int) -> () -> Int {
99
}
1010
let closure = getter
1111
let modifiedAmount = amount // expected-note{{captured value declared here}}
12-
// FIXME: Bogus warning!
13-
// expected-warning@-2 {{initialization of immutable value 'modifiedAmount' was never used; consider replacing with assignment to '_' or removing it}}
1412
return closure
1513
}
1614

@@ -21,8 +19,6 @@ public func captureBeforeDefVar(amount: Int) -> () -> Int {
2119
}
2220
let closure = incrementor
2321
var currentTotal = 0 // expected-note{{captured value declared here}}
24-
// FIXME: Bogus warning!
25-
// expected-warning@-2 {{variable 'currentTotal' was written to, but never read}}
2622
currentTotal = 1
2723
return closure
2824
}
@@ -33,8 +29,6 @@ public func captureBeforeDefWeakVar(obj: AnyObject) -> () -> AnyObject? {
3329
}
3430
let closure = getter
3531
weak var weakObj: AnyObject? = obj // expected-note{{captured value declared here}}
36-
// FIXME: Bogus warning!
37-
// expected-warning@-2 {{variable 'weakObj' was written to, but never read}}
3832
return closure
3933
}
4034

@@ -44,8 +38,6 @@ public func captureBeforeDefUnownedLet(obj: AnyObject) -> () -> AnyObject? {
4438
}
4539
let closure = getter
4640
unowned let unownedObj: AnyObject = obj // expected-note{{captured value declared here}}
47-
// FIXME: Bogus warning!
48-
// expected-warning@-2 {{immutable value 'unownedObj' was never used; consider replacing with '_' or removing it}}
4941
return closure
5042
}
5143

@@ -55,8 +47,6 @@ public func captureBeforeDefUnownedVar(obj: AnyObject) -> () -> AnyObject? {
5547
}
5648
let closure = getter
5749
unowned var unownedObj: AnyObject = obj // expected-note{{captured value declared here}}
58-
// FIXME: Bogus warning!
59-
// expected-warning@-2 {{variable 'unownedObj' was never used; consider replacing with '_' or removing it}}
6050
return closure
6151
}
6252

@@ -122,8 +112,6 @@ func captureInClosure() {
122112
}
123113

124114
var currentTotal = 0 // expected-note {{captured value declared here}}
125-
// FIXME: Bogus warning!
126-
// expected-warning@-2 {{initialization of variable 'currentTotal' was never used; consider replacing with assignment to '_' or removing it}}
127115

128116
_ = x
129117
}
@@ -139,8 +127,6 @@ func sr3210_crash() {
139127

140128
let b = 2 // expected-note {{captured value declared here}}
141129
// expected-warning@-1 {{code after 'return' will never be executed}}
142-
// FIXME: Bogus warning!
143-
// expected-warning@-3 {{initialization of immutable value 'b' was never used; consider replacing with assignment to '_' or removing it}}
144130
}
145131

146132
func sr3210() {
@@ -149,8 +135,6 @@ func sr3210() {
149135
}
150136

151137
let b = 2
152-
// FIXME: Bogus warning!
153-
// expected-warning@-2 {{initialization of immutable value 'b' was never used; consider replacing with assignment to '_' or removing it}}
154138
}
155139

156140
class SR4812 {
@@ -193,8 +177,7 @@ class rdar40600800 {
193177

194178
func innerFunction() {
195179
let closure = {
196-
// FIXME: Bogus warning!
197-
// expected-warning@-2 {{initialization of immutable value 'closure' was never used; consider replacing with assignment to '_' or removing it}}
180+
// expected-warning@-1 {{initialization of immutable value 'closure' was never used; consider replacing with assignment to '_' or removing it}}
198181
callback() // expected-note {{captured here}}
199182
}
200183
}

test/decl/var/usage.swift

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,4 +434,15 @@ func testLocalFunc() {
434434
_ = notMutatedVar
435435
mutatedVar = 1
436436
}
437-
}
437+
}
438+
439+
// False positive "'var' was never mutated" warning - rdar://60563962
440+
func testForwardReferenceCapture() {
441+
func innerFunc() {
442+
x = 10
443+
}
444+
445+
var x: Int = 1
446+
innerFunc()
447+
print(x)
448+
}

0 commit comments

Comments
 (0)