-
Notifications
You must be signed in to change notification settings - Fork 68
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
Fix for #976 to Ignore Unused Stock Variables #979
Conversation
This change treats stock variables the same as public during when checking for unused variables.
This error is intentional. The warning should only elide on includes. |
Yup, okay. I will go to take a look tomorrow for whether the messages were present from includes. Then, take another look at a code fix. |
Yes, there are messages from inclusions. #976 |
This changes to also check swhether the VarDecl is from the main source file or not. Checking this seems easy enough with the new source file tracking. If a variable is from the main source file even if it is marked with stock we shall now report a warning. We could also ignore any unused variable that is not from the main file. But I shall stick with only stock for now. Testing Extended out the test to cover both the pass and fail cases.
Updated to elide the warning on includes. |
} | ||
|
||
template <typename DeclType> | ||
bool Semantics::IsIncludedStock(DeclType* sym) { |
There was a problem hiding this comment.
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.
@@ -2307,6 +2307,18 @@ bool Semantics::CheckExprStmt(ExprStmt* stmt) { | |||
return true; | |||
} | |||
|
|||
bool Semantics::IsIncluded(Decl* sym) { | |||
const auto fileno = cc().sources()->GetSourceFileIndex(sym->pos()); |
There was a problem hiding this comment.
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.
const auto fileno = cc().sources()->GetSourceFileIndex(sym->pos()); | ||
if (fileno >= cc().sources()->opened_files().size()) { return false; } | ||
|
||
return ! cc().sources()->opened_files()[fileno]->is_main_file(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "!cc_" (no space)
@@ -2307,6 +2307,18 @@ bool Semantics::CheckExprStmt(ExprStmt* stmt) { | |||
return true; | |||
} | |||
|
|||
bool Semantics::IsIncluded(Decl* sym) { | |||
const auto fileno = cc().sources()->GetSourceFileIndex(sym->pos()); | |||
if (fileno >= cc().sources()->opened_files().size()) { return false; } |
There was a problem hiding this comment.
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.
The problem still exists with global variables, could you fix your PR? |
I'll fix up and land this. |
Solution
This change looks to ignore stock variables that are not from the main source file.
Testing
Added two new compile tests for stock variables and arrays.