Bug 63941 – Compilation bug - gcc 4.6.0 and c++0x

• Component: WebKit Qt
• Status: RESOLVED INVALID

[Helio Chissini de Castro]

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)

[Ademar Reis]

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.

[Loïc Yhuel]

Created attachment 101575 [details]
Patch

[Ademar Reis]

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.

[Andreas Kling]

Comment on attachment 101575 [details]
Patch

r- based on ademar's comment.

[Loïc Yhuel]

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

[Ademar Reis]

(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"?

[Loïc Yhuel]

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

[Ademar Reis]

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

[Loïc Yhuel]

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

[Shawn Rutledge]

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?

[Shawn Rutledge]

err no, nullptr is intended to be the new standard... fine.  But it doesn't build right now.

[Antonio Gomes]

Ossy, is this valid?

[Antonio Gomes]

just ran it, but the patch is definitively no the way to fix it

[Michelangelo De Simone]

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]

[Michelangelo De Simone]

Created attachment 133122 [details]
Patch

[Loïc Yhuel]

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

[Michelangelo De Simone]

This looks like a bug on Qmake side; see also https://bugreports.qt-project.org/browse/QTBUG-24974

[Michelangelo De Simone]

Created attachment 138115 [details]
Patch

[Michelangelo De Simone]

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

[Loïc Yhuel]

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

[Andras Becsi]

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.

[Tor Arne Vestbø]

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.

[Michelangelo De Simone]

Created attachment 138203 [details]
Patch

[Simon Hausmann]

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.

[Michelangelo De Simone]

Created attachment 138674 [details]
Patch

[Michelangelo De Simone]

Created attachment 138677 [details]
Patch

[Michelangelo De Simone]

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

[Darin Adler]

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?

[Darin Adler]

Oops, was just looking at the wrong patch. Please disregard.

[Tor Arne Vestbø]

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

[Michelangelo De Simone]

Is this still occurring?

[Michelangelo De Simone]

Looks like this is dead. If that's not the case, feel free to reopen it.