Bug 185194 – [GTK] Error building WebKitGTK+ with gcc version 7.3.0

• Component: WebKitGTK
• Status: RESOLVED INVALID

[Miguel Gomez]

Created attachment 339297 [details]
Build log.

WebKitGtk+ is not building on current ToT (r231227). It fails because we are using optional RefPtr attributes with classes that are forward-defined. I think it's related to https://trac.webkit.org/changeset/231223, that causes the compiler to use std::optional instead of the WTF implmentation. Maybe with std::optional we need to have the complete class definition?

Attaching the build log.

[Yusuke Suzuki]

(In reply to Miguel Gomez from comment #0)
> Created attachment 339297 [details]
> Build log.
> 
> WebKitGtk+ is not building on current ToT (r231227). It fails because we are
> using optional RefPtr attributes with classes that are forward-defined. I
> think it's related to https://trac.webkit.org/changeset/231223, that causes
> the compiler to use std::optional instead of the WTF implmentation. Maybe
> with std::optional we need to have the complete class definition?
> 
> Attaching the build log.

Hmm, strange. In my environment (Debian unstable, gcc (Debian 7.3.0-17) 7.3.0), WebKitGTK+ is built without any errors.
Could you take a look at this issue?

[Miguel Gomez]

Oh, sorry, testing too many environments at the same time. My gcc version is

gcc (GCC) 7.3.1 20180130 (Red Hat 7.3.1-2)

[Miguel Gomez]

These are the problematic definitions:

Source/WebCore/html/HTMLMediaElement.h:

using MediaProvider = std::optional<Variant<
#if ENABLE(MEDIA_STREAM)
    RefPtr<MediaStream>,
#endif
#if ENABLE(MEDIA_SOURCE)
    RefPtr<MediaSource>,
#endif
    RefPtr<Blob>>>;


Source/WebCore/html/HTMLSourceElement.h

mutable std::optional<RefPtr<const MediaQuerySet>> m_cachedParsedMediaAttribute;


These are easily fixed by adding the appropriate includes, but after that I'm getting into this:

magomez@mars:~/webkit/WebKit (master)$ Tools/Scripts/build-webkit --gtk --release
Use of uninitialized value $previousContents in chomp at /home/magomez/webkit/WebKit/Tools/Scripts/webkitdirs.pm line 1969.
Use of uninitialized value $previousContents in string ne at /home/magomez/webkit/WebKit/Tools/Scripts/webkitdirs.pm line 1972.
+ /home/magomez/webkit/WebKit/Tools/jhbuild/jhbuild-wrapper --gtk run cmake --build /home/magomez/webkit/WebKit/WebKitBuild/Release --config Release -- -j1
[2/1674] Building CXX object Source/WebCore/CMakeFiles/WebCore.dir/__/__/DerivedSources/WebCore/unified-sources/UnifiedSource20.cpp.o
FAILED: Source/WebCore/CMakeFiles/WebCore.dir/__/__/DerivedSources/WebCore/unified-sources/UnifiedSource20.cpp.o 
/usr/lib64/ccache/c++  -DBUILDING_GTK__=1 -DBUILDING_WEBKIT -DBUILDING_WITH_CMAKE=1 -DBUILDING_WebCore -DGETTEXT_PACKAGE=\"WebKit2GTK-4.0\" -DHAVE_CONFIG_H=1 -DJSC_GLIB_API_ENABLED -DSTATICALLY_LINKED_WITH_PAL=1 -DWEBKITGTK_API_VERSION_STRING=\"4.0\" -I. -I../../Source/WebCore -I../../Source/WebCore/Modules/airplay -I../../Source/WebCore/Modules/beacon -I../../Source/WebCore/Modules/applepay -I../../Source/WebCore/Modules/applepay/paymentrequest -I../../Source/WebCore/Modules/cache -I../../Source/WebCore/Modules/credentialmanagement -I../../Source/WebCore/Modules/encryptedmedia -I../../Source/WebCore/Modules/encryptedmedia/legacy -I../../Source/WebCore/Modules/entriesapi -I../../Source/WebCore/Modules/fetch -I../../Source/WebCore/Modules/geolocation -I../../Source/WebCore/Modules/indexeddb -I../../Source/WebCore/Modules/indexeddb/client -I../../Source/WebCore/Modules/indexeddb/server -I../../Source/WebCore/Modules/indexeddb/shared -I../../Source/WebCore/Modules/mediacapabilities -I../../Source/WebCore/Modules/mediacontrols -I../../Source/WebCore/Modules/mediasession -I../../Source/WebCore/Modules/mediasource -I../../Source/WebCore/Modules/mediastream -I../../Source/WebCore/Modules/navigatorcontentutils -I../../Source/WebCore/Modules/notifications -I../../Source/WebCore/Modules/paymentrequest -I../../Source/WebCore/Modules/plugins -I../../Source/WebCore/Modules/quota -I../../Source/WebCore/Modules/speech -I../../Source/WebCore/Modules/streams -I../../Source/WebCore/Modules/webaudio -I../../Source/WebCore/Modules/webauthn -I../../Source/WebCore/Modules/webdatabase -I../../Source/WebCore/Modules/webdriver -I../../Source/WebCore/Modules/websockets -I../../Source/WebCore/Modules/webvr -I../../Source/WebCore/accessibility -I../../Source/WebCore/animation -I../../Source/WebCore/bindings -I../../Source/WebCore/bindings/js -I../../Source/WebCore/bridge -I../../Source/WebCore/bridge/c -I../../Source/WebCore/bridge/jsc -I../../Source/WebCore/contentextensions -I../../Source/WebCore/crypto -I../../Source/WebCore/crypto/algorithms -I../../Source/WebCore/crypto/keys -I../../Source/WebCore/crypto/parameters -I../../Source/WebCore/css -I../../Source/WebCore/css/parser -I../../Source/WebCore/cssjit -I../../Source/WebCore/dom -I../../Source/WebCore/dom/default -I../../Source/WebCore/dom/messageports -I../../Source/WebCore/domjit -I../../Source/WebCore/editing -I../../Source/WebCore/fileapi -I../../Source/WebCore/history -I../../Source/WebCore/html -I../../Source/WebCore/html/canvas -I../../Source/WebCore/html/forms -I../../Source/WebCore/html/parser -I../../Source/WebCore/html/shadow -I../../Source/WebCore/html/track -I../../Source/WebCore/inspector -I../../Source/WebCore/inspector/agents -I../../Source/WebCore/inspector/agents/page -I../../Source/WebCore/inspector/agents/worker -I../../Source/WebCore/loader -I../../Source/WebCore/loader/appcache -I../../Source/WebCore/loader/archive -I../../Source/WebCore/loader/archive/mhtml -I../../Source/WebCore/loader/cache -I../../Source/WebCore/loader/icon -I../../Source/WebCore/mathml -I../../Source/WebCore/page -I../../Source/WebCore/page/animation -I../../Source/WebCore/page/csp -I../../Source/WebCore/page/scrolling -I../../Source/WebCore/platform -I../../Source/WebCore/platform/animation -I../../Source/WebCore/platform/audio -I../../Source/WebCore/platform/crypto -I../../Source/WebCore/platform/encryptedmedia -I../../Source/WebCore/platform/gamepad -I../../Source/WebCore/platform/graphics -I../../Source/WebCore/platform/graphics/cpu/arm -I../../Source/WebCore/platform/graphics/cpu/arm/filters -I../../Source/WebCore/platform/graphics/displaylists -I../../Source/WebCore/platform/graphics/filters -I../../Source/WebCore/platform/graphics/harfbuzz -I../../Source/WebCore/platform/graphics/harfbuzz/ng -I../../Source/WebCore/platform/graphics/iso -I../../Source/WebCore/platform/graphics/opentype -I../../Source/WebCore/platform/graphics/transforms -I../../Source/WebCore/platform/mediastream -I../../Source/WebCore/platform/mediastream/libwebrtc -I../../Source/WebCore/platform/mock -I../../Source/WebCore/platform/mock/mediasource -I../../Source/WebCore/platform/network -I../../Source/WebCore/platform/sql -I../../Source/WebCore/platform/text -I../../Source/WebCore/platform/text/icu -I../../Source/WebCore/platform/vr -I../../Source/WebCore/plugins -I../../Source/WebCore/rendering -I../../Source/WebCore/rendering/line -I../../Source/WebCore/rendering/mathml -I../../Source/WebCore/rendering/shapes -I../../Source/WebCore/rendering/style -I../../Source/WebCore/rendering/svg -I../../Source/WebCore/rendering/updating -I../../Source/WebCore/replay -I../../Source/WebCore/storage -I../../Source/WebCore/style -I../../Source/WebCore/svg -I../../Source/WebCore/svg/animation -I../../Source/WebCore/svg/graphics -I../../Source/WebCore/svg/graphics/filters -I../../Source/WebCore/svg/properties -I../../Source/WebCore/websockets -I../../Source/WebCore/workers -I../../Source/WebCore/workers/service -I../../Source/WebCore/workers/service/context -I../../Source/WebCore/workers/service/server -I../../Source/WebCore/xml -I../../Source/WebCore/xml/parser -IDerivedSources/WebCore -IDerivedSources/ForwardingHeaders/ANGLE -I../../Source/WebCore/platform/graphics/gpu -I../../Source/ThirdParty/openvr/headers -I../../Source/WebCore/platform/vr/openvr -I../../Source/ThirdParty/xdgmime/src -I../../Source/WebCore/platform/graphics/cairo -I../../Source/WebCore/platform/graphics/freetype -I../../Source/WebCore/platform/graphics/gstreamer -I../../Source/WebCore/platform/graphics/gstreamer/mse -I../../Source/WebCore/platform/graphics/gstreamer/eme -I../../Source/WebCore/platform/audio/gstreamer -I../../Source/WebCore/platform/image-decoders -I../../Source/WebCore/platform/image-decoders/bmp -I../../Source/WebCore/platform/image-decoders/gif -I../../Source/WebCore/platform/image-decoders/ico -I../../Source/WebCore/platform/image-decoders/jpeg -I../../Source/WebCore/platform/image-decoders/png -I../../Source/WebCore/platform/image-decoders/webp -I../../Source/WebCore/platform/graphics/texmap -I../../Source/WebCore/page/scrolling/coordinatedgraphics -I../../Source/WebCore/platform/graphics/texmap/coordinated -I../../Source/WebCore/platform/graphics/nicosia -I../../Source/WebCore/platform/graphics/nicosia/cairo -I../../Source/ThirdParty/ANGLE -I../../Source/ThirdParty/ANGLE/include/KHR -I../../Source/WebCore/accessibility/atk -I../../Source/WebCore/editing/atk -I../../Source/WebCore/page/gtk -I../../Source/WebCore/platform/geoclue -I../../Source/WebCore/platform/gtk -I../../Source/WebCore/platform/graphics/egl -I../../Source/WebCore/platform/graphics/glx -I../../Source/WebCore/platform/graphics/gtk -I../../Source/WebCore/platform/graphics/opengl -I../../Source/WebCore/platform/graphics/wayland -I../../Source/WebCore/platform/graphics/x11 -I../../Source/WebCore/platform/mediastream/gtk -I../../Source/WebCore/platform/network/gtk -I../../Source/WebCore/platform/network/soup -I../../Source/WebCore/platform/text/gtk -I../../Source/WebCore/bindings/gobject -isystem ../DependenciesGTK/Root/include/libxml2 -isystem ../../Source/ThirdParty/libwebrtc/Source -isystem ../../Source/ThirdParty/libwebrtc/Source/webrtc -isystem ../DependenciesGTK/Root/include/cairo -isystem ../DependenciesGTK/Root/include/freetype2/freetype -isystem ../DependenciesGTK/Root/include/freetype2 -isystem ../DependenciesGTK/Root/include/harfbuzz -isystem ../DependenciesGTK/Root/include/gstreamer-1.0 -isystem ../DependenciesGTK/Root/include/glib-2.0 -isystem ../DependenciesGTK/Root/lib/glib-2.0/include -isystem ../DependenciesGTK/Root/lib/gstreamer-1.0/include -isystem /usr/include/libdrm -isystem ../DependenciesGTK/Root/include/atk-1.0 -isystem /usr/include/enchant -isystem ../DependenciesGTK/Root/include/gio-unix-2.0 -isystem ../DependenciesGTK/Root/include/libsecret-1 -isystem ../DependenciesGTK/Root/include/libsoup-2.4 -IDerivedSources/ForwardingHeaders -I../../Source/bmalloc -IDerivedSources -I../../Source/ThirdParty -fdiagnostics-color=always -Wno-expansion-to-defined -Wno-attributes -Wno-noexcept-type -Wno-maybe-uninitialized -Wwrite-strings -Wundef -Wpointer-arith -Wmissing-format-attribute -Wformat-security -Wcast-align -Wextra -Wall  -fno-strict-aliasing -fno-exceptions -std=c++17 -fno-rtti -O2 -DNDEBUG -fPIC -MD -MT Source/WebCore/CMakeFiles/WebCore.dir/__/__/DerivedSources/WebCore/unified-sources/UnifiedSource20.cpp.o -MF Source/WebCore/CMakeFiles/WebCore.dir/__/__/DerivedSources/WebCore/unified-sources/UnifiedSource20.cpp.o.d -o Source/WebCore/CMakeFiles/WebCore.dir/__/__/DerivedSources/WebCore/unified-sources/UnifiedSource20.cpp.o -c DerivedSources/WebCore/unified-sources/UnifiedSource20.cpp
In file included from DerivedSources/ForwardingHeaders/wtf/text/StringImpl.h:32:0,
                 from DerivedSources/ForwardingHeaders/wtf/text/WTFString.h:31,
                 from ../../Source/WebCore/platform/graphics/ExtendedColor.h:32,
                 from ../../Source/WebCore/platform/graphics/Color.h:29,
                 from ../../Source/WebCore/dom/Document.h:30,
                 from ../../Source/WebCore/dom/Element.h:28,
                 from ../../Source/WebCore/html/DOMTokenList.h:28,
                 from DerivedSources/WebCore/JSDOMTokenList.h:23,
                 from DerivedSources/WebCore/JSDOMTokenList.cpp:22,
                 from DerivedSources/WebCore/unified-sources/UnifiedSource20.cpp:1:
DerivedSources/ForwardingHeaders/wtf/Vector.h: In instantiation of ‘static void WTF::VectorCopier<false, T>::uninitializedCopy(const T*, const T*, U*) [with U = WTF::Ref<WebCore::DataTransferItem>; T = WTF::Ref<WebCore::DataTransferItem>]’:
DerivedSources/ForwardingHeaders/wtf/Vector.h:265:79:   required from ‘static void WTF::VectorTypeOperations<T>::uninitializedCopy(const T*, const T*, T*) [with T = WTF::Ref<WebCore::DataTransferItem>]’
DerivedSources/ForwardingHeaders/wtf/Vector.h:874:42:   required from ‘WTF::Vector< <template-parameter-1-1>, <anonymous>, <template-parameter-1-3>, <anonymous>, <template-parameter-1-5> >::Vector(const WTF::Vector< <template-parameter-1-1>, <anonymous>, <template-parameter-1-3>, <anonymous>, <template-parameter-1-5> >&) [with T = WTF::Ref<WebCore::DataTransferItem>; long unsigned int inlineCapacity = 0; OverflowHandler = WTF::CrashOnOverflow; long unsigned int minCapacity = 16; Malloc = WTF::FastMalloc]’
/usr/include/c++/7/type_traits:1406:12:   required from ‘struct std::is_trivially_copy_constructible<WTF::Vector<WTF::Ref<WebCore::DataTransferItem> > >’
/usr/include/c++/7/optional:105:8:   required from ‘class std::_Optional_base<WTF::Vector<WTF::Ref<WebCore::DataTransferItem> > >’
/usr/include/c++/7/optional:453:11:   required from ‘class std::optional<WTF::Vector<WTF::Ref<WebCore::DataTransferItem> > >’
../../Source/WebCore/dom/DataTransferItemList.h:80:58:   required from here
DerivedSources/ForwardingHeaders/wtf/Vector.h:162:13: error: use of deleted function ‘WTF::Ref<T, <template-parameter-1-2> >::Ref(const WTF::Ref<T, <template-parameter-1-2> >&) [with T = WebCore::DataTransferItem; PtrTraits = WTF::DumbPtrTraits<WebCore::DataTransferItem>]’
             new (NotNull, dst) U(*src);
             ^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../Source/WebCore/platform/graphics/ExtendedColor.h:30:0,
                 from ../../Source/WebCore/platform/graphics/Color.h:29,
                 from ../../Source/WebCore/dom/Document.h:30,
                 from ../../Source/WebCore/dom/Element.h:28,
                 from ../../Source/WebCore/html/DOMTokenList.h:28,
                 from DerivedSources/WebCore/JSDOMTokenList.h:23,
                 from DerivedSources/WebCore/JSDOMTokenList.cpp:22,
                 from DerivedSources/WebCore/unified-sources/UnifiedSource20.cpp:1:
DerivedSources/ForwardingHeaders/wtf/Ref.h:71:5: note: declared here
     Ref(const Ref& other) = delete;
     ^~~
ninja: build stopped: subcommand failed.

[Miguel Gomez]

After some tests here and there, I decided to upgrade my Fedora in case it was a bug in gcc or the std lib. I upgraded to Fedora 27, which has gcc version 7.3.1 20180303 (Red Hat 7.3.1-5) and with after that I'm able to build without errors.

So this doesn't seem to be a WebKit problem. I'm closing it as invalid. Sorry for the noise.

[Michael Catanzaro]

Still broken here with GCC 8.0.1

*** This bug has been marked as a duplicate of bug 185159 ***

[Michael Catanzaro]

Actually, this one is different, sorry.

[Michael Catanzaro]

Still happens with freedesktop SDK's GCC 7.3.0 and today's trunk, reopening

[Michael Catanzaro]

JF, this might be a tricky one. Problem is, with --std=c++17 we must use the real std::optional rather than our own (fixed in bug #185159). But with, libstdc++ 7 std::optional does not support incomplete types. I've been fighting my way through the build, and most problems are fairly easy to fix by converting forward declarations to #includes, but now I've run into a circular dependency. IDBRequest.h now requires IDBIndex.h, because it uses std::optional<Source> with using Source = Variant<RefPtr<IDBObjectStore>, RefPtr<IDBIndex>, RefPtr<IDBCursor>>. But IDBIndex.h requires IDBRequest.h, since it uses ExceptionOr<Ref<IDBRequest>>. Forward declarations won't save us here: some code is going to need to be either rewritten or else seriously reorganized.

There was also another case (in DataTransferItemList.h) of std::optional<Vector<something>> where I had to replace std::optional with a raw pointer Vector<something>*, which was unfortunate. I fear that's probably going to be the way to fix IDBRequest.h, which is a shame because it's used in public member functions there.

[JF Bastien]

(In reply to Michael Catanzaro from comment #8)
> JF, this might be a tricky one. Problem is, with --std=c++17 we must use the
> real std::optional rather than our own (fixed in bug #185159). But with,
> libstdc++ 7 std::optional does not support incomplete types.

Can you avoid using the provided one in that case?

[Michael Catanzaro]

(In reply to JF Bastien from comment #9)
> Can you avoid using the provided one in that case?

No, because it's used by other stdlib headers, unless we drop back down to --std=c++14. Converting to raw pointers is probably more desirable. I'll keep going and see how much needs to be changed.

[Michael Catanzaro]

I'm having a *lot* of trouble with this. JF, is it OK if I rollout r231170 for now? I think we need to; unfortunately, I don't see a path right now to using --std=c++17 with GCC 7.

GCC 6 and GCC 8 don't seem to have any such problems. GCC 6 is good because it doesn't support std::optional yet; that's why all the bots are green. And I guess GCC 8 has a smarter std::optional.

--std=c++17 is still experimental, even in GCC 8. I think we still gained a lot from requiring GCC 6, even if we backtrack on making the transition to C++ 17 so early on.

(In reply to Miguel Gomez from comment #4)
> After some tests here and there, I decided to upgrade my Fedora in case it
> was a bug in gcc or the std lib. I upgraded to Fedora 27, which has gcc
> version 7.3.1 20180303 (Red Hat 7.3.1-5) and with after that I'm able to
> build without errors.

Are you sure you kept the WebKit revision the same? Because Yusuke's patch to use system std::optional got reverted for a little while. My guess is you tested Fedora 27's GCC using a revision where it had been reverted.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #11)
> GCC 6 and GCC 8 don't seem to have any such problems. GCC 6 is good because
> it doesn't support std::optional yet; that's why all the bots are green. And
> I guess GCC 8 has a smarter std::optional.

Well, GCC 8 actually can't compile WebCore, it just crashes... I'll try to find a workaround soon. But it gets far enough that I'm pretty sure it doesn't have this problem with std::optional.

[JF Bastien]

(In reply to Michael Catanzaro from comment #11)
> I'm having a *lot* of trouble with this. JF, is it OK if I rollout r231170
> for now? I think we need to; unfortunately, I don't see a path right now to
> using --std=c++17 with GCC 7.

I'm fine with it, but it might be better to ask on the thread where we agreed to do this migration, with an explanation of what's going on?

The next step should be this:
  https://bugs.webkit.org/show_bug.cgi?id=185176
But I was waiting on hearing from your work to make sure I don't dig a bigger hole that we'd need to dig ourselves out of.

Given the lack of bots, it seems like GCC 7 wasn't an actually supported target when we agreed to do this migration. Are you suggesting that GCC 7 should be officially supported? If so, can we ensure bot coverage of both GCC 6 as well as 7 so that breakage can be avoided?

[Yusuke Suzuki]

(In reply to JF Bastien from comment #13)
> (In reply to Michael Catanzaro from comment #11)
> > I'm having a *lot* of trouble with this. JF, is it OK if I rollout r231170
> > for now? I think we need to; unfortunately, I don't see a path right now to
> > using --std=c++17 with GCC 7.
> 
> I'm fine with it, but it might be better to ask on the thread where we
> agreed to do this migration, with an explanation of what's going on?
> 
> The next step should be this:
>   https://bugs.webkit.org/show_bug.cgi?id=185176
> But I was waiting on hearing from your work to make sure I don't dig a
> bigger hole that we'd need to dig ourselves out of.
> 
> Given the lack of bots, it seems like GCC 7 wasn't an actually supported
> target when we agreed to do this migration. Are you suggesting that GCC 7
> should be officially supported? If so, can we ensure bot coverage of both
> GCC 6 as well as 7 so that breakage can be avoided?

I'm really worried about GCC 7.0's current situation. Next chance we upgrade our compiler is maybe two years later.
If we can upgrade our GCC to 8.0 at that time, it is OK. But if we cannot do that and we need to use GCC 7.0 at that time,
it means we need 4 years to use C++17 from now.

Can we solve the current compile errors by carefully splitting headers, adding includes, etc.? If we can do that, I think we should do.

[Yusuke Suzuki]

(In reply to Michael Catanzaro from comment #8)
> JF, this might be a tricky one. Problem is, with --std=c++17 we must use the
> real std::optional rather than our own (fixed in bug #185159). But with,
> libstdc++ 7 std::optional does not support incomplete types. I've been
> fighting my way through the build, and most problems are fairly easy to fix
> by converting forward declarations to #includes, but now I've run into a
> circular dependency. IDBRequest.h now requires IDBIndex.h, because it uses
> std::optional<Source> with using Source = Variant<RefPtr<IDBObjectStore>,
> RefPtr<IDBIndex>, RefPtr<IDBCursor>>. But IDBIndex.h requires IDBRequest.h,
> since it uses ExceptionOr<Ref<IDBRequest>>. Forward declarations won't save
> us here: some code is going to need to be either rewritten or else seriously
> reorganized.
> 
> There was also another case (in DataTransferItemList.h) of
> std::optional<Vector<something>> where I had to replace std::optional with a
> raw pointer Vector<something>*, which was unfortunate. I fear that's
> probably going to be the way to fix IDBRequest.h, which is a shame because
> it's used in public member functions there.

Another solution is, moving the current our std::optional to WTF::optional. And use it. And in some ports, we can use official std::optional.

[Michael Catanzaro]

(In reply to JF Bastien from comment #13)
> The next step should be this:
>   https://bugs.webkit.org/show_bug.cgi?id=185176
> But I was waiting on hearing from your work to make sure I don't dig a
> bigger hole that we'd need to dig ourselves out of.

Thanks for waiting until we get this resolved.

We also have a problem with clang in bug #185198, but of course that one is related to std::optional too.

> Given the lack of bots, it seems like GCC 7 wasn't an actually supported
> target when we agreed to do this migration. Are you suggesting that GCC 7
> should be officially supported? If so, can we ensure bot coverage of both
> GCC 6 as well as 7 so that breakage can be avoided?

We definitely need to support both GCC 7 and GCC 8. I'll ask Carlos Lopez if bot coverage would be possible here.

(In reply to Yusuke Suzuki from comment #14)
> I'm really worried about GCC 7.0's current situation. Next chance we upgrade
> our compiler is maybe two years later.
> If we can upgrade our GCC to 8.0 at that time, it is OK. But if we cannot do
> that and we need to use GCC 7.0 at that time,
> it means we need 4 years to use C++17 from now.

My expectation is this:

 * Debian 10 releases with GCC 8 sometime in 2019. At this point, we'll drop GCC 6 and can require GCC 7.
 * Ubuntu 20.04 releases with GCC 9 in April 2020. At this point, we'll drop GCC 7 and can require GCC 8.

It seems like a reasonable schedule.

> Can we solve the current compile errors by carefully splitting headers,
> adding includes, etc.? If we can do that, I think we should do.

I failed at that.

(In reply to Yusuke Suzuki from comment #15)
> Another solution is, moving the current our std::optional to WTF::optional.
> And use it. And in some ports, we can use official std::optional.

We should try this. It would be a big patch, but I think it should work.

[Zan Dobersek]

GCC 7 and 8, as packaged in Debian, have no problem compiling WebKit with -std=c++17, no extraordinary fixes currently required.

This is most likely an issue in how Fedora packages GCC. I would like to have the cause identified from that perspective before proceeding to apply any workarounds to WebKit.

[Carlos Bentzen]

GCC 7.3.1 in Arch Linux also compiled OK with -std=c++17

[Michael Catanzaro]

As I mentioned earlier today, I'm actually not working with Fedora's GCC, but with the freedesktop.org GCC, which is GNOME's reference compiler, inside the BuildStream sandbox to ensure that there is no host pollution. So there is a reference platform here, and GNOME will not be able to upgrade to newer WebKit versions unless we fix this.

The fact that Miguel reproduced the exact same error with Fedora's GCC indicates that it's not somehow specific to the freedesktop SDK.

I intend to try replacing our std::optional with WTF::optional (but probably not until next week). I'm about 70% confident that will work. If it doesn't, I'll roll out the switch to std=c++17 and we can discuss how to proceed (or if we really want to be relying on experimental compiler features so soon).

[Zan Dobersek]

(In reply to Michael Catanzaro from comment #19)
> As I mentioned earlier today, I'm actually not working with Fedora's GCC,
> but with the freedesktop.org GCC, which is GNOME's reference compiler,
> inside the BuildStream sandbox to ensure that there is no host pollution. So
> there is a reference platform here, and GNOME will not be able to upgrade to
> newer WebKit versions unless we fix this.
> 

Again, given that Debian-provided GCC and libstdc++ are working just fine, this specific regression in the transition towards C++17 utilization doesn't seem to be WebKit's fault. Consider reporting the issue to whoever is packaging GCC in this setup.

> The fact that Miguel reproduced the exact same error with Fedora's GCC
> indicates that it's not somehow specific to the freedesktop SDK.
> 

He's reported in comment #4 that an upgrade fixed all issues.

> I intend to try replacing our std::optional with WTF::optional (but probably
> not until next week). I'm about 70% confident that will work. If it doesn't,
> I'll roll out the switch to std=c++17 and we can discuss how to proceed (or
> if we really want to be relying on experimental compiler features so soon).

As of C++17, std::optional is not an experimental feature.

[Michael Catanzaro]

(In reply to Zan Dobersek from comment #20)
> Again, given that Debian-provided GCC and libstdc++ are working just fine,
> this specific regression in the transition towards C++17 utilization doesn't
> seem to be WebKit's fault. Consider reporting the issue to whoever is
> packaging GCC in this setup.

No, I'm not going to report a GCC issue to the freedesktop SDK developers, that is completely absurd. They are not GCC experts. It's the latest release of GCC, 7.3.0, without any patches. I already linked you to the packaging up above, and Miguel has provided proof that it is not a distribution-specific issue.

Even if it happened to be some problem with the SDK -- which it's not -- I'm still responsible for making sure it builds.

(In reply to Zan Dobersek from comment #20)
> He's reported in comment #4 that an upgrade fixed all issues.

I suspect he may have upgraded WebKit to a version where Yusuke's change had been rolled out, see comment #11. We won't know until he returns from holidays.

> > I intend to try replacing our std::optional with WTF::optional (but probably
> > not until next week). I'm about 70% confident that will work. If it doesn't,
> > I'll roll out the switch to std=c++17 and we can discuss how to proceed (or
> > if we really want to be relying on experimental compiler features so soon).
> 
> As of C++17, std::optional is not an experimental feature.

C++ 17 is still experimental in GCC 8. The first non-experimental release will presumably be GCC 9 or perhaps GCC 10.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #21)
> It's the latest release of GCC, 7.3.0

(the latest release of GCC 7)

[Michael Catanzaro]

Zan did some investigation today and found that the upstream GCC 7.3.0 release is broken, but the problem is fixed somewhere on the GCC 7 subversion branch.

If Miguel was careful to test the same WebKit revision so as not to get tripped up by the revert of Yusuke's std::optional commit, then I'm guessing the fix landed in February or early March. gcc (GCC) 7.3.1 20180130 (Red Hat 7.3.1-2) is of course a January 30 snapshot, so we know that's broken. And gcc version 7.3.1 20180303 (Red Hat 7.3.1-5) is March 3.

Debian 7.3.0-17's is an April 27 snapshot. You can see the history here:

http://metadata.ftp-master.debian.org/changelogs/main/g/gcc-7/gcc-7_7.3.0-17_changelog

I don't know if Debian makes it easy to test old packages, but the granularity there is way smaller than what we have available in Fedora (which is just January 30 and March 3).

[Michael Catanzaro]

Note Fedora uses GCC 7.3.1 to indicate a SVN snapshot after 7.3.0... nowadays upstream GCC releases always end in .0. Seems pretty useless, but whatever.

[Michael Catanzaro]

Maybe https://github.com/gcc-mirror/gcc/commit/57b9683f0ce55a410c567fcb2dc365a2cc848d6a

[Michael Catanzaro]

So, when distributors run into this problem, we'll just have to tell them to upgrade to an SVN snapshot. That's reasonable, IMO.

[Adrian Perez]

(In reply to Michael Catanzaro from comment #26)
> So, when distributors run into this problem, we'll just have to tell them to
> upgrade to an SVN snapshot. That's reasonable, IMO.

I also think pointing distributors to the GCC bug and asking them to
upgrade GCC packages is reasonable in this particular case—they are
using a broken release of the compiler after all!