Bug 63941 - Compilation bug - gcc 4.6.0 and c++0x
Summary: Compilation bug - gcc 4.6.0 and c++0x
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-05 07:19 PDT by Helio Chissini de Castro
Modified: 2012-07-31 14:12 PDT (History)
18 users (show)

See Also:


Attachments
Patch (1.31 KB, patch)
2011-07-21 05:07 PDT, Loïc Yhuel
no flags Details | Formatted Diff | Diff
Patch (1.74 KB, patch)
2012-03-21 15:13 PDT, Michelangelo De Simone
no flags Details | Formatted Diff | Diff
Patch (2.03 KB, patch)
2012-04-20 10:35 PDT, Michelangelo De Simone
no flags Details | Formatted Diff | Diff
Patch (1.78 KB, patch)
2012-04-20 16:58 PDT, Michelangelo De Simone
no flags Details | Formatted Diff | Diff
Patch (1.88 KB, patch)
2012-04-24 15:51 PDT, Michelangelo De Simone
no flags Details | Formatted Diff | Diff
Patch (1.78 KB, patch)
2012-04-24 15:55 PDT, Michelangelo De Simone
vestbo: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Helio Chissini de Castro 2011-07-05 07:19:14 PDT
Compiler fails to compile due nullptr used in c++0x activated compiler flags

make[2]: *** [.obj/release-static/YarrSyntaxChecker.o] Error 1
In file included from ./wtf/PassRefPtr.h:25:0,
                 from ./wtf/CrossThreadRefCounted.h:35,
                 from ./wtf/text/StringImpl.h:28,
                 from ./runtime/UString.h:26,
                 from yarr/YarrPattern.h:30,
                 from yarr/YarrInterpreter.h:29,
                 from yarr/YarrInterpreter.cpp:28:
./wtf/NullPtr.h:48:1: error: identifier 'nullptr' will become a keyword in C++0x [-Werror=c++0x-compat]
cc1plus: all warnings being treated as errors

make[2]: *** [.obj/release-static/YarrInterpreter.o] Error 1
In file included from ./wtf/PassRefPtr.h:25:0,
                 from ./wtf/CrossThreadRefCounted.h:35,
                 from ./wtf/text/StringImpl.h:28,
                 from ./runtime/UString.h:26,
                 from yarr/YarrPattern.h:30,
                 from yarr/YarrPattern.cpp:28:
./wtf/NullPtr.h:48:1: error: identifier 'nullptr' will become a keyword in C++0x [-Werror=c++0x-compat]
cc1plus: all warnings being treated as errors

make[2]: *** [.obj/release-static/YarrPattern.o] Error 1
make[2]: Leaving directory `/var/tmp/qt-4.8.0-0.10.tp.fc16-topdir/BUILD/qt-everywhere-opensource-src-4.8.0-tp/src/3rdparty/webkit/Source/JavaScriptCore'
make[1]: *** [sub-JavaScriptCore-JavaScriptCore-pro-make_default-ordered] Error 2
make[1]: *** Waiting for unfinished jobs....
make[1]: Leaving directory `/var/tmp/qt-4.8.0-0.10.tp.fc16-topdir/BUILD/qt-everywhere-opensource-src-4.8.0-tp/src/3rdparty/webkit/Source'
make: *** [sub-webkit-make_default-ordered] Error 2
error: Bad exit status from /var/tmp/qt-4.8.0-0.10.tp.fc16-topdir/BUILDROOT/rpm-tmp.lISSlc (%build)
Comment 1 Ademar Reis 2011-07-05 07:23:04 PDT
As discussed on IRC, he's using gcc-4.6.0-9.fc15.x86_64 and building QtWebKit from Qt.

The code that fails is also on trunk, I guess we're just using different flags when building inside Qt.
Comment 2 Loïc Yhuel 2011-07-21 05:07:22 PDT
Created attachment 101575 [details]
Patch
Comment 3 Ademar Reis 2011-07-21 08:41:04 PDT
Comment on attachment 101575 [details]
Patch

Removing -Wall for everybody is not the right fix, we want -Wall. We have to identify why gcc-4.6.0 is failing and fix the real problem, not hide it. :-)

I didn't r- because I'm not an official reviewer.
Comment 4 Andreas Kling 2011-07-21 08:44:26 PDT
Comment on attachment 101575 [details]
Patch

