On 20/11/2012 03:16, Marvin Humphrey wrote:
I've checked out the branch and everything looks solid. Extra kudos for that
big bool_t-to-bool commit, which I imagine was tedious to prepare!
With a little help from sed, it was actually quite easy.
From the perspective of Charmonizer as a component in isolation (which has the
potential to spin off from Lucy one day) I wonder whether it wouldn't be
advantageous to leave intrusions into the standard namespace up to the
consumer:
#ifndef CHY_HAS_STDINT_H
#define int32_t chy_i32_t
...
#endif
We could keep the old Integers module, and rename the new version to
C99_Integers or so. But I think Charmonizer users should either use
(chy_)i32_t or int32_t. I can see no value in using both names at the
same time.
On top of that, I tested switching from 'chy_bool_t' to 'bool' from
stdbool.h (or from the C++ compiler when compiling as C++). This is a bit
problematic since 'bool' is often defined by other headers.
The fact that we require either C99 or C++ for Lucy and Clownfish ought to
make things easier because we are guaranteed to have either stdbool.h or the
C++ `bool` type available. Our `bool` is guaranteed to come from the system
one way or another.
Since Charmonizer only requires C89, it's not possible for Charmonizer to
resolve this problem in the general case. If you want to fake up a bool type
for C89, you need to `typedef bool` to something -- `char` and `int` are both
popular -- and there's significant potential for conflict with someone who
has made a similar but incompatible hack.
Maybe Charmonizer should also provide two separate modules for booleans.
My understanding is that the stdbool.h `bool` and the C++ `bool` are not
technically compatible, but I hope that won't matter in practice. Hopefully
no system vendor will define them to be different sizes.
stdbool.h defines 'bool' as '_Bool', which is a real boolean type
defined in C99. It can only have the two distinct values true or false
like 'bool' in C++. I can't imagine that a single compiler suite would
use different sizes for C++ 'bool' and C99 '_Bool', so both types should
be compatible.
Unfortunately, also the Perl headers define their own version of 'bool'
under certain conditions. As a work-around, I had to add -DHAS_BOOL to the
build options to make the Linux and Windows build work. This looks a bit
sketchy and might cause problems with other Perl versions.
The workaround doesn't seem any sketchier to me than the fact that Perl
defines its own `bool` in the first place.
After your changes, our `bool` is either a macro defined in stdbool.h or a C++
builtin. It's not a careless hack.
Right, but compiling with HAS_BOOL might break with certain Perl
versions. After all, we use a different definition of 'bool' than what
Perl was compiled with.
* If the Perl headers are conflicting with stdbool.h, that's messed up --
maybe it happens because Perl wants to use the keyword `bool`, but also
wants to specify `-std=c89`?
I think proper support for stdbool.h only came with Perl 5.16. 5.14 and
below are conflicting with stdbool.h. See here:
https://metacpan.org/source/JESSE/perl-5.14.0/handy.h#L73
https://metacpan.org/source/RJBS/perl-5.16.2/handy.h#L73
Only the stock Perl on MacOS seems to be an exception where earlier
versions are patched to use stdbool.h.
* If the Perl headers are conflicting with C++, that's more understandable.
But if the Perl devs have seen fit to provide a flag which enables C++
compatibility, let's take advantage of it.
I'm not sure whether HAS_BOOL is really meant to be used for C++
compatibility. I'd really like to see whether the bool changes work with
oldest Perl we officially support.
Nick
