| Message ID | 20131011135601.GT30970@tucnak.zalov.cz |
|---|---|
| State | New |
| Headers | show |
On 10/11/2013 09:56 AM, Jakub Jelinek wrote: > With the operator bool (), there is ambiguity in the > if (((mask >> something) & 1) == 0) > tests (so had to use OMP_CLAUSE_MASK_{1,0} instead of {1,0}) This is an example of why operator bool is a bad idea in general. If we were using C++11, we could make the operator 'explicit' so that it's only used in actual boolean context, but without 'explicit' it gets hit too often. To deal with this issue, C++98 programmers tend to use a conversion to pointer-to-member, which is also testable as a boolean, instead. http://www.artima.com/cppsource/safebool.html > another > possibility is not to add operator bool () overload that introduces that > ambiguity, but then if (mask & something) needs to be replaced with > if ((mask & something) != 0) and operator != (int) added. > I guess I slightly prefer the first patch since it is smaller. Since the coding standards say "Conversion operators should be avoided" (because they can't be explicit), I think this is the way to go. But it would be better to add operator!=(omp_clause_mask). Jason
--- gcc/c-family/c-common.h.jj 2013-10-11 11:23:59.000000000 +0200 +++ gcc/c-family/c-common.h 2013-10-11 15:40:32.581791018 +0200 @@ -1036,6 +1036,7 @@ extern void pp_dir_change (cpp_reader *, /* In c-omp.c */ #if HOST_BITS_PER_WIDE_INT >= 64 typedef unsigned HOST_WIDE_INT omp_clause_mask; +# define OMP_CLAUSE_MASK_0 ((omp_clause_mask) 0) # define OMP_CLAUSE_MASK_1 ((omp_clause_mask) 1) #else struct omp_clause_mask @@ -1052,6 +1053,7 @@ struct omp_clause_mask inline omp_clause_mask operator >> (int); inline omp_clause_mask operator << (int); inline bool operator == (omp_clause_mask) const; + inline operator bool () const; unsigned HOST_WIDE_INT low, high; }; @@ -1156,6 +1158,13 @@ omp_clause_mask::operator == (omp_clause return low == b.low && high == b.high; } +inline +omp_clause_mask::operator bool () const +{ + return low || high; +} + +# define OMP_CLAUSE_MASK_0 omp_clause_mask (0) # define OMP_CLAUSE_MASK_1 omp_clause_mask (1) #endif --- gcc/c/c-parser.c.jj 2013-10-11 11:23:59.000000000 +0200 +++ gcc/c/c-parser.c 2013-10-11 15:41:35.864464738 +0200 @@ -10644,7 +10644,8 @@ c_parser_omp_all_clauses (c_parser *pars first = false; - if (((mask >> c_kind) & 1) == 0 && !parser->error) + if (((mask >> c_kind) & OMP_CLAUSE_MASK_1) == OMP_CLAUSE_MASK_0 + && !parser->error) { /* Remove the invalid clause(s) from the list to avoid confusing the rest of the compiler. */ --- gcc/cp/parser.c.jj 2013-10-11 11:23:59.000000000 +0200 +++ gcc/cp/parser.c 2013-10-11 15:40:50.939719303 +0200 @@ -27865,7 +27865,7 @@ cp_parser_omp_all_clauses (cp_parser *pa first = false; - if (((mask >> c_kind) & 1) == 0) + if (((mask >> c_kind) & OMP_CLAUSE_MASK_1) == OMP_CLAUSE_MASK_0) { /* Remove the invalid clause(s) from the list to avoid confusing the rest of the compiler. */