Repository navigation
Fix #14769 FP unreadVariable with address in chained assignment of unused variable - #8926
chrchr-github wants to merge 2 commits into
Conversation
|
Test results for commit dddc7c8 (tools/test-my-pr.py, main compared to this PR): Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8926/ Posted automatically by the cppcheck PR test runner. +N: warnings only with this PR, -N: warnings only with main. The AI review is written by Claude and can be wrong. |
|
@claude review |
|
|
||
| // checked for chained assignments | ||
| if (tok != start && equal && equal->str() == "=") { | ||
| if (tok != start && (Token::simpleMatch(equal, "=") || (start->astParent() && Token::simpleMatch(start->astParent()->astParent(), "=")))) { |
There was a problem hiding this comment.
This new condition doesn't look at tok at all. It is true whenever start is the target of an assignment that sits inside another =. When that happens, the code calls variables.read() on whatever variable doAssignment returned. For a = c = &i; that variable is i, the one whose address is taken. So i is now marked as read directly instead of through the alias. That hides the FP, but it may also hide real warnings. For example:
void f() { int i = 2; int *a, *c; a = c = &i; }Here i is probably no longer reported, because *a is never used. Can you add a negative test like this one, or limit the check to the case where tok is an operand of the address-of operator? It would also help to add a test for a chained compound assignment such as x = y += 1;, since that path now goes through tok = tok->previous() as well.
|
The fix works for the reported case, but the new chained-assignment condition is broad. It reads the variable that 🤖 Generated with Claude Code |
No description provided.