From 87732c66576076a92632abb97a105a4b8cf8493f Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Sat, 10 May 2014 02:56:01 -0700 Subject: [PATCH] Added unused variable check, and then used it to remove some unused variables --- analysis/run.d | 4 +- analysis/unused.d | 283 ++++++++++++++++++++++++++++++++++++++++++++++ main.d | 1 - std/d/parser.d | 40 ++++--- 4 files changed, 305 insertions(+), 23 deletions(-) create mode 100644 analysis/unused.d diff --git a/analysis/run.d b/analysis/run.d index c86a24a..c211d75 100644 --- a/analysis/run.d +++ b/analysis/run.d @@ -21,6 +21,7 @@ import analysis.objectconst; import analysis.range; import analysis.constructors; import analysis.ifelsesame; +import analysis.unused; void messageFunction(string fileName, size_t line, size_t column, string message, bool isError) @@ -73,6 +74,7 @@ void analyze(File output, string[] fileNames, bool staticAnalyze = true) checks ~= new BackwardsRangeCheck(fileName); checks ~= new IfElseSameCheck(fileName); checks ~= new ConstructorCheck(fileName); + checks ~= new UnusedVariableCheck(fileName); foreach (check; checks) { @@ -84,7 +86,7 @@ void analyze(File output, string[] fileNames, bool staticAnalyze = true) foreach (message; check.messages) set.insert(message); foreach (message; set[]) - writefln("%s(%d:%d)[warn]: %s", message.fileName, message.line, + output.writefln("%s(%d:%d)[warn]: %s", message.fileName, message.line, message.column, message.message); p.deallocateAll(); } diff --git a/analysis/unused.d b/analysis/unused.d new file mode 100644 index 0000000..6c71fce --- /dev/null +++ b/analysis/unused.d @@ -0,0 +1,283 @@ +// Copyright Brian Schott (Sir Alaran) 2014. +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// http://www.boost.org/LICENSE_1_0.txt) + +module analysis.unused; + +import std.d.ast; +import std.d.lexer; +import analysis.base; +import std.container; + +/** + * Checks for unused variables. + */ +class UnusedVariableCheck : BaseAnalyzer +{ + alias visit = BaseAnalyzer.visit; + + this(string fileName) + { + super(fileName); + pushScope(); + } + + override void visit(const Declaration declaration) + { + if (!isOverride) foreach (attribute; declaration.attributes) + isOverride = isOverride || (attribute.storageClass !is null && + attribute.storageClass.token == tok!"override"); + declaration.accept(this); + } + + override void visit(const FunctionDeclaration functionDec) + { + if (functionDec.functionBody is null) + { + functionDec.accept(this); + } + else if (!isOverride) + { + pushScope(); + foreach (parameter; functionDec.parameters.parameters) + visit(parameter); + functionDec.accept(this); + popScope(); + } + } + + override void visit(const AssertExpression assertExpression) + { + interestDepth++; + assertExpression.accept(this); + interestDepth--; + } + + override void visit(const FunctionCallExpression functionCallExpression) + { + interestDepth++; + functionCallExpression.accept(this); + interestDepth--; + } + + override void visit(const SwitchStatement switchStatement) + { + if (switchStatement.expression !is null) + { + interestDepth++; + switchStatement.expression.accept(this); + interestDepth--; + } + switchStatement.accept(this); + } + + override void visit(const WhileStatement whileStatement) + { + interestDepth++; + whileStatement.expression.accept(this); + interestDepth--; + whileStatement.declarationOrStatement.accept(this); + } + + override void visit(const DoStatement doStatement) + { + interestDepth++; + doStatement.expression.accept(this); + interestDepth--; + doStatement.statementNoCaseNoDefault.accept(this); + } + + override void visit(const ForStatement forStatement) + { + if (forStatement.initialization !is null) + forStatement.initialization.accept(this); + if (forStatement.test !is null) + { + interestDepth++; + forStatement.test.accept(this); + interestDepth--; + } + if (forStatement.increment !is null) + { + interestDepth++; + forStatement.increment.accept(this); + interestDepth--; + } + forStatement.declarationOrStatement.accept(this); + } + + override void visit(const IfStatement ifStatement) + { + if (ifStatement.expression !is null) + { + interestDepth++; + ifStatement.expression.accept(this); + interestDepth--; + } + ifStatement.thenStatement.accept(this); + if (ifStatement.elseStatement !is null) + ifStatement.elseStatement.accept(this); + } + + override void visit(const TypeofExpression typeofExpression) {} + + override void visit(const ForeachStatement foreachStatement) + { + if (foreachStatement.low !is null) + { + interestDepth++; + foreachStatement.low.accept(this); + interestDepth--; + } + if (foreachStatement.high !is null) + { + interestDepth++; + foreachStatement.high.accept(this); + interestDepth--; + } + foreachStatement.accept(this); + } + + override void visit(const ArgumentList argumentList) + { + interestDepth++; + argumentList.accept(this); + interestDepth--; + } + + override void visit(const Initializer initializer) + { + interestDepth++; + initializer.accept(this); + interestDepth--; + } + + override void visit(const AssignExpression assignExp) + { + assignExp.ternaryExpression.accept(this); + if (assignExp.assignExpression !is null) + { + interestDepth++; + assignExp.assignExpression.accept(this); + interestDepth--; + } + } + + override void visit(const PrimaryExpression primary) + { + if (interestDepth > 0 && primary.identifierOrTemplateInstance !is null + && primary.identifierOrTemplateInstance.identifier != tok!"") + { + variableUsed(primary.identifierOrTemplateInstance.identifier.text); + } + primary.accept(this); + } + + override void visit(const ReturnStatement retStatement) + { + if (retStatement.expression !is null) + { + interestDepth++; + visit(retStatement.expression); + interestDepth--; + } + } + + override void visit(const BlockStatement blockStatement) + { + bool sb = inAggregateScope; + inAggregateScope = false; + pushScope(); + blockStatement.accept(this); + popScope(); + inAggregateScope = true; + } + + override void visit(const VariableDeclaration variableDeclaration) + { + foreach (d; variableDeclaration.declarators) + this.variableDeclared(d.name.text, d.name.line, d.name.column, false, false); + variableDeclaration.accept(this); + } + + override void visit(const AutoDeclaration autoDeclaration) + { + foreach (t; autoDeclaration.identifiers) + this.variableDeclared(t.text, t.line, t.column, false, false); + autoDeclaration.accept(this); + } + + override void visit(const Parameter parameter) + { + import std.algorithm; + import std.array; + if (parameter.name != tok!"") + variableDeclared(parameter.name.text, parameter.name.line, + parameter.name.column, true, canFind(parameter.parameterAttributes, + cast(IdType) tok!"ref")); + } + + override void visit(const StructBody structBody) + { + bool sb = inAggregateScope; + inAggregateScope = true; + foreach (dec; structBody.declarations) + visit(dec); + inAggregateScope = sb; + } + + void variableDeclared(string name, size_t line, size_t column, + bool isParameter, bool isRef) + { + if (inAggregateScope) + return; + tree[$ - 1].insert(new UnUsed(name, line, column, isParameter, isRef)); + } + + void variableUsed(string name) + { + size_t treeIndex = tree.length - 1; + auto uu = UnUsed(name); + while (true) + { + if (tree[treeIndex].removeKey(&uu) != 0 || treeIndex == 0) + break; + treeIndex--; + } + } + + void popScope() + { + foreach (uu; tree[$ - 1]) + { + if (!uu.isRef) + addErrorMessage(uu.line, uu.column, + (uu.isParameter ? "Parameter " : "Variable ") + ~ uu.name ~ " is never used"); + } + tree = tree[0 .. $ - 1]; + } + + void pushScope() + { + tree ~= new RedBlackTree!(UnUsed*, "a.name < b.name"); + } + + struct UnUsed + { + string name; + size_t line; + size_t column; + bool isParameter; + bool isRef; + } + + RedBlackTree!(UnUsed*, "a.name < b.name")[] tree; + + uint interestDepth; + + bool isOverride; + + bool inAggregateScope; +} diff --git a/main.d b/main.d index cb011f8..7c23ae8 100644 --- a/main.d +++ b/main.d @@ -31,7 +31,6 @@ int main(string[] args) bool highlight; bool ctags; bool recursive; - bool format; bool help; bool tokenCount; bool syntaxCheck; diff --git a/std/d/parser.d b/std/d/parser.d index f6e84e6..0560a4d 100644 --- a/std/d/parser.d +++ b/std/d/parser.d @@ -399,7 +399,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmAndExp parseAsmAndExp() { - auto node = allocate!AsmAndExp; +// auto node = allocate!AsmAndExp; assert (false, "asm"); // TODO asm } @@ -413,7 +413,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmBrExp parseAsmBrExp() { - auto node = allocate!AsmBrExp; +// auto node = allocate!AsmBrExp; assert (false, "asm"); // TODO asm } @@ -426,7 +426,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmEqualExp parseAsmEqualExp() { - auto node = allocate!AsmEqualExp; +// auto node = allocate!AsmEqualExp; assert (false, "asm"); // TODO asm } @@ -439,7 +439,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmExp parseAsmExp() { - auto node = allocate!AsmExp; +// auto node = allocate!AsmExp; assert (false, "asm"); // TODO asm } @@ -457,7 +457,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmInstruction parseAsmInstruction() { - auto node = allocate!AsmInstruction; +// auto node = allocate!AsmInstruction; assert (false, "asm"); // TODO asm } @@ -470,7 +470,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmLogAndExp parseAsmLogAndExp() { - auto node = allocate!AsmLogAndExp; +// auto node = allocate!AsmLogAndExp; assert (false, "asm"); // TODO asm } @@ -483,7 +483,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmLogOrExp parseAsmLogOrExp() { - auto node = allocate!AsmLogOrExp; +// auto node = allocate!AsmLogOrExp; assert (false, "asm"); // TODO asm } @@ -496,7 +496,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmMulExp parseAsmMulExp() { - auto node = allocate!AsmMulExp; +// auto node = allocate!AsmMulExp; assert (false, "asm"); // TODO asm } @@ -509,7 +509,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmOrExp parseAsmOrExp() { - auto node = allocate!AsmOrExp; +// auto node = allocate!AsmOrExp; assert (false, "asm"); // TODO asm } @@ -526,7 +526,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmPrimaryExp parseAsmPrimaryExp() { - auto node = allocate!AsmPrimaryExp; +// auto node = allocate!AsmPrimaryExp; assert (false, "asm"); // TODO asm } @@ -539,7 +539,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmRelExp parseAsmRelExp() { - auto node = allocate!AsmRelExp; +// auto node = allocate!AsmRelExp; assert (false, "asm"); // TODO asm } @@ -552,7 +552,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmShiftExp parseAsmShiftExp() { - auto node = allocate!AsmShiftExp; +// auto node = allocate!AsmShiftExp; assert (false, "asm"); // TODO asm } @@ -588,7 +588,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmTypePrefix parseAsmTypePrefix() { - auto node = allocate!AsmTypePrefix; +// auto node = allocate!AsmTypePrefix; assert (false, "asm"); // TODO asm } @@ -607,7 +607,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmUnaExp parseAsmUnaExp() { - auto node = allocate!AsmUnaExp; +// auto node = allocate!AsmUnaExp; assert (false, "asm"); // TODO asm } @@ -620,7 +620,7 @@ alias core.sys.posix.stdio.fileno fileno; */ AsmXorExp parseAsmXorExp() { - auto node = allocate!AsmXorExp; +// auto node = allocate!AsmXorExp; assert (false, "asm"); // TODO asm } @@ -3847,7 +3847,7 @@ invariant() foo(); */ Operands parseOperands() { - auto node = allocate!Operands; +// auto node = allocate!Operands; assert (false, "asm"); // TODO asm } @@ -5969,13 +5969,11 @@ q{doStuff(5)}c; auto node = allocate!UnionDeclaration; // grab line number even if it's anonymous auto l = expect(tok!"union").line; - bool templated = false; if (currentIs(tok!"identifier")) { node.name = advance(); if (currentIs(tok!"(")) { - templated = true; node.templateParameters = parseTemplateParameters(); if (currentIs(tok!"if")) node.constraint = parseConstraint(); @@ -6763,8 +6761,8 @@ protected: index = i; } - version (unittest) static void doNothingErrorFunction(string fileName, - size_t line, size_t column, string message, bool isError) {} + version (unittest) static void doNothingErrorFunction(string, + size_t, size_t, string, bool) {} version (unittest) static Parser getParserForUnittest(string sourceCode, string testName) @@ -6843,7 +6841,7 @@ protected: } else { - void trace(lazy string message) {} + void trace(lazy string) {} } enum string BASIC_TYPE_CASES = q{