| Message ID | AANLkTiksWH+mhBLxZ8jK9L6AQq5Wkm0uFw4ZC2yCqjsx@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Sep 21, 2010 at 11:29 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > Hello! > > Attached patch defines bool as _Bool when bootstrapping (or compiling > with gcc >= 4.4). > > 2010-09-21 Uros Bizjak <ubizjak@gmail.com> > > * system.h (bool): Define as _Bool for gcc >= 4.4. > (BOOL_BITFILED): Ditto. I think one issue is that libcpp needs to make sure it has the same definition for bool also. Thanks, Andrew Pinski
On 09/21/2010 08:29 PM, Uros Bizjak wrote: > Hello! > > Attached patch defines bool as _Bool when bootstrapping (or compiling > with gcc>= 4.4). > > 2010-09-21 Uros Bizjak<ubizjak@gmail.com> > > * system.h (bool): Define as _Bool for gcc>= 4.4. > (BOOL_BITFILED): Ditto. > > Attached patch was bootstrapped and regression tested on > x86_64-pc-linux-gnu {,-m32} and alphaev68-pc-linux-gnu without > problems. The version of bootstrap compiler is set to 4.4 since this > is the version of the bootstrap compiler I have, and I'm pretty > confident it won't make problems on the targets I have tested. > > RFC patch for now, but I think it is ready for inclusion into mainline SVN. I don't like this, because "GCC bool" (char) and _Bool have different semantics. We risk bitrotting the compatibility path. It can be done ultimately, but as a prerequisite, we should have warnings in -Wextra for all of boolvar++; ++boolvar; boolvar--; --boolvar; boolvar = nonbool; boolvar & nonbool; boolvar &= nonbool; boolvar | nonbool; boolvar |= nonbool; boolvar ^ nonbool; boolvar ^= nonbool; Paolo
On Wed, Sep 22, 2010 at 9:50 AM, Paolo Bonzini <bonzini@gnu.org> wrote: > On 09/21/2010 08:29 PM, Uros Bizjak wrote: >> Attached patch defines bool as _Bool when bootstrapping (or compiling >> with gcc>= 4.4). >> >> 2010-09-21 Uros Bizjak<ubizjak@gmail.com> >> >> * system.h (bool): Define as _Bool for gcc>= 4.4. >> (BOOL_BITFILED): Ditto. >> >> Attached patch was bootstrapped and regression tested on >> x86_64-pc-linux-gnu {,-m32} and alphaev68-pc-linux-gnu without >> problems. The version of bootstrap compiler is set to 4.4 since this >> is the version of the bootstrap compiler I have, and I'm pretty >> confident it won't make problems on the targets I have tested. >> >> RFC patch for now, but I think it is ready for inclusion into mainline >> SVN. > > I don't like this, because "GCC bool" (char) and _Bool have different > semantics. We risk bitrotting the compatibility path. > > It can be done ultimately, but as a prerequisite, we should have warnings in > -Wextra for all of > > boolvar++; ++boolvar; > boolvar--; --boolvar; > boolvar = nonbool; > boolvar & nonbool; boolvar &= nonbool; > boolvar | nonbool; boolvar |= nonbool; > boolvar ^ nonbool; boolvar ^= nonbool; Fair enough. I have CCed Manuel, perhaps he is interested in this warning. Uros.
On 24 September 2010 10:31, Uros Bizjak <ubizjak@gmail.com> wrote: > On Wed, Sep 22, 2010 at 9:50 AM, Paolo Bonzini <bonzini@gnu.org> wrote: >> On 09/21/2010 08:29 PM, Uros Bizjak wrote: > >>> Attached patch defines bool as _Bool when bootstrapping (or compiling >>> with gcc>= 4.4). >>> >>> 2010-09-21 Uros Bizjak<ubizjak@gmail.com> >>> >>> * system.h (bool): Define as _Bool for gcc>= 4.4. >>> (BOOL_BITFILED): Ditto. >>> >>> Attached patch was bootstrapped and regression tested on >>> x86_64-pc-linux-gnu {,-m32} and alphaev68-pc-linux-gnu without >>> problems. The version of bootstrap compiler is set to 4.4 since this >>> is the version of the bootstrap compiler I have, and I'm pretty >>> confident it won't make problems on the targets I have tested. >>> >>> RFC patch for now, but I think it is ready for inclusion into mainline >>> SVN. >> >> I don't like this, because "GCC bool" (char) and _Bool have different >> semantics. We risk bitrotting the compatibility path. >> >> It can be done ultimately, but as a prerequisite, we should have warnings in >> -Wextra for all of >> >> boolvar++; ++boolvar; >> boolvar--; --boolvar; >> boolvar = nonbool; >> boolvar & nonbool; boolvar &= nonbool; >> boolvar | nonbool; boolvar |= nonbool; >> boolvar ^ nonbool; boolvar ^= nonbool; > > Fair enough. I have CCed Manuel, perhaps he is interested in this warning. I am not sure it fits in -Wconversion. -Wbool-arith perhaps? For several reasons, I am not working on GCC any more, sorry. But please, open a PR, perhaps things will change in the future. Cheers, Manuel.
On Fri, 24 Sep 2010, Manuel López-Ibáñez wrote: > >> It can be done ultimately, but as a prerequisite, we should have warnings in > >> -Wextra for all of > >> > >> boolvar++; ++boolvar; > >> boolvar--; --boolvar; > >> boolvar = nonbool; > >> boolvar & nonbool; boolvar &= nonbool; > >> boolvar | nonbool; boolvar |= nonbool; > >> boolvar ^ nonbool; boolvar ^= nonbool; > > > > Fair enough. I have CCed Manuel, perhaps he is interested in this warning. > > I am not sure it fits in -Wconversion. -Wbool-arith perhaps? It sounds like a warning for a -Wc90-c99-compat or similar option (possibly in a more specific option such as -Wbool-arith) - pure C99 code has little use for such a warning, it's about code that might be compiled either with C99 _Bool, or with C90 unsigned char, and so needs to avoid cases where they are incompatible. Such an option, similar in spirit to -Wtraditional, could also allow you to get warnings in C99 mode for things currently diagnosed with -std=c90 -pedantic.
On Fri, Sep 24, 2010 at 5:03 PM, Joseph S. Myers <joseph@codesourcery.com> wrote: > On Fri, 24 Sep 2010, Manuel López-Ibáñez wrote: > >> >> It can be done ultimately, but as a prerequisite, we should have warnings in >> >> -Wextra for all of >> >> >> >> boolvar++; ++boolvar; >> >> boolvar--; --boolvar; >> >> boolvar = nonbool; >> >> boolvar & nonbool; boolvar &= nonbool; >> >> boolvar | nonbool; boolvar |= nonbool; >> >> boolvar ^ nonbool; boolvar ^= nonbool; >> > >> > Fair enough. I have CCed Manuel, perhaps he is interested in this warning. >> >> I am not sure it fits in -Wconversion. -Wbool-arith perhaps? > > It sounds like a warning for a -Wc90-c99-compat or similar option > (possibly in a more specific option such as -Wbool-arith) - pure C99 code > has little use for such a warning, it's about code that might be compiled > either with C99 _Bool, or with C90 unsigned char, and so needs to avoid > cases where they are incompatible. Such an option, similar in spirit to > -Wtraditional, could also allow you to get warnings in C99 mode for things > currently diagnosed with -std=c90 -pedantic. I have opened PR c/45780 [1] as proposed by Manuel and took the liberty to quote this entire message there. [1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45780 Thanks, Uros.
Index: system.h =================================================================== --- system.h (revision 164480) +++ system.h (working copy) @@ -621,29 +621,29 @@ This must be after all inclusion of system headers, as some of them will mess us up. */ -#undef TRUE -#undef FALSE +#ifndef __cplusplus +# undef bool -#ifdef __cplusplus - /* Obsolete. */ -# define TRUE true -# define FALSE false -#else /* !__cplusplus */ -# undef bool +# if GCC_VERSION >= 4004 +# define bool _Bool +# define BOOL_BITFIELD _Bool +# else +# define bool unsigned char +/* Some compilers do not allow the use of unsigned char in bitfields. */ +# define BOOL_BITFIELD unsigned int +# endif + # undef true # undef false - -# define bool unsigned char # define true 1 # define false 0 - - /* Obsolete. */ -# define TRUE true -# define FALSE false #endif /* !__cplusplus */ -/* Some compilers do not allow the use of unsigned char in bitfields. */ -#define BOOL_BITFIELD unsigned int +/* Obsolete. */ +#undef TRUE +#undef FALSE +#define TRUE true +#define FALSE false /* As the last action in this file, we poison the identifiers that shouldn't be used. Note, luckily gcc-3.0's token-based integrated
Hello! Attached patch defines bool as _Bool when bootstrapping (or compiling with gcc >= 4.4). 2010-09-21 Uros Bizjak <ubizjak@gmail.com> * system.h (bool): Define as _Bool for gcc >= 4.4. (BOOL_BITFILED): Ditto. Attached patch was bootstrapped and regression tested on x86_64-pc-linux-gnu {,-m32} and alphaev68-pc-linux-gnu without problems. The version of bootstrap compiler is set to 4.4 since this is the version of the bootstrap compiler I have, and I'm pretty confident it won't make problems on the targets I have tested. RFC patch for now, but I think it is ready for inclusion into mainline SVN. Uros.