I had sent this patch during stage-3 of gcc-4.9.
Is the patch OK ?
On Thu, Feb 20, 2014 at 6:26 PM, Marek Polacek <pola...@redhat.com> wrote:
> On Thu, Feb 20, 2014 at 05:52:09PM +0530, Prathamesh Kulkarni wrote:
>> Show column number of left operand instead of comma operator
>> in the warning "left-hand operand of comma expression has no effect"
>>
>> Example:
>> ax.c:4:6: warning: left-hand operand of comma expression has no effect
>> [-Wunused-value]
>> x , y;
>> ^
>
> Perhaps a PR should be opened for this.
>
>> Instead of comma operator, show location of left-operand:
>> ax.c:4:3: warning: left-hand operand of comma expression has no effect
>> [-Wunused-value]
>> x , y;
>> ^
>
> But this patch only handles LHS of comma expression, and not RHS,
> right? IMHO we should do both at the same time (that would be
> for 5.0 I'd say).
>
>> Bootstrapped on x86_64-unknown-linux-gnu.
>>
>> [gcc/c]
>> * c-parser.c (c_parser_expression): Pass tloc instead of loc to
>> build_compound_expr.
>>
>> [gcc/testsuite]
>> * gcc.dg/Wunused-value-4.c: New test case.
>>
>> I am not able to figure out, how to write test-case that
>> raises multiple warnings. example: (void) x, y, z.
>> I tried this:
>> /* { dg-warning "8:left-hand operand of comma expression has no effect
>> |11:left-hand operand of comma expression has no
>> effect" } */
>> x has column number 8 and y has column number 11, but this doesn't
>> seem to work (the patch works correctly).
>
> For this you'll need a second dg-warning that specifies the line, so
> e.g. something like
> /* { dg-warning "11:left-hand operand of comma expression has no effect" "" {
> target *-*-* } 10 } */
>
> (Not reviewing the patch per se now.)
>
>> Thanks and Regards,
>> Prathamesh
>
>> Index: gcc/c/c-parser.c
>> ===================================================================
>> --- gcc/c/c-parser.c (revision 207916)
>> +++ gcc/c/c-parser.c (working copy)
>> @@ -7838,7 +7838,6 @@ c_parser_expression (c_parser *parser)
>> {
>> struct c_expr next;
>> tree lhsval;
>> - location_t loc = c_parser_peek_token (parser)->location;
>> location_t expr_loc;
>> c_parser_consume_token (parser);
>> expr_loc = c_parser_peek_token (parser)->location;
>> @@ -7849,9 +7848,10 @@ c_parser_expression (c_parser *parser)
>> mark_exp_read (lhsval);
>> next = c_parser_expr_no_commas (parser, NULL);
>> next = convert_lvalue_to_rvalue (expr_loc, next, true, false);
>> - expr.value = build_compound_expr (loc, expr.value, next.value);
>> + expr.value = build_compound_expr (tloc, expr.value, next.value);
>> expr.original_code = COMPOUND_EXPR;
>> expr.original_type = next.original_type;
>> + tloc = expr_loc;
>> }
>> return expr;
>> }
>> Index: gcc/testsuite/gcc.dg/Wunused-value-4.c
>> ===================================================================
>> --- gcc/testsuite/gcc.dg/Wunused-value-4.c (revision 0)
>> +++ gcc/testsuite/gcc.dg/Wunused-value-4.c (working copy)
>> @@ -0,0 +1,12 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-Wunused-value" } */
>> +
>> +void foo(int x);
>> +
>> +int a[10];
>> +
>> +void f(void)
>> +{
>> + foo ((1, 2)); /* { dg-warning "9: left-hand operand of
>> comma expression has no effect" } */
>> + a[0x03, 004] = 1992; /* { dg-warning "5: left-hand operand of
>> comma expression has no effect" } */
>> +}
>
> Marek
