930675 - Consider including NullPtr.h everywhere

Status: RESOLVED INVALID

(Core :: MFBT, defect)

Description

[(no longer active)]

Over in bug 784739, we're hitting cases where somebody is using an unsupported compiler (such as gcc 4.5) on a platform that we don't test, and then we end up breaking their builds.  And it's not obvious at all where NullPtr.h includes are in fact needed.

Should we consider adding such an include to mozilla-config.h.in so that we have nullptr.h across our code base?  I can write that patch, but I'd like to check with Jeff first on whether he's ok with doing this.

Comment 1

[Bill Gianopoulos [:WG9s]]

The support for gcc 4.5 is questionable.  We say 4.6 is recommended and say 4.4 will not work we don't state anything about 4.5, as to are we obligated to fix issues or not.

Comment 2

[Bill Gianopoulos [:WG9s]]

Trying to say we do a better job on this on windows.  This is the window of what is supported, this is what we build on and test etc. So easier to determine what is a bug and what is not.

Comment 3

[(no longer active)]

Yeah whether we like it or not people build our code with gcc 4.5 and we shouldn't break their builds without a good reason.  Also, note that we build with gcc 4.4 on b2g so we need to keep that working anyway.

Comment 4

[Bill Gianopoulos [:WG9s]]

Yes the Windows build prerequisites page has  a 
nice matrix with green red boxes to tell what is supported what is not and makes it clear what is tested and what is used for official builds.  Perhaps we should do the same for Linux?

Comment 5

[Bill Gianopoulos [:WG9s]]

So idea is. if supported needs to be fixed.  If unsupported maybe not.  If official and tested then regressor needs to be backed out.  etc.

Comment 6

[Jeff Walden [:Waldo]]

(In reply to :Ehsan Akhgari (needinfo? me!) [Away 10/29-11/6] from comment #0)
> And it's not obvious at all where NullPtr.h includes are in fact needed.

Why?  Any file that uses nullptr should #include "mozilla/NullPtr.h".  It's not difficult.

> Should we consider adding such an include to mozilla-config.h.in so that we
> have nullptr.h across our code base?

Well, we kind of are already, aren't we?  ;-)

It's not clear to me why including NullPtr.h in the files that obviously need it -- as |ack nullptr| finds trivially -- is unfeasible.  That said, purely adding support for a C++ keyword to compilers that don't support it, seems minimal, and arguably more minimal than our defining __STDC_LIMIT_MACROS, __STDC_FORMAT_MACROS, and __STDC_CONSTANT_MACROS from the command line.  It leaves a little bit of a sickly feeling in my stomach, but I think I could live with the command-line include.  (But I'd put it in mozilla-config.h.in and js-config.h.in rather than on the command line, to use the somewhat-central extension place we already have.  That's just bikeshedding, of course.)

Comment 7

[(no longer active)]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #6)
> (In reply to :Ehsan Akhgari (needinfo? me!) [Away 10/29-11/6] from comment
> #0)
> > And it's not obvious at all where NullPtr.h includes are in fact needed.
> 
> Why?  Any file that uses nullptr should #include "mozilla/NullPtr.h".  It's
> not difficult.

That's pretty much every .cpp file in our tree.

> > Should we consider adding such an include to mozilla-config.h.in so that we
> > have nullptr.h across our code base?
> 
> Well, we kind of are already, aren't we?  ;-)
> 
> It's not clear to me why including NullPtr.h in the files that obviously
> need it -- as |ack nullptr| finds trivially -- is unfeasible.  That said,
> purely adding support for a C++ keyword to compilers that don't support it,
> seems minimal, and arguably more minimal than our defining
> __STDC_LIMIT_MACROS, __STDC_FORMAT_MACROS, and __STDC_CONSTANT_MACROS from
> the command line.  It leaves a little bit of a sickly feeling in my stomach,
> but I think I could live with the command-line include.  (But I'd put it in
> mozilla-config.h.in and js-config.h.in rather than on the command line, to
> use the somewhat-central extension place we already have.  That's just
> bikeshedding, of course.)

So that's a yes, I assume?  :-)

I won't have enough time to write this patch today, someone else please step up!

Comment 8

[Jeff Walden [:Waldo]]

We use new enough compilers now, happily, that this is unnecessary.