[C PATCH] fix column number in comma expression warning

[Prathamesh Kulkarni]

On Thu, Feb 20, 2014 at 6:26 PM, Marek Polacek <polacek@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).
There's a check for rhs of comma expression in build_compound_expr,
which uses correct location. I am not sure which case fires the
warning for rhs though.
>
>> 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 } */
Thanks. That worked.
>
> (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