Remove check for mixed signs on conditional operands from -Wsign-compare

Patch added at revision 135664.

[chandlerc, 2011-07-15 22:28:36]

http://codereview.appspot.com/4731042/diff/1001/test/Sema/conditional-expr.c
File test/Sema/conditional-expr.c (right):

http://codereview.appspot.com/4731042/diff/1001/test/Sema/conditional-expr.c#...
test/Sema/conditional-expr.c:54: test0 = test0 ? EVal : (int) test0; //
expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
Why is this now warning?

http://codereview.appspot.com/4731042/diff/1001/test/SemaCXX/compare.cpp
File test/SemaCXX/compare.cpp (right):

http://codereview.appspot.com/4731042/diff/1001/test/SemaCXX/compare.cpp#newc...
test/SemaCXX/compare.cpp:216: // -Wsign-compare should not warn when ?: operands
have different signedness.
maybe mention that we use -Wsign-conversion for that.

http://codereview.appspot.com/4731042/diff/1001/test/SemaCXX/conditional-expr...
File test/SemaCXX/conditional-expr.cpp (right):

http://codereview.appspot.com/4731042/diff/1001/test/SemaCXX/conditional-expr...
test/SemaCXX/conditional-expr.cpp:197: test0 = test0 ? EVal : (int) test0; //
expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
Another place where we now warn more often...

http://codereview.appspot.com/4731042/diff/1001/test/SemaCXX/conditional-expr...
test/SemaCXX/conditional-expr.cpp:331: 
Why the whitespace?

[rtrieu, 2011-07-16 00:44:26]

http://codereview.appspot.com/4731042/diff/1001/test/Sema/conditional-expr.c
File test/Sema/conditional-expr.c (right):

http://codereview.appspot.com/4731042/diff/1001/test/Sema/conditional-expr.c#...
test/Sema/conditional-expr.c:54: test0 = test0 ? EVal : (int) test0; //
expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
On 2011/07/15 22:28:37, chandlerc wrote:
> Why is this now warning?

test0 is unsigned.  Both EVal and (int) test0 are signed integers.  When either
one is assigned to test0, they must be sign converted, and thus get warned on.

http://codereview.appspot.com/4731042/diff/1001/test/SemaCXX/compare.cpp
File test/SemaCXX/compare.cpp (right):

http://codereview.appspot.com/4731042/diff/1001/test/SemaCXX/compare.cpp#newc...
test/SemaCXX/compare.cpp:216: // -Wsign-compare should not warn when ?: operands
have different signedness.
On 2011/07/15 22:28:37, chandlerc wrote:
> maybe mention that we use -Wsign-conversion for that.

Done.

http://codereview.appspot.com/4731042/diff/1001/test/SemaCXX/conditional-expr...
File test/SemaCXX/conditional-expr.cpp (right):

http://codereview.appspot.com/4731042/diff/1001/test/SemaCXX/conditional-expr...
test/SemaCXX/conditional-expr.cpp:197: test0 = test0 ? EVal : (int) test0; //
expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
On 2011/07/15 22:28:37, chandlerc wrote:
> Another place where we now warn more often...

test0 is unsigned.  Both EVal and (int) test0 are signed integers.  When either
one is assigned to test0, they must be sign converted, and thus get warned on.

http://codereview.appspot.com/4731042/diff/1001/test/SemaCXX/conditional-expr...
test/SemaCXX/conditional-expr.cpp:331: 
On 2011/07/15 22:28:37, chandlerc wrote:
> Why the whitespace?

Removed

[chandlerc, 2011-07-16 00:59:56]

http://codereview.appspot.com/4731042/diff/1001/test/Sema/conditional-expr.c
File test/Sema/conditional-expr.c (right):

http://codereview.appspot.com/4731042/diff/1001/test/Sema/conditional-expr.c#...
test/Sema/conditional-expr.c:54: test0 = test0 ? EVal : (int) test0; //
expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
On 2011/07/16 00:44:26, rtrieu wrote:
> On 2011/07/15 22:28:37, chandlerc wrote:
> > Why is this now warning?
> 
> test0 is unsigned.  Both EVal and (int) test0 are signed integers.  When
either
> one is assigned to test0, they must be sign converted, and thus get warned on.

Maybe get a signed value to assign to? That way its more clear that the ?:
doesn't warn.

