Extend IdenticalExprChecker to find more potential problems
Daniel Fahlgren
daniel at fahlgren.se
Tue Feb 4 13:05:58 PST 2014
Hi Jordan,
On Mon, 2014-02-03 at 18:37 -0800, Jordan Rose wrote:
> Thanks for running the timing checks. I agree that this seems pretty
> benign.
>
> I thought of another controversial test case:
> if (foo()) {
> doSomething();
> } else if (bar()) {
> /* do nothing */
> } else if (baz()) {
> /* do nothing */
> } else {
> error();
> }
> It'd be pretty easy to just not check empty compound-statements. What
> do you think?
I fail to understand the problem. Can you elaborate, please? If we
rearrange the code a bit we get
if (foo()) {
doSomething();
} else
if (bar()) {
/* do nothing */
} else
if (baz()) {
/* do nothing */
} else {
error();
}
with no two if-then-else-branches identical. The current code doesn't
(correctly?) warn about this.
A future addition might be to detect the same logical expressions in
multiple nested if statements, like:
if (i == 1) {
doSomething1();
} else if (i == 2) {
doSomething2();
} else if (i == 2) {
doSomething3();
} else {
error();
}
>
> More test cases for the logical op checker:
> x == 4 && y == 5 || x == 4
> x == 4 || y == 5 && x == 4
> x == 4 || x == 4 && y == 5
>
> All of these would be fair to warn on, but we can probably get away
> with not for the first pass. If we already support one, though, should
> we add it as a regression test?
Unfortunately we don't support that right now, but I agree it would be a
good addition.
> Finally, the typo "comparision" is still in the patch. :-)
I should have paid more attention. Sorry :-) Attached is a new version
of the second patch.
>
Cheers,
Daniel Fahlgren
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bin_op.patch
Type: text/x-patch
Size: 7174 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140204/658d2baa/attachment.bin>
More information about the cfe-commits
mailing list