Bug 180692 - REGRESSION(r225769): Build errors with constexpr std::tie on older gcc
Summary: REGRESSION(r225769): Build errors with constexpr std::tie on older gcc
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks: 178894
  Show dependency treegraph
 
Reported: 2017-12-12 02:33 PST by Ms2ger (he/him; ⌚ UTC+1/+2)
Modified: 2017-12-27 08:49 PST (History)
15 users (show)

See Also:


Attachments
Patch (4.54 KB, patch)
2017-12-12 03:43 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (4.46 KB, patch)
2017-12-12 18:16 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (4.41 KB, patch)
2017-12-12 23:29 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (4.42 KB, patch)
2017-12-12 23:29 PST, Yusuke Suzuki
cgarcia: review+
cgarcia: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ms2ger (he/him; ⌚ UTC+1/+2) 2017-12-12 02:33:37 PST
See <https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20Ubuntu%20LTS%20%28Build%29/builds/8173/steps/compile-webkit/logs/stdio> for errors like

../../Source/WebCore/platform/graphics/FontSelectionAlgorithm.h: In member function ‘constexpr bool WebCore::FontSelectionRange::operator==(const WebCore::FontSelectionRange&) const’:
../../Source/WebCore/platform/graphics/FontSelectionAlgorithm.h:287:5: error: call to non-constexpr function ‘std::tuple<_Elements& ...> std::tie(_Elements& ...) [with _Elements = {const WebCore::FontSelectionValue, const WebCore::FontSelectionValue}]’
     }
     ^
Comment 1 Yusuke Suzuki 2017-12-12 03:43:07 PST
Created attachment 329104 [details]
Patch
Comment 2 Yusuke Suzuki 2017-12-12 03:44:25 PST
Comment on attachment 329104 [details]
Patch

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

> Source/WTF/wtf/StdLibExtras.h:451
> +#if !GCC_VERSION_AT_LEAST(6, 0, 0)

GCC_VERSION_AT_LEAST is only defined in COMPILER(GCC) environment. So, we cannot use `#if COMPILER(GCC) && !GCC_VERSION_AT_LEAST(6, 0, 0)`.
Comment 3 Build Bot 2017-12-12 03:44:34 PST
Attachment 329104 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/StdLibExtras.h:454:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WTF/wtf/StdLibExtras.h:458:  Use 'using namespace std;' instead of 'using std::tie;'.  [build/using_std] [4]
ERROR: Source/WTF/wtf/StdLibExtras.h:461:  Use 'using namespace std;' instead of 'using std::tie;'.  [build/using_std] [4]
ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 4 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Carlos Alberto Lopez Perez 2017-12-12 05:12:30 PST
Comment on attachment 329104 [details]
Patch

Seems to have failed the Windows EWS build: https://webkit-queues.webkit.org/results/5628920

C:\WebKit-EWS\WebKit\Source\WTF\wtf/StdLibExtras.h(461): error C2039: 'tie': is not a member of 'std'
C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\VC\Tools\MSVC\14.12.25827\include\memory(17): note: see declaration of 'std'
C:\WebKit-EWS\WebKit\Source\WTF\wtf/StdLibExtras.h(461): error C2873: 'tie': symbol cannot be used in a using-declaration
[176/1795] Building CXX object Source\ThirdParty\ANGLE\CMakeFiles\libANGLE.dir\src\libANGLE\TransformFeedback.cpp.obj
Comment 5 Yusuke Suzuki 2017-12-12 18:16:08 PST
Created attachment 329190 [details]
Patch
Comment 6 Build Bot 2017-12-12 18:17:42 PST
Attachment 329190 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/StdLibExtras.h:452:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 2 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Carlos Alberto Lopez Perez 2017-12-12 18:31:56 PST
Comment on attachment 329190 [details]
Patch

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

> Source/WTF/wtf/StdLibExtras.h:449
> +// GCC5 does not have constexpr std::tie. Since we cannot redefine std::tie with constexpr, we define WTF::tie instead.
> +// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65978

This is a bug on libstdc++ (not on GCC itself).
So even if you build with Clang you will hit this problem if your Linux distribution ships a libstdc++ < 6
Comment 8 Yusuke Suzuki 2017-12-12 23:29:19 PST
Created attachment 329208 [details]
Patch
Comment 9 Yusuke Suzuki 2017-12-12 23:29:58 PST
Created attachment 329209 [details]
Patch
Comment 10 Build Bot 2017-12-12 23:32:36 PST
Attachment 329209 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/StdLibExtras.h:452:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 2 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Carlos Garcia Campos 2017-12-12 23:36:44 PST
Comment on attachment 329209 [details]
Patch

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

Thanks!

> Source/WebCore/ChangeLog:8
> +        No new tests (OOPS!).

Remove this line please, I think cq will reject the patch with this line in the changelog.
Comment 12 Darin Adler 2017-12-13 21:16:44 PST
Comment on attachment 329209 [details]
Patch

If we expect to keep WebKit compatible with the bugs in these older versions of GCC, I think we need to add an EWS instance for them. Otherwise, I, at least, will keep getting things like this wrong.
Comment 13 Yusuke Suzuki 2017-12-14 00:17:20 PST
Comment on attachment 329209 [details]
Patch

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

Yeah, we should have EWS + GCC5.4, which is the lower bound of our supported compiler.

>> Source/WebCore/ChangeLog:8
>> +        No new tests (OOPS!).
> 
> Remove this line please, I think cq will reject the patch with this line in the changelog.

Oops, thanks!
Comment 14 Yusuke Suzuki 2017-12-14 00:18:05 PST
Committed r225896: <https://trac.webkit.org/changeset/225896>
Comment 15 Radar WebKit Bug Importer 2017-12-14 00:19:51 PST
<rdar://problem/36042696>
Comment 16 Carlos Alberto Lopez Perez 2017-12-27 08:49:49 PST
For the record, in r226299 <https://trac.webkit.org/changeset/226299> I updated the comment on wtf/StdLibExtras.h to note that this workaround of using WTF::tie for constexpr result can be removed after 2019-04 (one year after Ubuntu 18.04 LTS is released) according to https://trac.webkit.org/wiki/WebKitGTK/DependenciesPolicy