Last Comment Bug 650304 - Use gcc C++0x mode by default, when it works as expected
: Use gcc C++0x mode by default, when it works as expected
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All Linux
: -- normal (vote)
: mozilla6
Assigned To: Mike Hommey [:glandium]
:
Mentors:
Depends on: gcc4.5 640494 652139 654082 654493 654653
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-15 09:16 PDT by Mike Hommey [:glandium]
Modified: 2011-09-05 10:17 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (1.47 KB, patch)
2011-04-15 09:16 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
Patch (1.47 KB, patch)
2011-04-16 01:04 PDT, Mike Hommey [:glandium]
benjamin: review+
Details | Diff | Review

Description Mike Hommey [:glandium] 2011-04-15 09:16:50 PDT
Created attachment 526277 [details] [diff] [review]
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] 2011-04-16 01:04:40 PDT
Created attachment 526464 [details] [diff] [review]
Patch

Use GNU_CXX instead of GNU_CC
Comment 2 Mike Hommey [:glandium] 2011-05-03 01:39:14 PDT
http://hg.mozilla.org/mozilla-central/rev/7528b2718827
Comment 3 :Ehsan Akhgari (busy, don't ask for review please) 2011-08-08 20:10:30 PDT
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] 2011-08-08 23:14:49 PDT
(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] 2011-08-08 23:15:56 PDT
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] 2011-08-08 23:17:15 PDT
(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) 2011-08-09 05:29:55 PDT
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) 2011-08-09 12:15:53 PDT
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 :Ehsan Akhgari (busy, don't ask for review please) 2011-08-10 16:11:05 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.