650304 - Use gcc C++0x mode by default, when it works as expected

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

Attachment: Patch

Now that we're getting near a switch to gcc 4.5, I think it's time to consider switching c++0x on by default with gcc, when it works and when it supports unicode literals (gcc 4.3, for instance has the c++0x experimental support but doesn't support unicode literals)

FWIW, I've been building with -std=gnu++0x on Debian for a year, with gcc 4.4 and now 4.5, with no real issues but a few at the beginning (bug 502301) and one now (bug 640494).

It has mainly two advantages: it avoids using a short wchar_t, which is always a good thing, and gcc c++0x support comes with extra checks that allowed to spot a real bug: https://bugzilla.mozilla.org/show_bug.cgi?id=502301#c10

bug 559278 is also related, but I don't think this bug needs to wait for it.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Patch

Use GNU_CXX instead of GNU_CC

Comment 2

[Mike Hommey [:glandium]]

http://hg.mozilla.org/mozilla-central/rev/7528b2718827

Comment 3

[(no longer active)]

I think this bug should be backed out because of 2 reasons:

1. The patch lies about only making this change for gcc.  It actually makes this change for any compiler which tries to be compatible with gcc, including clang, and perhaps icc too.
2. Our code base is not valid C++0x.  Try compiling mozilla-central with the clang trunk, which implements the invalid conversion from unsigned types to their signed variants.

This patch effectively causes our trunk to not be compilable with clang trunk, or any other gcc-masquerading C++0x conformant compiler.  I think we should *only* take this patch if we have evidence that our code base compiles fine with a C++0x compiler (if there exists one yet).

Comment 4

[Mike Hommey [:glandium]]

(In reply to Ehsan Akhgari [:ehsan] from comment #3)
> I think this bug should be backed out because of 2 reasons:
> 
> 1. The patch lies about only making this change for gcc.  It actually makes
> this change for any compiler which tries to be compatible with gcc,
> including clang, and perhaps icc too.

It also only does so if the compiler supports the option.

> 2. Our code base is not valid C++0x.  Try compiling mozilla-central with the
> clang trunk, which implements the invalid conversion from unsigned types to
> their signed variants.
> 
> This patch effectively causes our trunk to not be compilable with clang
> trunk, or any other gcc-masquerading C++0x conformant compiler.  I think we
> should *only* take this patch if we have evidence that our code base
> compiles fine with a C++0x compiler (if there exists one yet).

It builds fine with gcc 4.5 and 4.6 with the option enabled. If there are problems that clang catches that gcc don't, we should fix these problems. BTW, I know that the js code doesn't compile with gcc with -std=gnu++0x (it fails with a problem like bug 502301). It so happens that the js code is not compiled with -std=gnu++0x.

Comment 5

[Mike Hommey [:glandium]]

Alternatively, we do have variables for icc and clang in configure, we could opt-out of -std=gnu++0x for them.

Comment 6

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #4)
> It builds fine with gcc 4.5 and 4.6 with the option enabled.

It does mean that we're effectively building with -std=gnu++0x on our linux buildbots.

Comment 7

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

I would ask for a day to track what is the problem clang is having. The reduced test is that in c++-0x one cannot drop precision in initialization without a cast, so things like

int a = ...;
char b = a;

produce an error now that this part of c++0x has been implemented in clang. Fixing these is probably a good thing, but, as ehsan noted, we are sure to hit more of these as more of c++0x gets implemented.

Comment 8

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

I got firefox building with clang and c++0x again. I have started opening bugs with the patches.

I don't think that checking clang out of the c++0x support is a good idea. Clang and gcc have implemented different subsets of c++0x so far. As the implementation progresses, it will find that some firefox code is invalid. I can see the value in moving us to c++0x as the compilers evolve or just reverting this patch until c++0x gets more mature, but it is hard to see the point in following the implementation of just one compiler.

Comment 9

[(no longer active)]

OK, let's keep this in then.  I still don't think that C++0x is mature enough for us to claim that our code is valid C++0x, but I don't want to argue too much over this.