Skip to content

Commit 71cf245

Browse files
slavapestovslavapestov
authored
Merge pull request #7023 from KingOfBrian/bugfix/SR-2115
Generate unused variable warnings in top level statements
2 parents 11aee8f + c231b8c commit 71cf245

23 files changed

+169
-107
lines changed

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 56 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1865,8 +1865,7 @@ bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
18651865

18661866
namespace {
18671867
class VarDeclUsageChecker : public ASTWalker {
1868-
TypeChecker &TC;
1869-
1868+
DiagnosticEngine &Diags;
18701869
// Keep track of some information about a variable.
18711870
enum {
18721871
RK_Read = 1, ///< Whether it was ever read.
@@ -1894,16 +1893,18 @@ class VarDeclUsageChecker : public ASTWalker {
18941893
void operator=(const VarDeclUsageChecker &) = delete;
18951894

18961895
public:
1897-
VarDeclUsageChecker(TypeChecker &TC, AbstractFunctionDecl *AFD) : TC(TC) {
1896+
VarDeclUsageChecker(TypeChecker &TC, AbstractFunctionDecl *AFD) : Diags(TC.Diags) {
18981897
// Track the parameters of the function.
18991898
for (auto PL : AFD->getParameterLists())
19001899
for (auto param : *PL)
19011900
if (shouldTrackVarDecl(param))
19021901
VarDecls[param] = 0;
19031902

19041903
}
1905-
1906-
VarDeclUsageChecker(TypeChecker &TC, VarDecl *VD) : TC(TC) {
1904+
1905+
VarDeclUsageChecker(DiagnosticEngine &Diags) : Diags(Diags) {}
1906+
1907+
VarDeclUsageChecker(TypeChecker &TC, VarDecl *VD) : Diags(TC.Diags) {
19071908
// Track a specific VarDecl
19081909
VarDecls[VD] = 0;
19091910
}
@@ -2009,6 +2010,35 @@ class VarDeclUsageChecker : public ASTWalker {
20092010
// Don't walk into it though, it may not even be type checked yet.
20102011
return false;
20112012
}
2013+
if (auto *TLCD = dyn_cast<TopLevelCodeDecl>(D)) {
2014+
// If this is a TopLevelCodeDecl, scan for global variables
2015+
auto *body = TLCD->getBody();
2016+
for (auto node : body->getElements()) {
2017+
if (node.is<Decl *>()) {
2018+
// Flag all variables in a PatternBindingDecl
2019+
Decl *D = node.get<Decl *>();
2020+
PatternBindingDecl *PBD = dyn_cast<PatternBindingDecl>(D);
2021+
if (!PBD) continue;
2022+
for (PatternBindingEntry PBE : PBD->getPatternList()) {
2023+
PBE.getPattern()->forEachVariable([&](VarDecl *VD) {
2024+
VarDecls[VD] = RK_Read|RK_Written;
2025+
});
2026+
}
2027+
} else if (node.is<Stmt *>()) {
2028+
// Flag all variables in guard statements
2029+
Stmt *S = node.get<Stmt *>();
2030+
GuardStmt *GS = dyn_cast<GuardStmt>(S);
2031+
if (!GS) continue;
2032+
for (StmtConditionElement SCE : GS->getCond()) {
2033+
if (auto pattern = SCE.getPatternOrNull()) {
2034+
pattern->forEachVariable([&](VarDecl *VD) {
2035+
VarDecls[VD] = RK_Read|RK_Written;
2036+
});
2037+
}
2038+
}
2039+
}
2040+
}
2041+
}
20122042

20132043

20142044
// Note that we ignore the initialization behavior of PatternBindingDecls,
@@ -2091,8 +2121,8 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
20912121
// produce a fixit hint with a parent map, but this is a lot of effort for
20922122
// a narrow case.
20932123
if (access & RK_CaptureList) {
2094-
TC.diagnose(var->getLoc(), diag::capture_never_used,
2095-
var->getName());
2124+
Diags.diagnose(var->getLoc(), diag::capture_never_used,
2125+
var->getName());
20962126
continue;
20972127
}
20982128

@@ -2108,8 +2138,8 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
21082138
SourceRange replaceRange(
21092139
pbd->getStartLoc(),
21102140
pbd->getPatternList()[0].getPattern()->getEndLoc());
2111-
TC.diagnose(var->getLoc(), diag::pbd_never_used,
2112-
var->getName(), varKind)
2141+
Diags.diagnose(var->getLoc(), diag::pbd_never_used,
2142+
var->getName(), varKind)
21132143
.fixItReplace(replaceRange, "_");
21142144
continue;
21152145
}
@@ -2143,8 +2173,8 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
21432173
noParens = isIsTest = true;
21442174
}
21452175

2146-
auto diagIF = TC.diagnose(var->getLoc(),
2147-
diag::pbd_never_used_stmtcond,
2176+
auto diagIF = Diags.diagnose(var->getLoc(),
2177+
diag::pbd_never_used_stmtcond,
21482178
var->getName());
21492179
auto introducerLoc = SC->getCond()[0].getIntroducerLoc();
21502180
diagIF.fixItReplaceChars(introducerLoc,
@@ -2171,8 +2201,8 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
21712201
// let (a,b) = foo()
21722202
// Just rewrite the one variable with a _.
21732203
unsigned varKind = var->isLet();
2174-
TC.diagnose(var->getLoc(), diag::variable_never_used,
2175-
var->getName(), varKind)
2204+
Diags.diagnose(var->getLoc(), diag::variable_never_used,
2205+
var->getName(), varKind)
21762206
.fixItReplace(var->getLoc(), "_");
21772207
continue;
21782208
}
@@ -2205,18 +2235,18 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
22052235
// If this is a parameter explicitly marked 'var', remove it.
22062236
unsigned varKind = isa<ParamDecl>(var);
22072237
if (FixItLoc.isInvalid())
2208-
TC.diagnose(var->getLoc(), diag::variable_never_mutated,
2209-
var->getName(), varKind);
2238+
Diags.diagnose(var->getLoc(), diag::variable_never_mutated,
2239+
var->getName(), varKind);
22102240
else
2211-
TC.diagnose(var->getLoc(), diag::variable_never_mutated,
2212-
var->getName(), varKind)
2241+
Diags.diagnose(var->getLoc(), diag::variable_never_mutated,
2242+
var->getName(), varKind)
22132243
.fixItReplace(FixItLoc, "let");
22142244
continue;
22152245
}
22162246

22172247
// If this is a variable that was only written to, emit a warning.
22182248
if ((access & RK_Read) == 0) {
2219-
TC.diagnose(var->getLoc(), diag::variable_never_read, var->getName(),
2249+
Diags.diagnose(var->getLoc(), diag::variable_never_read, var->getName(),
22202250
isa<ParamDecl>(var));
22212251
continue;
22222252
}
@@ -2387,6 +2417,14 @@ void VarDeclUsageChecker::handleIfConfig(IfConfigStmt *ICS) {
23872417
}
23882418
}
23892419

2420+
/// Apply the warnings managed by VarDeclUsageChecker to the top level
2421+
/// code declarations that haven't been checked yet.
2422+
void swift::
2423+
performTopLevelDeclDiagnostics(TypeChecker &TC, TopLevelCodeDecl *TLCD) {
2424+
VarDeclUsageChecker checker(TC.Diags);
2425+
TLCD->walk(checker);
2426+
}
2427+
23902428
/// Perform diagnostics for func/init/deinit declarations.
23912429
void swift::performAbstractFuncDeclDiagnostics(TypeChecker &TC,
23922430
AbstractFunctionDecl *AFD) {

‎lib/Sema/MiscDiagnostics.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ namespace swift {
2828
class Expr;
2929
class InFlightDiagnostic;
3030
class Stmt;
31+
class TopLevelCodeDecl;
3132
class TypeChecker;
3233
class ValueDecl;
3334

@@ -41,6 +42,9 @@ void performStmtDiagnostics(TypeChecker &TC, const Stmt *S);
4142

4243
void performAbstractFuncDeclDiagnostics(TypeChecker &TC,
4344
AbstractFunctionDecl *AFD);
45+
46+
/// Perform diagnostics on the top level code declaration.
47+
void performTopLevelDeclDiagnostics(TypeChecker &TC, TopLevelCodeDecl *TLCD);
4448

4549
/// Emit a fix-it to set the accessibility of \p VD to \p desiredAccess.
4650
///

‎lib/Sema/TypeCheckStmt.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1608,4 +1608,5 @@ void TypeChecker::typeCheckTopLevelCodeDecl(TopLevelCodeDecl *TLCD) {
16081608
StmtChecker(*this, TLCD).typeCheckStmt(Body);
16091609
TLCD->setBody(Body);
16101610
checkTopLevelErrorHandling(TLCD);
1611+
performTopLevelDeclDiagnostics(*this, TLCD);
16111612
}

‎lib/Sema/TypeChecker.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#include "swift/Subsystems.h"
1919
#include "TypeChecker.h"
20+
#include "MiscDiagnostics.h"
2021
#include "GenericTypeResolver.h"
2122
#include "swift/AST/ASTWalker.h"
2223
#include "swift/AST/ASTVisitor.h"

‎test/ClangImporter/blocks_parse.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func checkTypeImpl<T>(_ a: inout T, _: T.Type) {}
3232
do {
3333
var blockOpt = blockWithoutNullability()
3434
checkTypeImpl(&blockOpt, Optional<my_block_t>.self)
35-
var block: my_block_t = blockWithoutNullability()
35+
var _: my_block_t = blockWithoutNullability()
3636
}
3737
do {
3838
var block = blockWithNonnull()
@@ -41,7 +41,7 @@ do {
4141
do {
4242
var blockOpt = blockWithNullUnspecified()
4343
checkTypeImpl(&blockOpt, Optional<my_block_t>.self)
44-
var block: my_block_t = blockWithNullUnspecified()
44+
var _: my_block_t = blockWithNullUnspecified()
4545
}
4646
do {
4747
var block = blockWithNullable()

‎test/Compatibility/tuple_arguments.swift

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,10 @@ do {
5858
}
5959

6060
do {
61-
var a = 3
62-
var b = 4
63-
var c = (3)
64-
var d = (a, b)
61+
var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
62+
var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
63+
var c = (3) // expected-warning {{variable 'c' was never mutated; consider changing to 'let' constant}}
64+
var d = (a, b) // expected-warning {{variable 'd' was never mutated; consider changing to 'let' constant}}
6565

6666
concrete(a)
6767
concrete((a))
@@ -565,9 +565,9 @@ do {
565565
}
566566

567567
do {
568-
var a = 3
569-
var b = 4
570-
var c = (a, b)
568+
var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
569+
var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
570+
var c = (a, b) // expected-warning {{variable 'c' was never mutated; consider changing to 'let' constant}}
571571

572572
_ = InitTwo(a, b)
573573
_ = InitTwo((a, b)) // expected-error {{missing argument for parameter #2 in call}}
@@ -621,9 +621,9 @@ do {
621621
}
622622

623623
do {
624-
var a = 3
625-
var b = 4
626-
var d = (a, b)
624+
var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
625+
var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
626+
var d = (a, b) // expected-warning {{variable 'd' was never mutated; consider changing to 'let' constant}}
627627

628628
var s1 = SubscriptTwo()
629629
_ = s1[a, b]
@@ -1028,9 +1028,9 @@ do {
10281028
}
10291029

10301030
do {
1031-
var a = 3
1032-
var b = 4
1033-
var c = (a, b)
1031+
var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
1032+
var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
1033+
var c = (a, b) // expected-warning {{variable 'c' was never mutated; consider changing to 'let' constant}}
10341034

10351035
// _ = GenericInit<(Int, Int)>(a, b) // Crashes in Swift 3
10361036
_ = GenericInit<(Int, Int)>((a, b)) // expected-error {{expression type 'GenericInit<(Int, Int)>' is ambiguous without more context}}
@@ -1109,9 +1109,9 @@ do {
11091109
}
11101110

11111111
do {
1112-
var a = 3.0
1113-
var b = 4.0
1114-
var d = (a, b)
1112+
var a = 3.0 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
1113+
var b = 4.0 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
1114+
var d = (a, b) // expected-warning {{variable 'd' was never mutated; consider changing to 'let' constant}}
11151115

11161116
var s1 = GenericSubscript<(Double, Double)>()
11171117
_ = s1[a, b]

‎test/Constraints/operator.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ do {
66
let a: String? = "a"
77
let b: String? = "b"
88
let c: String? = "c"
9-
let d: String? = a! + b! + c!
9+
let _: String? = a! + b! + c!
1010

1111
let x: Double = 1
1212
_ = x + x + x

‎test/Constraints/patterns.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ default:
123123

124124
// <rdar://problem/19382878> Introduce new x? pattern
125125
switch Optional(42) {
126-
case let x?: break
126+
case let x?: break // expected-warning{{immutable value 'x' was never used; consider replacing with '_' or removing it}}
127127
case nil: break
128128
}
129129

@@ -181,9 +181,9 @@ case x ?? 42: break // match value
181181
default: break
182182
}
183183

184-
for (var x) in 0...100 {}
185-
for var x in 0...100 {} // rdar://20167543
186-
for (let x) in 0...100 {} // expected-error {{'let' pattern cannot appear nested in an already immutable context}}
184+
for (var x) in 0...100 {} // expected-warning{{variable 'x' was never used; consider replacing with '_' or removing it}}
185+
for var x in 0...100 {} // rdar://20167543 expected-warning{{variable 'x' was never used; consider replacing with '_' or removing it}}
186+
for (let x) in 0...100 { _ = x} // expected-error {{'let' pattern cannot appear nested in an already immutable context}}
187187

188188
var (let y) = 42 // expected-error {{'let' cannot appear nested inside another 'var' or 'let' pattern}}
189189
let (var z) = 42 // expected-error {{'var' cannot appear nested inside another 'var' or 'let' pattern}}

‎test/Constraints/tuple_arguments.swift

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,10 @@ do {
4242
}
4343

4444
do {
45-
var a = 3
46-
var b = 4
47-
var c = (3)
48-
var d = (a, b)
45+
var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
46+
var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
47+
var c = (3) // expected-warning {{variable 'c' was never mutated; consider changing to 'let' constant}}
48+
var d = (a, b) // expected-warning {{variable 'd' was never mutated; consider changing to 'let' constant}}
4949

5050
concrete(a)
5151
concrete((a))
@@ -550,9 +550,9 @@ do {
550550
}
551551

552552
do {
553-
var a = 3
554-
var b = 4
555-
var c = (a, b)
553+
var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
554+
var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
555+
var c = (a, b) // expected-warning {{variable 'c' was never mutated; consider changing to 'let' constant}}
556556

557557
_ = InitTwo(a, b)
558558
_ = InitTwo((a, b)) // expected-error {{missing argument for parameter #2 in call}}
@@ -606,9 +606,9 @@ do {
606606
}
607607

608608
do {
609-
var a = 3
610-
var b = 4
611-
var d = (a, b)
609+
var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
610+
var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
611+
var d = (a, b) // expected-warning {{variable 'd' was never mutated; consider changing to 'let' constant}}
612612

613613
var s1 = SubscriptTwo()
614614
_ = s1[a, b]
@@ -1013,9 +1013,9 @@ do {
10131013
}
10141014

10151015
do {
1016-
var a = 3
1017-
var b = 4
1018-
var c = (a, b)
1016+
var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
1017+
var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
1018+
var c = (a, b) // expected-warning {{variable 'c' was never mutated; consider changing to 'let' constant}}
10191019

10201020
_ = GenericInit<(Int, Int)>(a, b) // expected-error {{extra argument in call}}
10211021
_ = GenericInit<(Int, Int)>((a, b))
@@ -1094,9 +1094,9 @@ do {
10941094
}
10951095

10961096
do {
1097-
var a = 3.0
1098-
var b = 4.0
1099-
var d = (a, b)
1097+
var a = 3.0 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
1098+
var b = 4.0 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
1099+
var d = (a, b) // expected-warning {{variable 'd' was never mutated; consider changing to 'let' constant}}
11001100

11011101
var s1 = GenericSubscript<(Double, Double)>()
11021102
_ = s1[a, b] // expected-error {{extra argument in call}}

‎test/Parse/availability_query.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ if 1 != 2, #available(iOS 8.0, *) {}
9797

9898
// Pattern then #available(iOS 8.0, *) {
9999
if case 42 = 42, #available(iOS 8.0, *) {}
100-
if let x = Optional(42), #available(iOS 8.0, *) {}
100+
if let _ = Optional(42), #available(iOS 8.0, *) {}
101101

102102
// Allow "macOS" as well.
103103
if #available(macOS 10.51, *) {

‎test/Parse/brace_recovery_eof.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22

33
// Make sure source ranges satisfy the verifier.
44
for foo in [1, 2] { // expected-note {{to match this opening '{'}}
5-
var x = foo
5+
_ = foo
66
// expected-error@+1{{expected '}' at end of brace statement}}

0 commit comments

Comments
 (0)