r- based on ademar's comment.
Comment 5 Loïc Yhuel 2011-07-21 09:06:59 PDT
(In reply to comment #3)
> (From update of attachment 101575 [details])
> Removing -Wall for everybody is not the right fix, we want -Wall. We have to identify why gcc-4.6.0 is failing and fix the real problem, not hide it. :-)
> 
Without the patch, there are two -Wall, the one in QMAKE_CXXFLAG_RELEASE from Fedora is after the -Wno-c++0x-compat already added in QMAKE_CXXFLAGS for GCC4.6.0.
"-Wall -Wextra ... -Wno-c++0x-compat ... -Wall"

So the patch removes second -Wall (the Fedora one), keeping the first (QtWebKit one).
Comment 6 Ademar Reis 2011-07-21 10:02:19 PDT
(In reply to comment #5)
> (In reply to comment #3)
> > (From update of attachment 101575 [details] [details])
> > Removing -Wall for everybody is not the right fix, we want -Wall. We have to identify why gcc-4.6.0 is failing and fix the real problem, not hide it. :-)
> > 
> Without the patch, there are two -Wall, the one in QMAKE_CXXFLAG_RELEASE from Fedora is after the -Wno-c++0x-compat already added in QMAKE_CXXFLAGS for GCC4.6.0.
> "-Wall -Wextra ... -Wno-c++0x-compat ... -Wall"
> 

OK, I see what you mean, but still, if -Wall was added somewhere but the intention is to ignore c++0x support warnings (via -Wno-c++0x-compat), then the flags should be introduced together. I mean, whoever is adding -Wall should also add -Wno-c++0x-compat.

Other alternatives are to remove -Werror when building on this configuration or add -Wno-c++0x-compat at the very end.

> So the patch removes second -Wall (the Fedora one), keeping the first (QtWebKit one).

I'm a bit lost here. What do you mean by "the Fedora one"?
Comment 7 Loïc Yhuel 2011-07-21 11:39:08 PDT
(In reply to comment #6)
> OK, I see what you mean, but still, if -Wall was added somewhere but the intention is to ignore c++0x support warnings (via -Wno-c++0x-compat), then the flags should be introduced together. I mean, whoever is adding -Wall should also add -Wno-c++0x-compat.
QtWebKit adds -Wno-* flags after its -Wall, but Fedora qmake.conf adds its -Wall, and obviously it doesn't know QtWebKit needs to ignore some warnings.

> Other alternatives are to remove -Werror when building on this configuration or add -Wno-c++0x-compat at the very end.
In fact it's not only -Wno-c++0x-compat, there are other -Wno-* (which could cause problems).
Adding at the very end would mean adding at least all -Wno-* to QMAKE_CXXFLAGS_RELEASE and QMAKE_CXXFLAGS_DEBUG instead of QMAKE_CXXFLAGS.

> 
> > So the patch removes second -Wall (the Fedora one), keeping the first (QtWebKit one).
> 
> I'm a bit lost here. What do you mean by "the Fedora one"?
qmake.conf from Fedora :
QMAKE_CFLAGS_RELEASE    += -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2  -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables
The patch removes this -Wall, added by Fedora Qt configuration, and keeps the one from WebKit.pri.
Comment 8 Ademar Reis 2011-07-21 11:55:31 PDT
(In reply to comment #7)
> > 
> > > So the patch removes second -Wall (the Fedora one), keeping the first (QtWebKit one).
> > 
> > I'm a bit lost here. What do you mean by "the Fedora one"?
> qmake.conf from Fedora :
> QMAKE_CFLAGS_RELEASE    += -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2  -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables
> The patch removes this -Wall, added by Fedora Qt configuration, and keeps the one from WebKit.pri.

So the patch only works when building on this specific fedora, it disables the flag everywhere else...

We're working on fixing the c++0x compatibility issues (the root of the problem) and since we're using -Werror by default on Linux, I'm afraid you'll have to workaround this for a while when building on Fedora15.

Will keep the bug open for a while, until we have more news regarding c++0x support.
Comment 9 Loïc Yhuel 2011-07-21 12:06:25 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > > 
> > > > So the patch removes second -Wall (the Fedora one), keeping the first (QtWebKit one).
> > > 
> > > I'm a bit lost here. What do you mean by "the Fedora one"?
> > qmake.conf from Fedora :
> > QMAKE_CFLAGS_RELEASE    += -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2  -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables
> > The patch removes this -Wall, added by Fedora Qt configuration, and keeps the one from WebKit.pri.
> 
> So the patch only works when building on this specific fedora, it disables the flag everywhere else...
No, QMAKE_CXXFLAGS is not modified, the -Wall added in WebKit.pri is still there, so the command line doesn't change on other distributions.
> 
> We're working on fixing the c++0x compatibility issues (the root of the problem) and since we're using -Werror by default on Linux, I'm afraid you'll have to workaround this for a while when building on Fedora15.
> 
> Will keep the bug open for a while, until we have more news regarding c++0x support.
It's not only -Wno-c++0x-compat, it's the same with -Wno-sign-compare and -Wno-switch (but I don't know if they would occur).
Comment 10 Shawn Rutledge 2011-08-05 09:15:27 PDT
I can also confirm this problem on arch linux 64-bit with gcc 4.6.1.  A quickie fix is to make sure the compiler uses the -std=gnu++98 flag.  But I think the right fix is to stop using this "nullptr" definition.  What's wrong with plain old NULL, as a #define?
Comment 11 Shawn Rutledge 2011-08-05 09:17:35 PDT
err no, nullptr is intended to be the new standard... fine.  But it doesn't build right now.
Comment 12 Antonio Gomes 2012-03-14 13:15:53 PDT
Ossy, is this valid?
Comment 13 Antonio Gomes 2012-03-14 13:16:14 PDT
just ran it, but the patch is definitively no the way to fix it
Comment 14 Michelangelo De Simone 2012-03-20 10:06:53 PDT
Same here running on GCC 4.6.1, Ubuntu x64.


In file included from ../../../../Source/JavaScriptCore/wtf/PassRefPtr.h:25:0,
                 from ../../../../Source/JavaScriptCore/wtf/ByteArray.h:30,
                 from ../../../../Source/JavaScriptCore/wtf/ByteArray.cpp:27:
../../../../Source/JavaScriptCore/wtf/NullPtr.h:46:1: error: identifier ‘nullptr’ will become a keyword in C++0x [-Werror=c++0x-compat]In file included from ../../../../Source/JavaScriptCore/wtf/OwnArrayPtr.h:26:0,
                 from ../../../../Source/JavaScriptCore/wtf/Assertions.cpp:36:
../../../../Source/JavaScriptCore/wtf/NullPtr.h:46:1: error: identifier ‘nullptr’ will become a keyword in C++0x [-Werror=c++0x-compat]
Comment 15 Michelangelo De Simone 2012-03-21 15:13:27 PDT
Created attachment 133122 [details]
Patch
Comment 16 Loïc Yhuel 2012-03-22 11:29:42 PDT
(In reply to comment #15)
> Created an attachment (id=133122) [details]
> Patch

Your problem seems different from the original cause : -Wall being added to QMAKE_CFLAGS_RELEASE in qmake.conf.
"gcc -dumpversion" is not correct when crosscompiling.
Comment 17 Michelangelo De Simone 2012-03-26 13:31:23 PDT
This looks like a bug on Qmake side; see also https://bugreports.qt-project.org/browse/QTBUG-24974
Comment 18 Michelangelo De Simone 2012-04-20 10:35:58 PDT
Created attachment 138115 [details]
Patch
Comment 19 Michelangelo De Simone 2012-04-20 10:44:44 PDT
(In reply to comment #16)

> Your problem seems different from the original cause : -Wall being added to QMAKE_CFLAGS_RELEASE in qmake.conf.

Isn't the problem related to the fact that -Wno-c++0x-compat is actually not being set, leading the build to fail (treating warnings as errors)?
-Wall is ok, we just need to disable the c++0x-compat warning to avoid encountering the nullptr "conflict".

> "gcc -dumpversion" is not correct when crosscompiling.

The patch is limited to the linux-g++ mkspec context.
Comment 20 Loïc Yhuel 2012-04-20 11:19:46 PDT
(In reply to comment #19)
> (In reply to comment #16)
> 
> > Your problem seems different from the original cause : -Wall being added to QMAKE_CFLAGS_RELEASE in qmake.conf.
> 
> Isn't the problem related to the fact that -Wno-c++0x-compat is actually not being set, leading the build to fail (treating warnings as errors)?
> -Wall is ok, we just need to disable the c++0x-compat warning to avoid encountering the nullptr "conflict".
The problem on Fedora is different : -Wno-c++0x-compat is set, but it is added to QMAKE_CFLAGS, and is overridden by the -Wall set in QMAKE_CFLAGS_RELEASE in qmake.conf (QMAKE_CFLAGS_RELEASE is after QMAKE_CFLAGS on the command line).
> 
> > "gcc -dumpversion" is not correct when crosscompiling.
> 
> The patch is limited to the linux-g++ mkspec context.
If you have several gcc versions installed, you could add specs like "linux-g++-4.6", so if it works in your case, you should use QMAKE_CC.
Comment 21 Andras Becsi 2012-04-20 11:35:43 PDT
Comment on attachment 138115 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=138115&action=review

> Tools/qmake/mkspecs/features/unix/default_post.prf:22
> +        QT_GCC_VERSION = $$system(gcc -dumpversion)

linux-g++* also includes linux-g++-maemo where the compiler is called arm-linux-gnuabi-gcc, so this is not correct.
QMAKE_CC would be better as stated above.
Comment 22 Tor Arne Vestbø 2012-04-20 12:37:10 PDT
Comment on attachment 138115 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=138115&action=review

>> Tools/qmake/mkspecs/features/unix/default_post.prf:22
>> +        QT_GCC_VERSION = $$system(gcc -dumpversion)
> 
> linux-g++* also includes linux-g++-maemo where the compiler is called arm-linux-gnuabi-gcc, so this is not correct.
> QMAKE_CC would be better as stated above.

Also, this should probably be in default_pre instead, so that the variables can be used in pro/pri files as well.
Comment 23 Michelangelo De Simone 2012-04-20 16:58:29 PDT
Created attachment 138203 [details]
Patch
Comment 24 Simon Hausmann 2012-04-23 03:52:18 PDT
Comment on attachment 138203 [details]
Patch

I don't like the idea of putting these workarounds in place here, I'd rather see this fixed in Qt 4 and then we require Qt 4.8.2 for example. In the meantime this should at least be in a haveQt(4) block.
Comment 25 Michelangelo De Simone 2012-04-24 15:51:00 PDT
Created attachment 138674 [details]
Patch
Comment 26 Michelangelo De Simone 2012-04-24 15:55:54 PDT
Created attachment 138677 [details]
Patch
Comment 27 Michelangelo De Simone 2012-04-24 15:59:22 PDT
(In reply to comment #24)
> (From update of attachment 138203 [details])
> I don't like the idea of putting these workarounds in place here, I'd rather see this fixed in Qt 4 and then we require Qt 4.8.2 for example.

I strongly agree; it's just a temporary workaround. I've cleared this in a FIXME.

> In the meantime this should at least be in a haveQt(4) block.

Done.
Comment 28 Darin Adler 2012-04-24 17:17:39 PDT
Comment on attachment 138674 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=138674&action=review

> Tools/qmake/mkspecs/features/unix/default_pre.prf:26
> +        message("DENTROOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO")

What is this?
Comment 29 Darin Adler 2012-04-24 17:20:09 PDT
Oops, was just looking at the wrong patch. Please disregard.
Comment 30 Tor Arne Vestbø 2012-04-25 00:21:26 PDT
Comment on attachment 138677 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=138677&action=review

> Tools/qmake/mkspecs/features/unix/default_pre.prf:25
> +haveQt(4):linux-g++*:if (isEmpty(QT_GCC_MAJOR_VERSION) || isEmpty(QT_GCC_MINOR_VERSION)) {

This is not valid qmake syntax i believe, please write as:

haveQt(4):qpa {
    # Qt doesn't expose these constants if built with QPA enabled.
    # See https://bugs.webkit.org/show_bug.cgi?id=63941 for details.
    # FIXME: This is a temporary workaround for QTBUG-24974. 
    isEmpty(QT_GCC_MAJOR_VERSION)|isEmpty(QT_GCC_MINOR_VERSION) {
       stuff
    }
}
Comment 31 Michelangelo De Simone 2012-06-19 14:42:41 PDT
Is this still occurring?
Comment 32 Michelangelo De Simone 2012-07-31 14:12:30 PDT
Looks like this is dead. If that's not the case, feel free to reopen it.