Skip to content

Fix for #976 to Ignore Unused Stock Variables #979

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions compiler/semantics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2307,6 +2307,18 @@ bool Semantics::CheckExprStmt(ExprStmt* stmt) {
return true;
}

bool Semantics::IsIncluded(Decl* sym) {
const auto fileno = cc().sources()->GetSourceFileIndex(sym->pos());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc() should be cc_, here and elsewhere.

if (fileno >= cc().sources()->opened_files().size()) { return false; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: no single-line conditionals. should be:

if (fileno >= cc().sources()->opened_files().size()
    return false;

Though, I think this condition is unnecessary. We should never get a source index for a file not in the open list.


return ! cc().sources()->opened_files()[fileno]->is_main_file();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "!cc_" (no space)

}

template <typename DeclType>
bool Semantics::IsIncludedStock(DeclType* sym) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be VarDeclBase, no template needed.

return sym->is_stock() && IsIncluded(sym);
}

/* testsymbols - test for unused local or global variables
*
* "Public" functions are excluded from the check, since these
Expand Down Expand Up @@ -2355,9 +2367,14 @@ bool Semantics::TestSymbol(Decl* sym, bool testconst) {
default: {
auto var = sym->as<VarDeclBase>();
/* a variable */
if (!var->is_used() && !var->is_public()) {
report(sym, 203) << sym->name(); /* symbol isn't used (and not public) */
} else if (!var->is_public() && !var->is_read()) {

// We ignore variables that are marked as public or stock that was included.
if (var->is_public() || IsIncludedStock(var))
break;

if (!var->is_used()) {
report(sym, 203) << sym->name(); /* symbol isn't used (and not public/stock) */
} else if (!var->is_read()) {
report(sym, 204) << sym->name(); /* value assigned to symbol is never used */
}
}
Expand Down
5 changes: 5 additions & 0 deletions compiler/semantics.h
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,11 @@ class Semantics final

bool CheckScalarType(Expr* expr);

bool IsIncluded(Decl* expr);

template <typename DeclType>
bool IsIncludedStock(DeclType* expr);

private:
CompileContext& cc_;
TypeManager* types_ = nullptr;
Expand Down
9 changes: 9 additions & 0 deletions tests/compile-only/fail-stock-array-unused.sp
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// warnings_are_errors: true

stock const char messages[2][] =
{
"first",
"second"
};

public void main() {}
1 change: 1 addition & 0 deletions tests/compile-only/fail-stock-array-unused.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(3) : error 203: symbol is never used: "messages"
5 changes: 5 additions & 0 deletions tests/compile-only/fail-stock-var-unused.sp
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// warnings_are_errors: true

stock const int X = 1;

public void main() {}
1 change: 1 addition & 0 deletions tests/compile-only/fail-stock-var-unused.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(3) : error 204: symbol is assigned a value that is never used: "X"
5 changes: 5 additions & 0 deletions tests/compile-only/ok-included-stock-array-unused.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
stock const char messages[2][] =
{
"first",
"second"
};
5 changes: 5 additions & 0 deletions tests/compile-only/ok-included-stock-array-unused.sp
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// warnings_are_errors: true

#include "ok-included-stock-array-unused.inc"

public void main() {}
1 change: 1 addition & 0 deletions tests/compile-only/ok-included-stock-var-unused.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
stock const int X = 1;
5 changes: 5 additions & 0 deletions tests/compile-only/ok-included-stock-var-unused.sp
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// warnings_are_errors: true

#include "ok-included-stock-var-unused.inc"

public void main() {}
Loading