feat(unused_var): analyze global variables inside function body #2944
Conversation
|
Thanks! I will look this evening.. |
danmar
left a comment
danmar
left a comment
There was a problem hiding this comment.
small nits.. I will test it out a bit..
danmar
left a comment
danmar
left a comment
There was a problem hiding this comment.
hmm I have tried it out and it works better than I thought!
| // check if pointer to global variable assigned to another variable (another_var = &global_var) | ||
| if (Token::simpleMatch(bodyToken->tokAt(-1), "&") && Token::simpleMatch(bodyToken->tokAt(-2), "=")) { | ||
| const Token* assigned_var_token = bodyToken->tokAt(-3); | ||
| if (assigned_var_token && assigned_var_token->variable()) |
There was a problem hiding this comment.
if this condition is true then you don't care that the address is used somehow.. would probably be safer to return false then.
There was a problem hiding this comment.
As I see, isVariableChanged() works fine enough with "pointer-to-global" variables. I prefer to leave it as is, if you not insist.
Do you see any test cases which will force this code work not as expected?
There was a problem hiding this comment.
I think it works well.. right now I am only worried about the todo test case. Do you know it it's complex to fix that?
There was a problem hiding this comment.
Probably, it should be not so complex. But it can affect existing tests and I will need some time to figure out with them.
Also I can not estimate a free time amount I will be able to put on this work in the nearest time. =)
How about this one - I will return false for now and update astutils.cpp and this piece of code in next PR?
There was a problem hiding this comment.
It's fine to make a partial fix.. and finish later. but then I feel more comfortable if there are no false positives in the meantime. So well.. could you return false if there is a global array/pointer variable.
lib/checkunusedvar.cpp
Outdated
| } | ||
| // check if global variable is changed | ||
| if (bodyVariable->isGlobal() || | ||
| (std::find(pointersToGlobals.begin(), pointersToGlobals.end(), bodyVariable) != pointersToGlobals.end())) |
There was a problem hiding this comment.
Now we can use pointersToGlobals.find(bodyVariable) != pointersToGlobals.end() instead here.
|
Thanks! |
Welcome. |
2 participants
Before this PR, any global variable found in function body marked this function as side-effect one. Analyzing global variables more precisely from now.
Any help with missed test cases will be highly appreciated.