| Message ID | 20160211194424.GF3017@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Feb 11, 2016 at 08:44:24PM +0100, Jakub Jelinek wrote: > Hi! > > On Thu, Feb 11, 2016 at 01:56:09PM -0500, Jason Merrill wrote: > > On 02/11/2016 11:25 AM, Jakub Jelinek wrote: > > >+ && !integer_zerop (tree_strip_nop_conversions (op1))) > > > > Maybe cp_fold rather than tree_strip_nop_conversions? > > Is it safe to call cp_fully_fold (typeck.c only calls it by that name, > not cp_fold) on perhaps type or value dependent argument? > E.g. on > template <int N> > int > bar () > { > return "foo1" != (void *) N > && "foo2" != (const char *) ((void *) N) > && "foo3" != (const char *) ((void *) (N - N)) > && "foo4" != (const char *) ((void *) (&e - &e)) > && "foo5" != "foo6"; > } > op1 is NON_DEPENDENT_EXPR. If yes, here is an alternative (that has the same > behavior on the testcase as 5.x does). The C FE didn't use to fold it, so > I'm preserving its behavior. > > 2016-02-11 Jakub Jelinek <jakub@redhat.com> > > PR c/69768 > * c-typeck.c (parser_build_binary_op): Strip nops from integer_zerop > arguments for -Waddress warning. > > * typeck.c (cp_build_binary_op): cp_fully_fold integer_zerop > arguments for -Waddress warning. Fix up formatting. > > * c-c++-common/Waddress-1.c: New test. > > --- gcc/c/c-typeck.c.jj 2016-02-11 20:28:51.316491659 +0100 > +++ gcc/c/c-typeck.c 2016-02-11 20:29:50.778672554 +0100 > @@ -3597,8 +3597,10 @@ parser_build_binary_op (location_t locat > of testing for equality or inequality of a string literal with NULL. */ > if (code == EQ_EXPR || code == NE_EXPR) > { > - if ((code1 == STRING_CST && !integer_zerop (arg2.value)) > - || (code2 == STRING_CST && !integer_zerop (arg1.value))) > + if ((code1 == STRING_CST > + && !integer_zerop (tree_strip_nop_conversions (arg2.value))) > + || (code2 == STRING_CST > + && !integer_zerop (tree_strip_nop_conversions (arg1.value)))) > warning_at (location, OPT_Waddress, > "comparison with string literal results in unspecified behavior"); > } The C part looks ok to me. Marek
On 02/11/2016 02:44 PM, Jakub Jelinek wrote: > Hi! > > On Thu, Feb 11, 2016 at 01:56:09PM -0500, Jason Merrill wrote: >> On 02/11/2016 11:25 AM, Jakub Jelinek wrote: >>> + && !integer_zerop (tree_strip_nop_conversions (op1))) >> >> Maybe cp_fold rather than tree_strip_nop_conversions? > > Is it safe to call cp_fully_fold (typeck.c only calls it by that name, > not cp_fold) on perhaps type or value dependent argument? In a template cp_fold does nothing, so it's safe but won't warn until instantiation time. OK either way. Jason
--- gcc/c/c-typeck.c.jj 2016-02-11 20:28:51.316491659 +0100 +++ gcc/c/c-typeck.c 2016-02-11 20:29:50.778672554 +0100 @@ -3597,8 +3597,10 @@ parser_build_binary_op (location_t locat of testing for equality or inequality of a string literal with NULL. */ if (code == EQ_EXPR || code == NE_EXPR) { - if ((code1 == STRING_CST && !integer_zerop (arg2.value)) - || (code2 == STRING_CST && !integer_zerop (arg1.value))) + if ((code1 == STRING_CST + && !integer_zerop (tree_strip_nop_conversions (arg2.value))) + || (code2 == STRING_CST + && !integer_zerop (tree_strip_nop_conversions (arg1.value)))) warning_at (location, OPT_Waddress, "comparison with string literal results in unspecified behavior"); } --- gcc/cp/typeck.c.jj 2016-02-11 20:28:51.196493312 +0100 +++ gcc/cp/typeck.c 2016-02-11 20:34:41.124672969 +0100 @@ -4487,9 +4487,12 @@ cp_build_binary_op (location_t location, warning (OPT_Wfloat_equal, "comparing floating point with == or != is unsafe"); if ((complain & tf_warning) - && ((TREE_CODE (orig_op0) == STRING_CST && !integer_zerop (op1)) - || (TREE_CODE (orig_op1) == STRING_CST && !integer_zerop (op0)))) - warning (OPT_Waddress, "comparison with string literal results in unspecified behaviour"); + && ((TREE_CODE (orig_op0) == STRING_CST + && !integer_zerop (cp_fully_fold (op1))) + || (TREE_CODE (orig_op1) == STRING_CST + && !integer_zerop (cp_fully_fold (op0))))) + warning (OPT_Waddress, "comparison with string literal results " + "in unspecified behaviour"); build_type = boolean_type_node; if ((code0 == INTEGER_TYPE || code0 == REAL_TYPE --- gcc/testsuite/c-c++-common/Waddress-1.c.jj 2016-02-11 20:29:50.781672512 +0100 +++ gcc/testsuite/c-c++-common/Waddress-1.c 2016-02-11 20:35:40.299857817 +0100 @@ -0,0 +1,15 @@ +/* PR c/69768 */ +/* { dg-do compile } */ +/* { dg-options "-Waddress" } */ + +static int e; + +int +foo () +{ + return "foo1" != (void *) 0 /* { dg-bogus "comparison with string literal results in unspecified behaviou?r" } */ + && "foo2" != (const char *) ((void *) 0) /* { dg-bogus "comparison with string literal results in unspecified behaviou?r" } */ + && "foo3" != (const char *) ((void *) (10 - 10)) /* { dg-bogus "comparison with string literal results in unspecified behaviou?r" } */ + && "foo4" != (const char *) ((void *) (&e - &e)) /* { dg-warning "comparison with string literal results in unspecified behaviou?r" "" { target c } } */ + && "foo5" != "foo6"; /* { dg-warning "comparison with string literal results in unspecified behaviou?r" } */ +}