Re: [STYLE] use true/false in C++, not TRUE/FALSE

[Mats Kindahl]

On 09/08/2010 02:47 PM, Tor Didriksen wrote:
> On 2010-09-08 14:22, Mats Kindahl wrote:
>>>> One way to avoid the problems that Mark points out is to redefine
>>>> >>  my_bool to be "int". This would work for all places where
>>>> my_bool is
>>>> >>  used and IIRC, there are no places in ABIs where my_bool is
>>>> used. If
>>>> >>  there are, it is easy to replace them with "unsigned char" to
>>>> ensure
>>>> >>  that the ABI does not change.
>>>> >>         
>>> >  naah, I think we want sizeof(my_bool) == 1
>>>      
>> Why?
>>
>> /Matz
>>
>>    
> - save space

Hardly relevant in this case.

    * If my_bool is used inside a structure, alignment on modern
      architectures will force it to align on a 4-byte boundary anyway
    * If several my_bool are used in a sequence, memory could be saved,
      but if space is so important, it is better to use a bit field
    * As return value they will not save space, and even generate larger
      code. Not much, but it defuses the argument of saving space.
    * As auto variables, they are likely to align at 4-byte boundaries
      anyway, if they end up on the stack, and be stored in a register
      if not.

> - compatibility with bool in C++ (and C99)

I cannot see at all why there is any advantage in allowing
sizeof(my_bool) == sizeof(bool). The type bool is used in C++ and
my_bool is used in C:  there will not be a connection between them that
make any difference whatsoever.

>
> I encourage you to try 'typedef int my_bool;'
> my guess is you will get quite a few errors to fix, both compile time
> and run-time

Out of curiosity, I did.

    * I found some violations of the ABI in that InnoDB uses "char" in
      place of "my_bool", and same for the semi-sync plugin.
    * I had to rewrite the Sys_var_mybool to Sys_var_bool instead (it's
      C++ code and was using "my_bool" inside the class but "bool" for
      the underlying variables, which cause the casting problems that
      Mark was talking about).

That's all. I didn't run all tests, just the "main" test suite.

Patch attached for the curios. There is one change to the ABI, but that
is only because I just wanted the compile to pass. The ABI can be
maintained, but right now it's too late for me to bother about fixing
that for the demo patch.

Just my few cents,
Mats Kindahl

>
> -- didrik
>
>


Attachment: [text/x-diff] patch-1.diff