On Fri, Oct 11, 2013 at 10:11:21AM -0400, Jason Merrill wrote: > >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.
We then violate the coding standard in vec.h: /* Type to provide NULL values for vec<T, A, L>. This is used to provide nil initializers for vec instances. Since vec must be a POD, we cannot have proper ctor/dtor for it. To initialize a vec instance, you can assign it the value vNULL. */ struct vnull { template <typename T, typename A, typename L> operator vec<T, A, L> () { return vec<T, A, L>(); } }; extern vnull vNULL; but perhaps we can keep that as an exception and don't allow further exceptions... > But it would be better to add operator!=(omp_clause_mask). Thanks, that works too, thus here is what I've committed. 2013-10-11 Jakub Jelinek <ja...@redhat.com> * c-common.h (omp_clause_mask::operator !=): New method. * c-omp.c (c_omp_split_clauses): Use if ((mask & something) != 0) instead of if (mask & something) tests everywhere. --- gcc/c-family/c-common.h.jj 2013-10-11 16:14:35.157176208 +0200 +++ gcc/c-family/c-common.h 2013-10-11 16:15:30.031891445 +0200 @@ -1052,6 +1052,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 bool operator != (omp_clause_mask) const; unsigned HOST_WIDE_INT low, high; }; @@ -1156,6 +1157,12 @@ omp_clause_mask::operator == (omp_clause return low == b.low && high == b.high; } +inline bool +omp_clause_mask::operator != (omp_clause_mask b) const +{ + return low != b.low || high != b.high; +} + # define OMP_CLAUSE_MASK_1 omp_clause_mask (1) #endif --- gcc/c-family/c-omp.c.jj 2013-10-11 16:14:35.153176228 +0200 +++ gcc/c-family/c-omp.c 2013-10-11 16:14:48.164106509 +0200 @@ -631,7 +631,7 @@ c_omp_split_clauses (location_t loc, enu cclauses[i] = NULL; /* Add implicit nowait clause on #pragma omp parallel {for,for simd,sections}. */ - if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS)) + if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS)) != 0) switch (code) { case OMP_FOR: @@ -691,10 +691,10 @@ c_omp_split_clauses (location_t loc, enu OMP_CLAUSE_CHAIN (c) = cclauses[C_OMP_CLAUSE_SPLIT_SIMD]; cclauses[C_OMP_CLAUSE_SPLIT_SIMD] = c; } - if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SCHEDULE)) + if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SCHEDULE)) != 0) { - if (mask & (OMP_CLAUSE_MASK_1 - << PRAGMA_OMP_CLAUSE_DIST_SCHEDULE)) + if ((mask & (OMP_CLAUSE_MASK_1 + << PRAGMA_OMP_CLAUSE_DIST_SCHEDULE)) != 0) { c = build_omp_clause (OMP_CLAUSE_LOCATION (clauses), OMP_CLAUSE_COLLAPSE); @@ -729,19 +729,20 @@ c_omp_split_clauses (location_t loc, enu target and simd. Put it on the outermost of those and duplicate on parallel. */ case OMP_CLAUSE_FIRSTPRIVATE: - if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS)) + if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS)) + != 0) { - if (mask & ((OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS) - | (OMP_CLAUSE_MASK_1 - << PRAGMA_OMP_CLAUSE_DIST_SCHEDULE))) + if ((mask & ((OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS) + | (OMP_CLAUSE_MASK_1 + << PRAGMA_OMP_CLAUSE_DIST_SCHEDULE))) != 0) { c = build_omp_clause (OMP_CLAUSE_LOCATION (clauses), OMP_CLAUSE_FIRSTPRIVATE); OMP_CLAUSE_DECL (c) = OMP_CLAUSE_DECL (clauses); OMP_CLAUSE_CHAIN (c) = cclauses[C_OMP_CLAUSE_SPLIT_PARALLEL]; cclauses[C_OMP_CLAUSE_SPLIT_PARALLEL] = c; - if (mask & (OMP_CLAUSE_MASK_1 - << PRAGMA_OMP_CLAUSE_NUM_TEAMS)) + if ((mask & (OMP_CLAUSE_MASK_1 + << PRAGMA_OMP_CLAUSE_NUM_TEAMS)) != 0) s = C_OMP_CLAUSE_SPLIT_TEAMS; else s = C_OMP_CLAUSE_SPLIT_DISTRIBUTE; @@ -751,14 +752,15 @@ c_omp_split_clauses (location_t loc, enu #pragma omp parallel{, for{, simd}, sections}. */ s = C_OMP_CLAUSE_SPLIT_PARALLEL; } - else if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS)) + else if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS)) + != 0) { /* This must be #pragma omp {,target }teams distribute. */ gcc_assert (code == OMP_DISTRIBUTE); s = C_OMP_CLAUSE_SPLIT_TEAMS; } - else if (mask & (OMP_CLAUSE_MASK_1 - << PRAGMA_OMP_CLAUSE_DIST_SCHEDULE)) + else if ((mask & (OMP_CLAUSE_MASK_1 + << PRAGMA_OMP_CLAUSE_DIST_SCHEDULE)) != 0) { /* This must be #pragma omp distribute simd. */ gcc_assert (code == OMP_SIMD); @@ -777,19 +779,21 @@ c_omp_split_clauses (location_t loc, enu case OMP_CLAUSE_LASTPRIVATE: if (code == OMP_FOR || code == OMP_SECTIONS) { - if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS)) + if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS)) + != 0) s = C_OMP_CLAUSE_SPLIT_PARALLEL; else s = C_OMP_CLAUSE_SPLIT_FOR; break; } gcc_assert (code == OMP_SIMD); - if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SCHEDULE)) + if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SCHEDULE)) != 0) { c = build_omp_clause (OMP_CLAUSE_LOCATION (clauses), OMP_CLAUSE_LASTPRIVATE); OMP_CLAUSE_DECL (c) = OMP_CLAUSE_DECL (clauses); - if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS)) + if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS)) + != 0) s = C_OMP_CLAUSE_SPLIT_PARALLEL; else s = C_OMP_CLAUSE_SPLIT_FOR; @@ -806,7 +810,8 @@ c_omp_split_clauses (location_t loc, enu s = C_OMP_CLAUSE_SPLIT_TEAMS; break; } - if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS)) + if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS)) + != 0) { c = build_omp_clause (OMP_CLAUSE_LOCATION (clauses), OMP_CLAUSE_CODE (clauses)); @@ -837,9 +842,10 @@ c_omp_split_clauses (location_t loc, enu OMP_CLAUSE_CHAIN (c) = cclauses[C_OMP_CLAUSE_SPLIT_SIMD]; cclauses[C_OMP_CLAUSE_SPLIT_SIMD] = c; } - if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SCHEDULE)) + if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SCHEDULE)) != 0) { - if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS)) + if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS)) + != 0) { c = build_omp_clause (OMP_CLAUSE_LOCATION (clauses), OMP_CLAUSE_REDUCTION); @@ -852,8 +858,8 @@ c_omp_split_clauses (location_t loc, enu cclauses[C_OMP_CLAUSE_SPLIT_PARALLEL] = c; s = C_OMP_CLAUSE_SPLIT_TEAMS; } - else if (mask & (OMP_CLAUSE_MASK_1 - << PRAGMA_OMP_CLAUSE_NUM_THREADS)) + else if ((mask & (OMP_CLAUSE_MASK_1 + << PRAGMA_OMP_CLAUSE_NUM_THREADS)) != 0) s = C_OMP_CLAUSE_SPLIT_PARALLEL; else s = C_OMP_CLAUSE_SPLIT_FOR; @@ -865,7 +871,8 @@ c_omp_split_clauses (location_t loc, enu break; case OMP_CLAUSE_IF: /* FIXME: This is currently being discussed. */ - if (mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS)) + if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS)) + != 0) s = C_OMP_CLAUSE_SPLIT_PARALLEL; else s = C_OMP_CLAUSE_SPLIT_TARGET; Jakub