| Message ID | AM4PR0701MB21623C26FFFF12884B06EA0FE4C60@AM4PR0701MB2162.eurprd07.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
OK. On Fri, Oct 7, 2016 at 11:18 AM, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > Hi! > > This extends -Wint-in-bool-context to uses of enum values in boolean > context, and fixes one place where accidentally an enum value was > passed to a bool parameter. > > I excluded enum values 0 and 1 because that is used in > gimple-ssa-strength-reduction.c, where we have enums > which are passed in bool function arguments: > > enum stride_status > { > UNKNOWN_STRIDE = 0, > KNOWN_STRIDE = 1 > }; > > enum phi_adjust_status > { > NOT_PHI_ADJUST = 0, > PHI_ADJUST = 1 > }; > > enum count_phis_status > { > DONT_COUNT_PHIS = 0, > COUNT_PHIS = 1 > }; > > I would'nt use an enum in that way, but I think it is > at least not completely wrong to do it like that... > > > Unfortunately C is less strict with enum values, and from > and enum we only see an integer value without an enum type > in C. > > Therefore this warning does not work in C, only in C++. > Also integer constants do not have a source location, so > the displayed location is usually a bit vague. > But I think it is still better than no warning at all. > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? > > > Thanks > Bernd.
On 2016.10.07 at 15:18 +0000, Bernd Edlinger wrote: > Hi! > > This extends -Wint-in-bool-context to uses of enum values in boolean > context, and fixes one place where accidentally an enum value was > passed to a bool parameter. > > I excluded enum values 0 and 1 because that is used in > gimple-ssa-strength-reduction.c, where we have enums > which are passed in bool function arguments: > > enum stride_status > { > UNKNOWN_STRIDE = 0, > KNOWN_STRIDE = 1 > }; > > enum phi_adjust_status > { > NOT_PHI_ADJUST = 0, > PHI_ADJUST = 1 > }; > > enum count_phis_status > { > DONT_COUNT_PHIS = 0, > COUNT_PHIS = 1 > }; > > I would'nt use an enum in that way, but I think it is > at least not completely wrong to do it like that... > > > Unfortunately C is less strict with enum values, and from > and enum we only see an integer value without an enum type > in C. > > Therefore this warning does not work in C, only in C++. > Also integer constants do not have a source location, so > the displayed location is usually a bit vague. > But I think it is still better than no warning at all. > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? I came across an borderline example in Chromium today: 62 enum FlushBehavior { 63 // More bytes are coming, don't flush the codec. 64 DoNotFlush = 0, 65 66 // A fetch has hit EOF. Some codecs handle fetches differently, for compat 67 // reasons. 68 FetchEOF, 69 70 // Do a full flush of the codec. 71 DataEOF 72 }; 73 74 static_assert(!DoNotFlush, "DoNotFlush should be falsy"); 75 static_assert(FetchEOF, "FetchEOF should be truthy"); 76 static_assert(DataEOF, "DataEOF should be truthy"); ../../third_party/WebKit/Source/wtf/text/TextCodec.h:76:51: warning: enum constant in boolean context [-Wint-in-bool-context] static_assert(DataEOF, "DataEOF should be truthy"); ^ -- Markus
c-family: 2016-10-07 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/77700 * c-common.c (c_common_truthvalue_conversion): Warn also for suspicious enum values in boolean context. cp: 2016-10-07 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/77700 * parser.c (cp_parser_base_specifier): Fix a warning. testsuite: 2016-10-07 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/77700 * c-c++-common/Wint-in-bool-context.c: Update test. Index: gcc/c-family/c-common.c =================================================================== --- gcc/c-family/c-common.c (revision 240838) +++ gcc/c-family/c-common.c (working copy) @@ -4588,6 +4588,11 @@ c_common_truthvalue_conversion (location_t locatio return expr; case INTEGER_CST: + if (TREE_CODE (TREE_TYPE (expr)) == ENUMERAL_TYPE + && !integer_zerop (expr) + && !integer_onep (expr)) + warning_at (location, OPT_Wint_in_bool_context, + "enum constant in boolean context"); return integer_zerop (expr) ? truthvalue_false_node : truthvalue_true_node; Index: gcc/cp/parser.c =================================================================== --- gcc/cp/parser.c (revision 240838) +++ gcc/cp/parser.c (working copy) @@ -23334,7 +23334,7 @@ cp_parser_base_specifier (cp_parser* parser) cp_parser_nested_name_specifier_opt (parser, /*typename_keyword_p=*/true, /*check_dependency_p=*/true, - typename_type, + /*type_p=*/true, /*is_declaration=*/true); /* If the base class is given by a qualified name, assume that names we see are type names or templates, as appropriate. */ Index: gcc/testsuite/c-c++-common/Wint-in-bool-context.c =================================================================== --- gcc/testsuite/c-c++-common/Wint-in-bool-context.c (revision 240838) +++ gcc/testsuite/c-c++-common/Wint-in-bool-context.c (working copy) @@ -2,6 +2,8 @@ /* { dg-options "-Wint-in-bool-context" } */ /* { dg-do compile } */ +enum truth { yes, no, maybe }; + int foo (int a, int b) { if (a > 0 && a <= (b == 1) ? 1 : 2) /* { dg-warning "boolean context" } */ @@ -27,5 +29,11 @@ int foo (int a, int b) for (a = 0; 1 << a; a++); /* { dg-warning "boolean context" } */ + if (yes || no || maybe) /* { dg-warning "boolean context" "" { target c++ } } */ + return 8; + + if (yes || no) /* { dg-bogus "boolean context" } */ + return 9; + return 0; }
Hi! This extends -Wint-in-bool-context to uses of enum values in boolean context, and fixes one place where accidentally an enum value was passed to a bool parameter. I excluded enum values 0 and 1 because that is used in gimple-ssa-strength-reduction.c, where we have enums which are passed in bool function arguments: enum stride_status { UNKNOWN_STRIDE = 0, KNOWN_STRIDE = 1 }; enum phi_adjust_status { NOT_PHI_ADJUST = 0, PHI_ADJUST = 1 }; enum count_phis_status { DONT_COUNT_PHIS = 0, COUNT_PHIS = 1 }; I would'nt use an enum in that way, but I think it is at least not completely wrong to do it like that... Unfortunately C is less strict with enum values, and from and enum we only see an integer value without an enum type in C. Therefore this warning does not work in C, only in C++. Also integer constants do not have a source location, so the displayed location is usually a bit vague. But I think it is still better than no warning at all. Bootstrapped and reg-tested on x86_64-pc-linux-gnu. Is it OK for trunk? Thanks Bernd.