Also, this diagnostic is really confusing -- it makes it sound like only one of
the operands would be subject to this conversion when they both would... but
that is likely an orthogonal issue...

[rtrieu, 2011-07-19 03:58:49]

http://codereview.appspot.com/4731042/diff/1001/test/Sema/conditional-expr.c
File test/Sema/conditional-expr.c (right):

http://codereview.appspot.com/4731042/diff/1001/test/Sema/conditional-expr.c#...
test/Sema/conditional-expr.c:54: test0 = test0 ? EVal : (int) test0; //
expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
On 2011/07/16 00:59:56, chandlerc wrote:
> On 2011/07/16 00:44:26, rtrieu wrote:
> > On 2011/07/15 22:28:37, chandlerc wrote:
> > > Why is this now warning?
> > 
> > test0 is unsigned.  Both EVal and (int) test0 are signed integers.  When
> either
> > one is assigned to test0, they must be sign converted, and thus get warned
on.
> 
> Maybe get a signed value to assign to? That way its more clear that the ?:
> doesn't warn.
> 
> Also, this diagnostic is really confusing -- it makes it sound like only one
of
> the operands would be subject to this conversion when they both would... but
> that is likely an orthogonal issue...

Added a comment to clarify the warning reason.  Moved the related test cases
together.

[chandlerc, 2011-07-19 08:03:05]

http://codereview.appspot.com/4731042/diff/1001/test/Sema/conditional-expr.c
File test/Sema/conditional-expr.c (right):

http://codereview.appspot.com/4731042/diff/1001/test/Sema/conditional-expr.c#...
test/Sema/conditional-expr.c:54: test0 = test0 ? EVal : (int) test0; //
expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
On 2011/07/19 03:58:50, rtrieu wrote:
> On 2011/07/16 00:59:56, chandlerc wrote:
> > On 2011/07/16 00:44:26, rtrieu wrote:
> > > On 2011/07/15 22:28:37, chandlerc wrote:
> > > > Why is this now warning?
> > > 
> > > test0 is unsigned.  Both EVal and (int) test0 are signed integers.  When
> > either
> > > one is assigned to test0, they must be sign converted, and thus get warned
> on.
> > 
> > Maybe get a signed value to assign to? That way its more clear that the ?:
> > doesn't warn.
> > 
> > Also, this diagnostic is really confusing -- it makes it sound like only one
> of
> > the operands would be subject to this conversion when they both would... but
> > that is likely an orthogonal issue...
> 
> Added a comment to clarify the warning reason.  Moved the related test cases
> together.

I like the grouping, but why not just move 'test1's declaration up from line 61
to line 57, and assign to test1 which is signed? Then there won't be any warning
to explain...

[rtrieu, 2011-07-19 19:40:36]

http://codereview.appspot.com/4731042/diff/1001/test/Sema/conditional-expr.c
File test/Sema/conditional-expr.c (right):

http://codereview.appspot.com/4731042/diff/1001/test/Sema/conditional-expr.c#...
test/Sema/conditional-expr.c:54: test0 = test0 ? EVal : (int) test0; //
expected-warning {{operand of ? changes signedness: 'int' to 'unsigned long'}}
On 2011/07/19 08:03:05, chandlerc wrote:
> On 2011/07/19 03:58:50, rtrieu wrote:
> > On 2011/07/16 00:59:56, chandlerc wrote:
> > > On 2011/07/16 00:44:26, rtrieu wrote:
> > > > On 2011/07/15 22:28:37, chandlerc wrote:
> > > > > Why is this now warning?
> > > > 
> > > > test0 is unsigned.  Both EVal and (int) test0 are signed integers.  When
> > > either
> > > > one is assigned to test0, they must be sign converted, and thus get
warned
> > on.
> > > 
> > > Maybe get a signed value to assign to? That way its more clear that the ?:
> > > doesn't warn.
> > > 
> > > Also, this diagnostic is really confusing -- it makes it sound like only
one
> > of
> > > the operands would be subject to this conversion when they both would...
but
> > > that is likely an orthogonal issue...
> > 
> > Added a comment to clarify the warning reason.  Moved the related test cases
> > together.
> 
> I like the grouping, but why not just move 'test1's declaration up from line
61
> to line 57, and assign to test1 which is signed? Then there won't be any
warning
> to explain...

Done.