Patchwork [RFC] : Define "bool" as _Bool when bootstrapping with gcc >= 4.4

login
register
mail settings
Submitter Uros Bizjak
Date Sept. 21, 2010, 6:29 p.m.
Message ID <AANLkTiksWH+mhBLxZ8jK9L6AQq5Wkm0uFw4ZC2yCqjsx@mail.gmail.com>
Download mbox | patch
Permalink /patch/65364/
State New
Headers show

Comments

Uros Bizjak - Sept. 21, 2010, 6:29 p.m.
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.
Andrew Pinski - Sept. 21, 2010, 6:33 p.m.
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
Paolo Bonzini - Sept. 22, 2010, 7:50 a.m.
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
Uros Bizjak - Sept. 24, 2010, 8:31 a.m.
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.
Manuel López-Ibáñez - Sept. 24, 2010, 10:01 a.m.
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.
Joseph S. Myers - Sept. 24, 2010, 3:03 p.m.
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.
Uros Bizjak - Sept. 24, 2010, 6:43 p.m.
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.

Patch

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