Re: GinPageIs* don't actually return a boolean

On Fri, Feb 12, 2016 at 8:48 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> On 2016-02-12 07:59:06 -0500, Robert Haas wrote:
>> OK, that seems reasonable from here. What I'm still fuzzy about is
>> why including stdbool.h causes a failure. Is it because it defines a
>> type called "bool" that clashes with ours? That seems like the
>> obvious explanation, but then how does that not cause the compiler to
>> just straight-up error out?
>
> No, that's not the problem. Our own definition is actually
> conditional. The problem is that the standard's booleans behave
> differently. They only ever contain 0 or 1, even if you assign something
> outside of that range - essentially they store !!(right-hand-side). If
> you know compare a boolean with the values returned by one of these
> macros you'll get into problems.
>
> E.g. if you include stdbool.h:
> Buffer
> ginStepRight(Buffer buffer, Relation index, int lockmode)
> {
> bool isLeaf = GinPageIsLeaf(page);
> bool isData = GinPageIsData(page);
> ...
> if (isLeaf != GinPageIsLeaf(page) || isData != GinPageIsData(page))
> elog(ERROR, "right sibling of GIN page is of different type");
>
> doesn't work correctly because isLeaf/isData contain only 0/1 (due to
> the standard's bool behaviour), but the values they're compared to
> returns some bit set. Due to integer promotion rules isLeaf is promoted
> to an integer and compared... And thus the tests fail.

Ah-ha. OK, now I get it. So then I agree we should back-patch this
at least as far as 9.3 where MSVC 2013 became a supported platform,
per Michael's remarks, and maybe all the way. Do you want to do that?
If not, I'll do it.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company