Bugzilla – Bug 2550
dead assignment on object-base.cc
Last modified: 2017-02-21 16:25:52 UTC
Here the problem is a bit more interesting. In function ConstructSelf (line 80), there is an assignment to the variable "found" (line 123) that is never read, because there is a "continue" right after. Out of the loop, there is no reference to "found" ... so we basically never enter the (!found) branch, never checking the env settings and never setting the default value (even if I don't understand, if there is no attribute, how we can set the default value?!). I can't provide the patch because I'm scared into working on the core/ subdirectory; can anyone take a look? Thanks
(In r 12400 3dd69b81ac12, the declaration is line 115) Stripping out the work, the skeleton of lines 115-169 is: bool found = false; if (value != 0) { // We have a matching attribute value. if (DoSet (info.accessor, info.checker, *value)) { found = true; continue; } } if (!found) { // No matching attribute value so we try to look at the env var. ... found = true; ... } if (!found) { // No matching attribute value so we try to set the default value. } As you note, the "if (value !=0)" stanza doesn't need found. As a result the first "if (!found)" stanza is guaranteed to execute, so doesn't need the test. However, that stanza might succeed, so the second "if (!found)" test is required, as is the flag found. I agree the declaration could move down one stanza, but it's not an unused variable. I propose setting this bug to RESOLVED INVALID, unless Natale disagrees.
The static analyzer signals that is a dead assignment, not an unused variable. I was wrong in explaining the situation (and you corrected it clearly), but the problem still persists, we have a dead assignment. When you assign to that variable the true value, it is never read because then there is a continue, and then the variable is initialized again with the false value. Merging this in what you said, I think that the variable can be eliminated without problems: if (value == 0 || !DoSet (info.accessor, info.checker, *value)) { if (env ...) { } else { } } What do you think ?
I find that conditional too hard to read. I'd prefer if (value != 0) { // We have a matching attribute value. if (DoSet (info.accessor, info.checker, *value)) { continue; } } bool found = false; // No matching attribute value so we try to look at the env var. ... found = true; ... if (!found) { // No matching attribute value so we try to set the default value. }
Yes, that would be ok. Thank you!
Fixed and pushed in r12701:4a05435b2208