Bug 168805 – We should be able to use std::tuples as keys in HashMap

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Keith Miller]

We should be able to use std::tuples as keys in HashMap

[Keith Miller]

Created attachment 302587 [details]
Patch

[WebKit Commit Bot]

Attachment 302587 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/HashFunctions.h:262:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Comment on attachment 302587 [details]
Patch

r=me

[JF Bastien]

For reference: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0029r0.html

[Keith Miller]

Looks like the gtk bots are using an outdated version of the stl. Is there any way we can get a newer version? It's not the end of the world since I can hand roll a constexpr all true function.

[Michael Catanzaro]

No, we still support building with libstdc++ 4.9, sorry.

[Keith Miller]

Created attachment 302623 [details]
Patch

[WebKit Commit Bot]

Attachment 302623 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/HashFunctions.h:261:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Miller]

Created attachment 302624 [details]
Patch

[WebKit Commit Bot]

Attachment 302624 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/HashFunctions.h:262:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[JF Bastien]

Comment on attachment 302624 [details]
Patch

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

> Source/WTF/ChangeLog:8
> +        This patch adds support for using std::tupeles as the key

Typo.

> Source/WTF/wtf/StdLibExtras.h:477
> +    for (InputType element : list)

`InputType&& element : list` is ever-so-slightly better.

[Keith Miller]

Ugh GCC sucks... I guess I'll have to try a specialized function then :/

[Keith Miller]

Created attachment 302631 [details]
Patch

[WebKit Commit Bot]

Attachment 302631 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/HashFunctions.h:270:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

The EWS that is failing is using gcc 5.4 so the requirements are much higher than 4.9. It built fine here with gcc 6.3. Windows build is failing too :-(

[Michael Catanzaro]

(In reply to comment #15)
> The EWS that is failing is using gcc 5.4 so the requirements are much higher
> than 4.9. It built fine here with gcc 6.3. Windows build is failing too :-(

And we have to support libstdc++ 5 for as long as possible; that's a runtime dependency, and as soon as we require newer, we permanently cut off security updates to Ubuntu 16.04. Unfortunate, but there's nothing to be done about it.

[JF Bastien]

(In reply to comment #16)
> (In reply to comment #15)
> > The EWS that is failing is using gcc 5.4 so the requirements are much higher
> > than 4.9. It built fine here with gcc 6.3. Windows build is failing too :-(
> 
> And we have to support libstdc++ 5 for as long as possible; that's a runtime
> dependency, and as soon as we require newer, we permanently cut off security
> updates to Ubuntu 16.04. Unfortunate, but there's nothing to be done about
> it.

FWIW that's not quite true: the runtime dependency is on the libstdc++ runtime, which is quite small, and on the ABI of some STL classes. Keith is running into issues in purely inlined and header-only code which is ABI-independent.

I'm well aware that assuring binary compatibility is hard, and that maintaining chroots  is a PITA. However, were it necessary it would be possible to update libstdc++ headers while still supporting older systems.

That being said, this issue doesn't seem to be a sufficient reason to do this.

[Keith Miller]

Created attachment 302670 [details]
Patch

[WebKit Commit Bot]

Attachment 302670 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/HashFunctions.h:266:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Miller]

Committed r212969: <http://trac.webkit.org/changeset/212969>

[Chris Dumez]

This broke the GTK build:
https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Build%29/builds/77620/steps/compile-webkit/logs/stdio

[Chris Dumez]

Comment on attachment 302670 [details]
Patch

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

> Source/WTF/wtf/HashFunctions.h:210
> +        template<typename Bool>

X11/Xlib.h has:
#define Bool int

:D

[JF Bastien]

Comment on attachment 302670 [details]
Patch

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

>> Source/WTF/wtf/HashFunctions.h:210
>> +        template<typename Bool>
> 
> X11/Xlib.h has:
> #define Bool int
> 
> :D

Hahahahahahahahahahahahahahahahahaahahahaghaa

[Michael Catanzaro]

Yeah, Xlib.h is terrible. It's not the first time we've had a conflict like this due to that header, although this particular one is new to me. I can't even call it a namespace conflict, since it doesn't namespace its definitions at all. :(

Unfortunately it's really hard to change API in any public header file that begins with "Copyright 1985"....