(webkit)tanty@pomeron:~/webkit/WebKit.git (git::master)$ Tools/Scripts/build-webkit --release --gtk --update-gtk --cmakeargs="-GNinja -j8" --makeargs="-j8" ... [77/5629] Building CXX object Source/WTF/wtf/CMakeFiles/WTF.dir/gtk/MainThreadGtk.cpp.o FAILED: /usr/lib/ccache/c++ -DBUILDING_GTK__=1 -DBUILDING_WITH_CMAKE=1 -DBUILDING_WTF -DDATA_DIR=\"share\" -DENABLE_3D_RENDERING=1 -DGETTEXT_PACKAGE=\"WebKitGTK-3.0\" -DHAVE_CONFIG_H=1 -DMOZ_X11 -DUSER_AGENT_GTK_MAJOR_VERSION=537 -DUSER_AGENT_GTK_MINOR_VERSION=30 -DWEBKITGTK_API_VERSION_STRING=\"3.0\" -DWTF_PLATFORM_X11=1 -DWTF_USE_3D_GRAPHICS=1 -DWTF_USE_ACCELERATED_COMPOSITING=1 -DWTF_USE_EGL=1 -DWTF_USE_GLX=1 -DWTF_USE_GSTREAMER -DWTF_USE_LEVELDB=1 -DWTF_USE_OPENGL=1 -DWTF_USE_TEXTURE_MAPPER=1 -DWTF_USE_TEXTURE_MAPPER_GL=1 -DWTF_USE_WEBAUDIO_GSTREAMER -DXP_UNIX -std=c++11 -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-omit-frame-pointer -fno-tree-dce -O3 -DNDEBUG -I../../Source/WTF -I../../Source/WTF/wtf -I../../Source/WTF/wtf/dtoa -I../../Source/WTF/wtf/threads -I../../Source/WTF/wtf/unicode -I../../Source/ThirdParty -I. -I../Dependencies/Root/include/glib-2.0 -I../Dependencies/Root/lib64/glib-2.0/include -Wall -Wextra -Wcast-align -Wformat-security -Wmissing-format-attribute -Wpointer-arith -Wundef -Wwrite-strings -fPIC -MMD -MT Source/WTF/wtf/CMakeFiles/WTF.dir/gtk/MainThreadGtk.cpp.o -MF "Source/WTF/wtf/CMakeFiles/WTF.dir/gtk/MainThreadGtk.cpp.o.d" -o Source/WTF/wtf/CMakeFiles/WTF.dir/gtk/MainThreadGtk.cpp.o -c ../../Source/WTF/wtf/gtk/MainThreadGtk.cpp ../../Source/WTF/wtf/gtk/MainThreadGtk.cpp: In function ‘void WTF::scheduleDispatchFunctionsOnMainThread()’: ../../Source/WTF/wtf/gtk/MainThreadGtk.cpp:43:133: error: call of overloaded ‘schedule(const char [41], void (&)())’ is ambiguous ../../Source/WTF/wtf/gtk/MainThreadGtk.cpp:43:133: note: candidates are: In file included from ../../Source/WTF/wtf/gtk/MainThreadGtk.cpp:33:0: ../../Source/WTF/wtf/gobject/GMainLoopSource.h:53:10: note: void WTF::GMainLoopSource::schedule(const char*, std::function<void()>, int, std::function<void()>, GMainContext*) In file included from ../../Source/WTF/wtf/gtk/MainThreadGtk.cpp:33:0: ../../Source/WTF/wtf/gobject/GMainLoopSource.h:54:10: note: void WTF::GMainLoopSource::schedule(const char*, std::function<bool()>, int, std::function<void()>, GMainContext*) [77/5629] Building CXX object Source/WTF/wtf/CMakeFiles/WTF.dir/gtk/RunLoopGtk.cpp.o FAILED: /usr/lib/ccache/c++ -DBUILDING_GTK__=1 -DBUILDING_WITH_CMAKE=1 -DBUILDING_WTF -DDATA_DIR=\"share\" -DENABLE_3D_RENDERING=1 -DGETTEXT_PACKAGE=\"WebKitGTK-3.0\" -DHAVE_CONFIG_H=1 -DMOZ_X11 -DUSER_AGENT_GTK_MAJOR_VERSION=537 -DUSER_AGENT_GTK_MINOR_VERSION=30 -DWEBKITGTK_API_VERSION_STRING=\"3.0\" -DWTF_PLATFORM_X11=1 -DWTF_USE_3D_GRAPHICS=1 -DWTF_USE_ACCELERATED_COMPOSITING=1 -DWTF_USE_EGL=1 -DWTF_USE_GLX=1 -DWTF_USE_GSTREAMER -DWTF_USE_LEVELDB=1 -DWTF_USE_OPENGL=1 -DWTF_USE_TEXTURE_MAPPER=1 -DWTF_USE_TEXTURE_MAPPER_GL=1 -DWTF_USE_WEBAUDIO_GSTREAMER -DXP_UNIX -std=c++11 -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-omit-frame-pointer -fno-tree-dce -O3 -DNDEBUG -I../../Source/WTF -I../../Source/WTF/wtf -I../../Source/WTF/wtf/dtoa -I../../Source/WTF/wtf/threads -I../../Source/WTF/wtf/unicode -I../../Source/ThirdParty -I. -I../Dependencies/Root/include/glib-2.0 -I../Dependencies/Root/lib64/glib-2.0/include -Wall -Wextra -Wcast-align -Wformat-security -Wmissing-format-attribute -Wpointer-arith -Wundef -Wwrite-strings -fPIC -MMD -MT Source/WTF/wtf/CMakeFiles/WTF.dir/gtk/RunLoopGtk.cpp.o -MF "Source/WTF/wtf/CMakeFiles/WTF.dir/gtk/RunLoopGtk.cpp.o.d" -o Source/WTF/wtf/CMakeFiles/WTF.dir/gtk/RunLoopGtk.cpp.o -c ../../Source/WTF/wtf/gtk/RunLoopGtk.cpp ../../Source/WTF/wtf/gtk/RunLoopGtk.cpp: In member function ‘void WTF::RunLoop::wakeUp()’: ../../Source/WTF/wtf/gtk/RunLoopGtk.cpp:104:31: error: call of overloaded ‘schedule(const char [22], std::_Bind_helper<false, void (WTF::RunLoop::*)(), WTF::RunLoop* const>::type, int, WTF::RunLoop::wakeUp()::<lambda()>)’ is ambiguous ../../Source/WTF/wtf/gtk/RunLoopGtk.cpp:104:31: note: candidates are: In file included from ../../Source/WTF/wtf/RunLoop.h:40:0, from ../../Source/WTF/wtf/gtk/RunLoopGtk.cpp:28: ../../Source/WTF/wtf/gobject/GMainLoopSource.h:53:10: note: void WTF::GMainLoopSource::schedule(const char*, std::function<void()>, int, std::function<void()>, GMainContext*) In file included from ../../Source/WTF/wtf/RunLoop.h:40:0, from ../../Source/WTF/wtf/gtk/RunLoopGtk.cpp:28: ../../Source/WTF/wtf/gobject/GMainLoopSource.h:54:10: note: void WTF::GMainLoopSource::schedule(const char*, std::function<bool()>, int, std::function<void()>, GMainContext*) [77/5629] Building CXX object Source/WebCore/CMakeFiles/ANGLESupport.dir/__/ThirdParty/ANGLE/src/compiler/translator/SearchSymbol.cpp.o ninja: build stopped: subcommand failed.
This error happens after fixing bug 130027: http://trac.webkit.org/changeset/165952
(webkit)tanty@pomeron:~/webkit/WebKit.git (git::master)$ gcc --version gcc (Ubuntu/Linaro 4.7.3-1ubuntu1) 4.7.3 Copyright (C) 2012 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
The behavior is explained at: http://stackoverflow.com/questions/22146749/overload-resolution-with-stdfunction And seems to be related to: http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-active.html#2132 Possible options are: * Provide a explicit cast for the failing overloads * Bump the mimum GCC version up to 4.8.x
Created attachment 227423 [details] Patch
Created attachment 227425 [details] Patch
(In reply to comment #5) > Created an attachment (id=227425) [details] > Patch This is an alternative patch just bumping GCC version to 4.8.0
Comment on attachment 227423 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227423&action=review > Source/WTF/wtf/gtk/MainThreadGtk.cpp:43 > - GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] dispatchFunctionsFromMainThread", dispatchFunctionsFromMainThread); > + GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] dispatchFunctionsFromMainThread", (std::function<void()>) dispatchFunctionsFromMainThread); Using the std::function<void ()> constructor would be more in line with the coding style than the C-style cast: GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] dispatchFunctionsFromMainThread", std::function<void ()>(dispatchFunctionsFromMainThread)); > Source/WTF/wtf/gtk/RunLoopGtk.cpp:103 > - GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] RunLoop work", std::bind(&RunLoop::performWork, this), > + GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] RunLoop work", (std::function<void()>) std::bind(&RunLoop::performWork, this), Ditto.
I prefer to bump the gcc requirements than adding unnecessary casts to workaround the issue, even more considering that it doesn't fail to compile with clang either.
I'm of indifferent opinion here. We'll bump to GCC 4.8.0 at some point, so we might as well do it now if there's consent for that. If EFL can't yet commit to bumping, we can only deploy this requirement for the GTK port. But I'm fine with the changes in attachment #227423 [details] as well.
Created attachment 227428 [details] Patch
Created attachment 227429 [details] Patch
Comment on attachment 227423 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227423&action=review >> Source/WTF/wtf/gtk/MainThreadGtk.cpp:43 >> + GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] dispatchFunctionsFromMainThread", (std::function<void()>) dispatchFunctionsFromMainThread); > > Using the std::function<void ()> constructor would be more in line with the coding style than the C-style cast: > > GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] dispatchFunctionsFromMainThread", std::function<void ()>(dispatchFunctionsFromMainThread)); You are right, I just messed up because of reading http://www.stroustrup.com/C++11FAQ.html#std-function and the examples for expliciting the overloaded functions in the std::bind examples O:) >> Source/WTF/wtf/gtk/RunLoopGtk.cpp:103 >> + GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] RunLoop work", (std::function<void()>) std::bind(&RunLoop::performWork, this), > > Ditto. Done.
Comment on attachment 227428 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227428&action=review > Source/cmake/OptionsGTK.cmake:311 > + message(FATAL_ERROR "You need at least GCC 4.8.0 and current version is ${CMAKE_CXX_COMPILER_VERSION}") Modified the previous patch so this error condition only happens for the GTK port. Now, it seems that there is shared GStreamer code with the EFL port that would fail also because of the same reason: https://bugs.webkit.org/show_bug.cgi?id=130078 Hence, I suppose the original patch was actually better than this, now.
Comment on attachment 227428 [details] Patch We cannot bump our dependencies until the next major release. Seems silly to do it for something that has a trivial workaround too.
(In reply to comment #14) > (From update of attachment 227428 [details]) > We cannot bump our dependencies until the next major release. Seems silly to do it for something that has a trivial workaround too. This is a building dependency, not a running one, so I'm not sure it would then be a reason to wait until a major release. In any case, I'm all for going with explicit casts :)
(In reply to comment #15) > (In reply to comment #14) > > (From update of attachment 227428 [details] [details]) > > We cannot bump our dependencies until the next major release. Seems silly to do it for something that has a trivial workaround too. > > This is a building dependency, not a running one, so I'm not sure it would then be a reason to wait until a major release. Exactly. > In any case, I'm all for going with explicit casts :) I would add a FIXME or something, since it's a workaround that works with newer gcc and clang. I prefer to keep the code clone and less confusing and bump the gcc requirements.
(In reply to comment #16) > > This is a building dependency, not a running one, so I'm not sure it would then be a reason to wait until a major release. > > Exactly. My understanding is that if you cannot build it on old Debian, you cannot ship it on old Debian. Perhaps that's not how it works though?
Comment on attachment 227429 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227429&action=review > Source/WTF/wtf/gtk/RunLoopGtk.cpp:104 > + GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] RunLoop work", std::function<void()>(std::bind(&RunLoop::performWork, this)), > G_PRIORITY_DEFAULT, [this] { deref(); }); Please use a lambda with a RefPtr capture here instead of calling ref and deref.
(In reply to comment #18) > (From update of attachment 227429 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=227429&action=review > > > Source/WTF/wtf/gtk/RunLoopGtk.cpp:104 > > + GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] RunLoop work", std::function<void()>(std::bind(&RunLoop::performWork, this)), > > G_PRIORITY_DEFAULT, [this] { deref(); }); > > Please use a lambda with a RefPtr capture here instead of calling ref and deref. I thought about it, but then the lambda does nothing and it looked more confusing to me, is that preferred? I guess it would be something like RefPtr<RunLoop> protector(this); GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] RunLoop work", std::function<void()>(std::bind(&RunLoop::performWork, this)), [protector] {}); right?
(In reply to comment #17) > My understanding is that if you cannot build it on old Debian, you > cannot ship it on old Debian. Perhaps that's not how it works > though? I don't think there's any problem from the Debian side since we already switched to gcc 4.8 in both the 2.2 and the 2.4 branches. The problem would be in Debian stable, but I think there are other dependencies missing, so I wouldn't bother too much.
(In reply to comment #20) > (In reply to comment #17) > > My understanding is that if you cannot build it on old Debian, you > > cannot ship it on old Debian. Perhaps that's not how it works > > though? > > I don't think there's any problem from the Debian side since we > already switched to gcc 4.8 in both the 2.2 and the 2.4 branches. > > The problem would be in Debian stable, but I think there are other > dependencies missing, so I wouldn't bother too much. And EFL guys already agreed on upgrading their bots to use gcc 4.8, they are working on it, so it will happen soon. I still prefer to bump the gcc requirements instead of adding unnecessary casts as a workaround.
> > I guess it would be something like > > RefPtr<RunLoop> protector(this); > GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] RunLoop work", std::function<void()>(std::bind(&RunLoop::performWork, this)), [protector] {}); Why is the second parameter even needed? Can't you do something like RefPtr<RunLoop> runLoop(this); GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] RunLoop work", [runLoop] { runLoop->performWork(); }); As long as the function object is destroyed at the right time, no extra parameter is needed.
(In reply to comment #22) > > > > I guess it would be something like > > > > RefPtr<RunLoop> protector(this); > > GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] RunLoop work", std::function<void()>(std::bind(&RunLoop::performWork, this)), [protector] {}); > > Why is the second parameter even needed? Can't you do something like > > RefPtr<RunLoop> runLoop(this); > GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] RunLoop work", [runLoop] { > runLoop->performWork(); > }); > > As long as the function object is destroyed at the right time, no extra parameter is needed. Yes, this would work for this particular case, since the function is detroyed right before the destroy callback.
Not only GMainLoopSource::createAndDeleteOnDestroy().schedule() fails at casting when gcc is less than 4.8, but also, in r163536, the function std::chrono::steady_clock::now() is used and it is only supported since gcc 4.8. So either EFL and GTK ports need gcc 4.8
(In reply to comment #20) > I don't think there's any problem from the Debian side since we > already switched to gcc 4.8 in both the 2.2 and the 2.4 branches. > > The problem would be in Debian stable, but I think there are other > dependencies missing, so I wouldn't bother too much. Can we guarantee this for all downstreams though?
(In reply to comment #25) > > I don't think there's any problem from the Debian side since we > > already switched to gcc 4.8 in both the 2.2 and the 2.4 branches. > Can we guarantee this for all downstreams though? I was checking a bit the status of gcc 4.8 on some major distributions, and it seems to be available in all of them: ubuntu: since 13.10 (saucy) packages.ubuntu.com/gcc-4.8 fedora: since F9 (july 2013) https://admin.fedoraproject.org/updates/gcc?_csrf_token=5612f8b4909851d73b973c8196fa79957fcf32b7 opensuse: http://software.opensuse.org/package/gcc48 gentoo: http://packages.gentoo.org/package/sys-devel/gcc freebsd: http://svnweb.freebsd.org/ports/head/lang/gcc48/
gcc 4.9 will be released soon (see http://gcc.gnu.org/ml/gcc/2014-03/msg00190.html) so I would say let's support 4.7 for the time being and require 4.8 once the new version is available.
(In reply to comment #24) > Not only GMainLoopSource::createAndDeleteOnDestroy().schedule() fails at casting when gcc is less than 4.8, but also, in r163536, the function std::chrono::steady_clock::now() is used and it is only supported since gcc 4.8. So either EFL and GTK ports need gcc 4.8 Reading the documentation for libstdc++, not really ... steady_clock was the same than system_clock and since 4.8.1 they are different classes. However, not having any bot with GCC < 4.8.x will make it difficult to detect when new code that is not supported is added. I think if we want to keep supporting GCC 4.7.x we need a bot using it or we will just keep chasing changes that may break the compilation. Anyway, I'm double checking just not.
Created attachment 228738 [details] Patch
(In reply to comment #29) > Created an attachment (id=228738) [details] > Patch The problem is spreading to many other places in the code. If we really want to keep compatibility with GCC < 4.8 we will *need* a buildbot in the waterfall that would be compiliing with that version of GCC. Otherwise, new changes will get into the trunk unnoticed.
(In reply to comment #28) > (In reply to comment #24) > > Not only GMainLoopSource::createAndDeleteOnDestroy().schedule() fails at casting when gcc is less than 4.8, but also, in r163536, the function std::chrono::steady_clock::now() is used and it is only supported since gcc 4.8. So either EFL and GTK ports need gcc 4.8 > > Reading the documentation for libstdc++, not really ... steady_clock was the same than system_clock and since 4.8.1 they are different classes. Double checked: this is NOT a problem with GCC 4.7.x
Created attachment 228739 [details] Patch
(In reply to comment #32) > Created an attachment (id=228739) [details] > Patch I think we only have the option of bumping GCC version without a gatekeeper to check when new code not compatible with GCC 4.7.x is added. I'm upgrading GCC to 4.8 myself now so I won't know any more ...
Comment on attachment 228738 [details] Patch We can do this or simply rename on of the schedule functions.
(In reply to comment #34) > (From update of attachment 228738 [details]) > We can do this or simply rename on of the schedule functions. Please, don't rename the functions, I'm fine adding workarounds for broken compiler versions, but not changing an API for that if not required.
Comment on attachment 228738 [details] Patch Clearing flags on attachment: 228738 Committed r166916: <http://trac.webkit.org/changeset/166916>
All reviewed patches have been landed. Closing bug.