Bug 193332 - [WPE] Workaround for incorrect template specialization being selected when UChar=char16_t
Summary: [WPE] Workaround for incorrect template specialization being selected when UC...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WPE WebKit (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Charlie Turner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-10 12:46 PST by Charlie Turner
Modified: 2019-02-05 08:03 PST (History)
4 users (show)

See Also:


Attachments
Patch (2.19 KB, patch)
2019-01-10 12:51 PST, Charlie Turner
no flags Details | Formatted Diff | Diff
Patch for landing (2.20 KB, patch)
2019-01-14 02:20 PST, Charlie Turner
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Charlie Turner 2019-01-10 12:46:18 PST
[WPE] Workaround for incorrect template specialization being selected when UChar=char16_t
Comment 1 Charlie Turner 2019-01-10 12:51:45 PST
Created attachment 358821 [details]
Patch

Temporary fix for bad template deducation when ICU switched to the UChar=char16_t typedef. With this definition, the specliazation for Signed && Integral wins over the typedef for UChar
Comment 2 Michael Catanzaro 2019-01-11 17:07:56 PST
Comment on attachment 358821 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WTF/StringConcatenate.cpp:88
> -#if PLATFORM(WIN)
> +#if PLATFORM(WIN) || U_ICU_VERSION_MAJOR_NUM >= 59
>      EXPECT_EQ("hello 42 world", makeString("hello ", static_cast<unsigned short>(42) , " world"));
>  #else
>      EXPECT_EQ("hello * world", makeString("hello ", static_cast<unsigned short>(42) , " world")); // Treated as a character.

I wonder how many bugs this behavior difference is causing. This is really unfortunate. :/

I almost gave this r-, but the new version is better and the old approach will be obsoleted eventually. Can we remove the #if PLATFORM(WIN), though? Surely that is there because they were using newer ICU?
Comment 3 Charlie Turner 2019-01-13 15:08:42 PST
(In reply to Michael Catanzaro from comment #2)
> I almost gave this r-, but the new version is better and the old approach
> will be obsoleted eventually. Can we remove the #if PLATFORM(WIN), though?
> Surely that is there because they were using newer ICU?

IIUC, Windows hits the problem for a different reason, not sure if they use ICU, but on that platform UChar=wchar_t rather than unsigned short as assumed in this test.
Comment 4 Michael Catanzaro 2019-01-13 15:35:50 PST
Let's continue discussion of how to fix the footgun here in bug #193101.
Comment 5 Charlie Turner 2019-01-14 02:20:56 PST
Created attachment 359023 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2019-01-14 02:59:12 PST
Comment on attachment 359023 [details]
Patch for landing

Clearing flags on attachment: 359023

Committed r239920: <https://trac.webkit.org/changeset/239920>
Comment 7 WebKit Commit Bot 2019-01-14 02:59:13 PST
All reviewed patches have been landed.  Closing bug.