| Message ID | CAJXstsAzQ1F5b9vXtOtfvHKPgCU53egXuKwCLJ+XOxdSfRzSUg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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
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 <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). > >> 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
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" } */ +}
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; ^ 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; ^ 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). Thanks and Regards, Prathamesh