Bug 180622 – Use relaxed constexpr for StringHasher

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

[Yusuke Suzuki]

Use relaxed constexpr for StringHasher

[Yusuke Suzuki]

Created attachment 328910 [details]
Patch

[JF Bastien]

Comment on attachment 328910 [details]
Patch

Wooh! r=me

[Yusuke Suzuki]

Comment on attachment 328910 [details]
Patch

Yay!

[WebKit Commit Bot]

Comment on attachment 328910 [details]
Patch

Clearing flags on attachment: 328910

Committed r225726: <https://trac.webkit.org/changeset/225726>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35953150>

[Darin Adler]

Comment on attachment 328910 [details]
Patch

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

> Source/WTF/wtf/text/StringHasher.h:250
> +    static constexpr UChar defaultConverter(char character)
>      {
>          return character;
>      }

I believe this is incorrect on systems where char is signed. A correct version would be one of these:

    return static_cast<unsigned char>(character);
    return static_cast<uint8_t>(character);
    return static_cast<LChar>(character);

[Yusuke Suzuki]

Comment on attachment 328910 [details]
Patch

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

>> Source/WTF/wtf/text/StringHasher.h:250
>>      }
> 
> I believe this is incorrect on systems where char is signed. A correct version would be one of these:
> 
>     return static_cast<unsigned char>(character);
>     return static_cast<uint8_t>(character);
>     return static_cast<LChar>(character);

Oh, nice. I think this does not cause a problem right now since it is only used for string literals: if a character's MSB is set, it is a leading character in UTF-8.
I'll land a follow-up patch.

[Yusuke Suzuki]

Committed r225730: <https://trac.webkit.org/changeset/225730>

[JF Bastien]

Looks like this or the follow-up causes build failures on some bots?


Step #3: ../../Source/WTF/wtf/text/StringHasher.h:252:28: error: class member cannot be redeclared
Step #3:     static constexpr UChar defaultConverter(char16_t character)
Step #3:                            ^
Step #3: ../../Source/WTF/wtf/text/StringHasher.h:237:28: note: previous definition is here
Step #3:     static constexpr UChar defaultConverter(UChar character)
Step #3:                            ^

https://oss-fuzz-build-logs.storage.googleapis.com/log-520a7b40-d87a-4f05-a85f-074a673871b9.txt

[Yusuke Suzuki]

(In reply to JF Bastien from comment #10)
> Looks like this or the follow-up causes build failures on some bots?
> 
> 
> Step #3: ../../Source/WTF/wtf/text/StringHasher.h:252:28: error: class
> member cannot be redeclared
> Step #3:     static constexpr UChar defaultConverter(char16_t character)
> Step #3:                            ^
> Step #3: ../../Source/WTF/wtf/text/StringHasher.h:237:28: note: previous
> definition is here
> Step #3:     static constexpr UChar defaultConverter(UChar character)
> Step #3:                            ^
> 
> https://oss-fuzz-build-logs.storage.googleapis.com/log-520a7b40-d87a-4f05-
> a85f-074a673871b9.txt

Nice, I've opened a bug for this. https://bugs.webkit.org/show_bug.cgi?id=180656