Extend IdenticalExprChecker to find more potential problems

[Daniel Fahlgren]

Hi,

On Tue, 2014-02-04 at 19:54 -0800, Jordan Rose wrote:

> Hm, you're right; the current code doesn't handle this, and perhaps
> shouldn't (even in the non-empty case). How about this case, though?

> if (foo()) {
> doSomething();
> } else if (bar()) {
> /* no action needed */
> } else {
> /* FIXME: Implement this. */
> }

> This one will get flagged, correct? Should it?

> (You could argue that this isn't great code, but since it's easy to
> special-case this I wanted to ask what you think.)

Yes it will emit a warning, and I think it should. With code written
like this it's easy to see that the warning is harmless, or fix it. If
you end up with two empty branches after the preprocessing the warning
is more valid. The function call to bar() might be expensive and should
be removed, or have side effects that must be handled in one of the
branches. Calling bar() as condition to an if statement also indicates
that you should run some code based on its return value, and now you
don't.

I have added a test case for this.

> > > 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.

> I would put them in anyway, then, even if there's no warning today.

Ok. I have added them to the test cases with a fixme. New patches
attached.

Cheers,
Daniel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: if_stmt.patch
Type: text/x-patch
Size: 17722 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140205/196ba522/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bin_op.patch
Type: text/x-patch
Size: 7430 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140205/196ba522/attachment-0001.bin>