Warn on constants in logical expressions

Warn on code such as:

return x || 5;

Submitted at revision 132327.

[chandlerc, 2011-05-26 10:17:17]

http://codereview.appspot.com/4444068/diff/1/test/SemaCXX/expressions.cpp
File test/SemaCXX/expressions.cpp (right):

http://codereview.appspot.com/4444068/diff/1/test/SemaCXX/expressions.cpp#new...
test/SemaCXX/expressions.cpp:43: return x || false;
It'd be good to add a test for 0 and 1 not warning, and a comment about why its
intentional so someone doesn't "fix" this later.

http://codereview.appspot.com/4444068/diff/1/test/SemaCXX/expressions.cpp#new...
test/SemaCXX/expressions.cpp:49: return x || (0); // expected-warning {{use of
logical || with constant operand; switch to bitwise | or remove constant}}
I find it surprising that we don't ignore parens in the implementation of this
warning. Was that intentional? It'd be good to comment here or there as to why.

[rtrieu, 2011-05-28 01:45:08]

http://codereview.appspot.com/4444068/diff/1/test/SemaCXX/expressions.cpp
File test/SemaCXX/expressions.cpp (right):

http://codereview.appspot.com/4444068/diff/1/test/SemaCXX/expressions.cpp#new...
test/SemaCXX/expressions.cpp:43: return x || false;
On 2011/05/26 10:17:17, chandlerc wrote:
> It'd be good to add a test for 0 and 1 not warning, and a comment about why
its
> intentional so someone doesn't "fix" this later.
These tests have been added to Sema/exprs.c since 0 and 1 will only warn on
non-bool languages like C.

http://codereview.appspot.com/4444068/diff/1/test/SemaCXX/expressions.cpp#new...
test/SemaCXX/expressions.cpp:49: return x || (0); // expected-warning {{use of
logical || with constant operand; switch to bitwise | or remove constant}}
On 2011/05/26 10:17:17, chandlerc wrote:
> I find it surprising that we don't ignore parens in the implementation of this
> warning. Was that intentional? It'd be good to comment here or there as to
why.

Parens are ignored since the expression is evaluated.  A comment about ignored
parens was added to the code.

[chandlerc, 2011-05-28 07:17:31]

Please go ahead and mail this to cfe-commits. I want to give folks a chance to
grumble on this one.

http://codereview.appspot.com/4444068/diff/4001/test/SemaCXX/expressions.cpp
File test/SemaCXX/expressions.cpp (right):

http://codereview.appspot.com/4444068/diff/4001/test/SemaCXX/expressions.cpp#...
test/SemaCXX/expressions.cpp:51: return x || (-1); // expected-warning {{use of
logical || with constant operand; switch to bitwise | or remove constant}}
FYI, i'd only use parentheses where it matters or in one place that is clearly
just testing their removal with (((x))), etc. The reason is I keep thinking
they're significant here for some reason...

http://codereview.appspot.com/4444068/diff/4001/test/SemaCXX/switch.cpp
File test/SemaCXX/switch.cpp (right):

http://codereview.appspot.com/4444068/diff/4001/test/SemaCXX/switch.cpp#newco...
test/SemaCXX/switch.cpp:12: switch (n && m) { // expected-warning {{bool}}
Why not just 'n && true' here?

[rtrieu, 2011-05-29 19:41:24]

http://codereview.appspot.com/4444068/diff/4001/test/SemaCXX/expressions.cpp
File test/SemaCXX/expressions.cpp (right):

http://codereview.appspot.com/4444068/diff/4001/test/SemaCXX/expressions.cpp#...
test/SemaCXX/expressions.cpp:51: return x || (-1); // expected-warning {{use of
logical || with constant operand; switch to bitwise | or remove constant}}
On 2011/05/28 07:17:31, chandlerc wrote:
> FYI, i'd only use parentheses where it matters or in one place that is clearly
> just testing their removal with (((x))), etc. The reason is I keep thinking
> they're significant here for some reason...

Duplicated the test cases without the parens.

http://codereview.appspot.com/4444068/diff/4001/test/SemaCXX/switch.cpp
File test/SemaCXX/switch.cpp (right):

http://codereview.appspot.com/4444068/diff/4001/test/SemaCXX/switch.cpp#newco...
test/SemaCXX/switch.cpp:12: switch (n && m) { // expected-warning {{bool}}
On 2011/05/28 07:17:31, chandlerc wrote:
> Why not just 'n && true' here?

Done.