297832 - Build error on NetBSD and possibly other OSes due to nptypes.h

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[David H. Gutteridge]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Build Identifier: 

As requested, I've opened this bug as a spin-off from 259325, which was
OpenBSD-centric.  The same problem could happen when compiling under any OS not
tested for in any of the conditionals, among them NetBSD.  (As another example,
I think older releases of everyone's favourite SVR4 flavour -- SCO Unix -- will
also break due to a lack of stdbool.h, but I don't have an environment on-hand
to test with.)  Note that I've actually supplied this patch to NetBSD's pkgsrc
team and it's checked in
(http://www.netbsd.org/cgi-bin/query-pr-single.pl?number=27033), so anyone
building through pkgsrc on any of its supported platforms (various OSes) won't
get a build error (at least with GCC compiling the source as C++), but I'd
prefer the developers here fix the problem upstream, hence this report.

As far as the final conditional branch goes, its comment says it's "for those
that ship a standard C99 stdint.h header file", but it's obviously not that
specific.  The patch I've suggested is only a fix for C++, merely a defence
against unnecessarily trying to include stdbool.h when compiling as C++.  Any
somewhat recent C++ compiler should support bool as an integral datatype.  While
the condition in question is intended in part to allow for older GCC releases,
that still shouldn't be relevant when compiling as C++.  stdbool.h is a C99
header, it's not for C++.  (Incidentally, I have a test environment running
Solaris 7 with GCC 2.8.1 [not by choice], and it does accept a typedef for bool
in C, I'm not sure why 2.91 wouldn't...  It also provides bool in C++.)  If
there are systems that "ship with a stdbool.h that doesn't compile", it'd be
better to test for them specifically.

For Un*x builds, it would be neater to use GNU autoconf to determine what is
available and what is not, rather than coding conditional statements that aren't
always going to provide the best choices.  For instance, there's a block which
handles various SVR4 flavours of Unix, including HP-UX.  Recent HP-UX releases
do ship with stdbool.h available, so it would make sense to make use of it. 
Ideally you'd test for the existence of stdbool.h and anything else desired with
autoconf.  (Though I realize autoconf is only optional right now, maybe this
isn't a compelling enough reason to make it mandatory.)

Reproducible: Always

Comment 1

[David H. Gutteridge]

Attachment: proposed patch

This is my proposed patch to fix C++ compilation on NetBSD specifically, it may
help with other OSes too.  An OS without stdint.h would cause a break
regardless, and older NetBSD releases will choke on stdbool.h if compiling as
C.  So this more fodder for discussion and a temporary kludge than a proper
fix.  (I didn't supply NetBSD-specific code because I think a more general fix
is warranted.)

Comment 2

[Wan-Teh Chang]

Comment on attachment 186398 [details] [diff] [review]
proposed patch

I think this patch only needs to be reviewed by the
author of this file, jst.

David, does the #ifndef __cplusplus need to protect
#include <stdbool.h>, too?

Comment 3

[Wan-Teh Chang]

Comment on attachment 186398 [details] [diff] [review]
proposed patch

>+    #if !defined(__GNUC__) || (__GNUC__ > 2 || __GNUC_MINOR__ > 95)
>+      #include <stdbool.h>
>+    #else
>+      /*
>+       * GCC 2.91 can't deal with a typedef for bool, but a #define
>+       * works.
>+       */
>+      #define bool int
>+    #endif

The comment "GCC 2.91" should be changed to "GCC 2.9x" or
"GCC 2.91-2.95" to match the code.

Comment 4

[David H. Gutteridge]

Yeah, when compiling the code as C++, stdbool.h should be unnecessary, because a
recent C++ compiler should support bool as an integral type, and if it doesn't
it's unlikely it has the C99 stdbool.h header available anyway.  Some OSes (not
specifically dealt with in an earlier branch in the code) don't ship with
stdbool.h, and the compile will break there.

(Regarding the GCC comment, the test is applicable for any version of GCC before
the [unofficial] 2.96 version.  Also, there's a comment in a conditional further
up that references AIX and SunOS, it should be updated to include the other
flavours now covered off there.)

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 186398 [details] [diff] [review]
proposed patch

r+sr=jst. Sorry for the way late responce.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Checked in for 1.9.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 186398 [details] [diff] [review]
proposed patch

We should take this on the branch, imo.  It looks pretty safe to me and
compiling is always nice.

Comment 8

[Asa Dotzler [:asa]]

Comment on attachment 186398 [details] [diff] [review]
proposed patch

ports can pull this in locally but we're too late in the game to be taking
_any_ risk for a change that doesn't improve what we're shipping.