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

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 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 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.

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 on attachment 186398 [details] [diff] [review] proposed patch r+sr=jst. Sorry for the way late responce.

Checked in for 1.9.

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 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.