| Message ID | AM4PR0701MB2162A33F0772770B9ADDA601E4D60@AM4PR0701MB2162.eurprd07.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
On 10/23/2016 05:31 AM, Bernd Edlinger wrote: > On 10/22/16 08:52, Bernd Edlinger wrote: >> > On 10/22/16 04:17, Martin Sebor wrote: >>> >> On 10/21/2016 04:37 PM, Joseph Myers wrote: >>>> >>> The quoting in the diagnostic should be %<&&%>, not '&&'. >>> >> >>> >> Presumably same for '*' (i.e., %<*%>). >>> >> >>> >> But I would actually suggest a somewhat more formal phrasing than >>> >> "better use xxx here" such as "suggest %<&&%> instead" or something >>> >> akin to what's already in place elsewhere in gcc.pot. >>> >> >> > >> > Aehm, yes. That would be better then: >> > >> > >> > Index: c-common.c >> > =================================================================== >> > --- c-common.c (revision 241400) >> > +++ c-common.c (working copy) >> > @@ -3327,6 +3327,11 @@ >> > return c_common_truthvalue_conversion (location, >> > TREE_OPERAND (expr, 0)); >> > >> > + case MULT_EXPR: >> > + warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context, >> > + "%<*%> in boolean context, suggest %<&&%> instead"); >> > + break; >> > + >> > case LSHIFT_EXPR: >> > /* We will only warn on signed shifts here, because the majority of >> > false positive warnings happen in code where unsigned arithmetic >> > >> > >> > I assume then I should adjust the warning a few lines below as well: >> > >> > warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context, >> > "<< in boolean context, did you mean '<' ?"); >> > >> > > Attached is the updated patch with those quotes fixed. > > I have now put the << and < in correct quotes, but left the ?: in > the next two warnings unquoted: > > "?: using integer constants in boolean context, " > "the expression will always evaluate to %<true%>" > > I copied that style from the warning about omitted middle operand of > conditional expressions: > > "the omitted middle operand in ?: will always be %<true%>, suggest > explicit " > "middle operand" > > I think that could be explained because ?: is not really a keyword > like <<, and is just a shorter phrase than "conditional expression". > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? > > > Thanks > Bernd. > > > patch-bool-context5.diff > > > c-family: > 2016-10-23 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * c-common.c (c_common_truthvalue_conversion): Warn for > multiplications in boolean context. Fix the quoting of '<<' and '<' > in the shift warning. > > gcc: > 2016-10-23 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * doc/invoke.text (Wint-in-bool-context): Update documentation. > * value-prof.c (stringop_block_profile): Fix a warning. > > testsuite: > 2016-10-23 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * c-c++-common/Wint-in-bool-context-3.c: New test. OK. Jeff
c-family: 2016-10-23 Bernd Edlinger <bernd.edlinger@hotmail.de> * c-common.c (c_common_truthvalue_conversion): Warn for multiplications in boolean context. Fix the quoting of '<<' and '<' in the shift warning. gcc: 2016-10-23 Bernd Edlinger <bernd.edlinger@hotmail.de> * doc/invoke.text (Wint-in-bool-context): Update documentation. * value-prof.c (stringop_block_profile): Fix a warning. testsuite: 2016-10-23 Bernd Edlinger <bernd.edlinger@hotmail.de> * c-c++-common/Wint-in-bool-context-3.c: New test. Index: gcc/c-family/c-common.c =================================================================== --- gcc/c-family/c-common.c (revision 241437) +++ gcc/c-family/c-common.c (working copy) @@ -3327,6 +3327,11 @@ c_common_truthvalue_conversion (location_t locatio return c_common_truthvalue_conversion (location, TREE_OPERAND (expr, 0)); + case MULT_EXPR: + warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context, + "%<*%> in boolean context, suggest %<&&%> instead"); + break; + case LSHIFT_EXPR: /* We will only warn on signed shifts here, because the majority of false positive warnings happen in code where unsigned arithmetic @@ -3336,7 +3341,7 @@ c_common_truthvalue_conversion (location_t locatio if (TREE_CODE (TREE_TYPE (expr)) == INTEGER_TYPE && !TYPE_UNSIGNED (TREE_TYPE (expr))) warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context, - "<< in boolean context, did you mean '<' ?"); + "%<<<%> in boolean context, did you mean %<<%> ?"); break; case COND_EXPR: Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 241437) +++ gcc/doc/invoke.texi (working copy) @@ -6169,8 +6169,9 @@ of the C++ standard. @opindex Wno-int-in-bool-context Warn for suspicious use of integer values where boolean values are expected, such as conditional expressions (?:) using non-boolean integer constants in -boolean context, like @code{if (a <= b ? 2 : 3)}. Or left shifting in -boolean context, like @code{for (a = 0; 1 << a; a++);}. +boolean context, like @code{if (a <= b ? 2 : 3)}. Or left shifting of signed +integers in boolean context, like @code{for (a = 0; 1 << a; a++);}. Likewise +for all kinds of multiplications regardless of the data type. This warning is enabled by @option{-Wall}. @item -Wno-int-to-pointer-cast Index: gcc/value-prof.c =================================================================== --- gcc/value-prof.c (revision 241437) +++ gcc/value-prof.c (working copy) @@ -1878,12 +1878,12 @@ stringop_block_profile (gimple *stmt, unsigned int else { gcov_type count; - int alignment; + unsigned int alignment; count = histogram->hvalue.counters[0]; alignment = 1; while (!(count & alignment) - && (alignment * 2 * BITS_PER_UNIT)) + && (alignment <= UINT_MAX / 2 / BITS_PER_UNIT)) alignment <<= 1; *expected_align = alignment * BITS_PER_UNIT; gimple_remove_histogram_value (cfun, stmt, histogram); Index: gcc/testsuite/c-c++-common/Wint-in-bool-context-3.c =================================================================== --- gcc/testsuite/c-c++-common/Wint-in-bool-context-3.c (revision 0) +++ gcc/testsuite/c-c++-common/Wint-in-bool-context-3.c (working copy) @@ -0,0 +1,15 @@ +/* { dg-options "-Wint-in-bool-context" } */ +/* { dg-do compile } */ + +#define BITS_PER_UNIT 8 + +int foo (int count) +{ + int alignment; + + alignment = 1; + while (!(count & alignment) + && (alignment * 2 * BITS_PER_UNIT)) /* { dg-warning "boolean context" } */ + alignment <<= 1; + return alignment * BITS_PER_UNIT; +}