Summary: | Upgrade GCC baseline | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||||||
Component: | WebKit Misc. | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | achristensen, annulen, cgarcia, clopez, darin, gustavo, jbicha, jfbastien, mark.lam, mcatanzaro, saam, sam, ysuzuki | ||||||||||
Priority: | P2 | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Yusuke Suzuki
2017-07-05 03:24:54 PDT
Even if we take very conservative version (GCC 5), it means that we have full-featured C++14 including related-constexpr. (And it is also supported in MSVC 2017). GTK and WPE are ready for GCC 5, except for a couple nonessential bots that are not run by Igalia. We would prefer to defer requiring GCC 6 until next summer. AFAIK the only thing we have to change is the version number in OptionsGTK.cmake? (In reply to Michael Catanzaro from comment #2) > GTK and WPE are ready for GCC 5, except for a couple nonessential bots that > are not run by Igalia. We would prefer to defer requiring GCC 6 until next > summer. Anyway, the large part of C++17 is shipped in GCC 7. So I think if we would like to use C++17, we need to upgrade GCC to 7 at least. So, personally, GCC 5 for now seems fine to me. It would be nice if we can upgrade our GCC to 7 next summer... BTW, to use C++14 relaxed constexpr, we need to upgrade our MSVC to 2017 too. I think it is ok because Windows does not have bunch of distributions which does not have the easy way to install MSVC 2017. > AFAIK the only thing we have to change is the version number in > OptionsGTK.cmake? I think so. (In reply to Yusuke Suzuki from comment #3) > (In reply to Michael Catanzaro from comment #2) > > GTK and WPE are ready for GCC 5, except for a couple nonessential bots that > > are not run by Igalia. We would prefer to defer requiring GCC 6 until next > > summer. > > Anyway, the large part of C++17 is shipped in GCC 7. So I think if we would > like to use C++17, we need to upgrade GCC to 7 at least. > So, personally, GCC 5 for now seems fine to me. > It would be nice if we can upgrade our GCC to 7 next summer... > According to https://trac.webkit.org/wiki/WebKitGTK/DependenciesPolicy * Its fine to require now at least GCC 5.3 (that is what Ubuntu 16.04 ships) * On May of 2018 (when Ubuntu 18.04 will be released -- next LTS) it will be fine to require at least GCC 6.3 (that is what Debian 9 ships currently). * It won't be fine to require GCC >= 7 until at least 2019-2020 (when Debian 10 will be released) I understand the desire to use new compiler features, but at the same time I understand that requiring a too much new version of the compiler will cause trouble for many users. This is specially true for embedded users where the version of the compiler/toolchain is not something you can upgrade easier. I think our current policy is a good compromise between using new compiler features and allowing WebKit to be built with a reasonably old toolchain. Created attachment 314601 [details]
Patch
Created attachment 314602 [details]
Patch
Comment on attachment 314602 [details] Patch Should we move this version check to a more generic cmake file so it applies to all ports (including WPE)? Perhaps on the top level CMakeLists.txt ? At the same time, i think we can remove other workarounds we have for GCC < 5.3.0 like the one done on https://trac.webkit.org/changeset/184857 (In reply to Carlos Alberto Lopez Perez from comment #7) > Comment on attachment 314602 [details] > Patch > > Should we move this version check to a more generic cmake file so it applies > to all ports (including WPE)? Perhaps on the top level CMakeLists.txt ? I think the toplevel CMakeLists.txt is a reasonable location for this. > At the same time, i think we can remove other workarounds we have for GCC < > 5.3.0 like the one done on https://trac.webkit.org/changeset/184857 Yeah good catch. Problem is, it's hard to find these since they're scattered in different places. Created attachment 314676 [details]
Patch
Created attachment 314679 [details]
Patch
Comment on attachment 314679 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=314679&action=review > Source/WTF/wtf/Compiler.h:98 > +#if !GCC_VERSION_AT_LEAST(5, 3, 0) > +#error "Please use a newer version of GCC. WebKit requires GCC 5.3.0 or newer to compile." This seems pretty arbitrary. Since there's one fewer significant digit in the version now, it's like setting the requirement at 4.9.3. I would just set it at 5.0 and be done with it. But this is fine too. Comment on attachment 314679 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=314679&action=review >> Source/WTF/wtf/Compiler.h:98 >> +#error "Please use a newer version of GCC. WebKit requires GCC 5.3.0 or newer to compile." > > This seems pretty arbitrary. Since there's one fewer significant digit in the version now, it's like setting the requirement at 4.9.3. I would just set it at 5.0 and be done with it. > > But this is fine too. It would be fine. I've changed it. Committed r219186: <http://trac.webkit.org/changeset/219186> Done: Installed clang-3.8 from jessie-backports on the Debian Stable (currently: Jessie) bot and set it as compiler with the CC/CXX environment variables https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20Debian%20Stable%20%28Build%29 |