Summary: | TestWTF.WTF.StringConcatenate_Unsigned fails for unsigned short on Windows | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ross Kirsling <ross.kirsling> | ||||||||||||
Component: | Web Template Framework | Assignee: | Ross Kirsling <ross.kirsling> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | achristensen, benjamin, bfulgham, cdumez, cmarcelo, commit-queue, dbates, don.olmstead, ews-watchlist, Hironori.Fujii, jfbastien, pvollan, stephan.szabo, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=193101 | ||||||||||||||
Attachments: |
|
Created attachment 345123 [details]
Patch
The reason for #if OS(WINDOWS) here is because we need a raw `unsigned short` in order to get prioritized above the is_integral && !is_signed case, yet SFINAE only works on template params, so we can't do something like the following:
> template<>
> class StringTypeAdapter<unsigned short, typename std::enable_if_t<std::is_same<UChar, wchar_t>::value>> : public StringTypeAdapter<UChar, void>
See following bugs about this issue: Bug 180720 – makeString: support more integral types Bug 180753 – makeString: add single-character helpers Comment on attachment 345123 [details] Patch Attachment 345123 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8560502 New failing tests: http/tests/security/canvas-remote-read-remote-video-redirect.html http/tests/security/canvas-remote-read-remote-video-localhost.html Created attachment 345150 [details]
Archive of layout-test-results from ews205 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
(In reply to Fujii Hironori from comment #3) > See following bugs about this issue: > > Bug 180720 – makeString: support more integral types > Bug 180753 – makeString: add single-character helpers Thanks Fujii-san! JF, do you have an opinion about how this should be handled? (Am I just making Windows behavior worse so I can please a test case?) Rereading Bug 180720 and Bug 180753, would it make the most sense to simply guard this oddly-behaving test with #if !PLATFORM(WIN) for now? Created attachment 345424 [details]
Patch
Comment on attachment 345424 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=345424&action=review LGTM. > Tools/ChangeLog:1 > +2018-07-19 Ross Kirsling <rkirsling@gmail.com> gmail.com (In reply to Fujii Hironori from comment #9) > Comment on attachment 345424 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=345424&action=review > > LGTM. > > > Tools/ChangeLog:1 > > +2018-07-19 Ross Kirsling <rkirsling@gmail.com> > > gmail.com Ack, I submitted this from a different computer. Thanks for pointing that out. Created attachment 345425 [details]
Patch for landing
Comment on attachment 345425 [details] Patch for landing Rejecting attachment 345425 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 345425, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Hironori Fujii found in /Volumes/Data/EWS/WebKit/Tools/ChangeLog does not appear to be a valid reviewer according to contributors.json. /Volumes/Data/EWS/WebKit/Tools/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: https://webkit-queues.webkit.org/results/8595808 Created attachment 345426 [details]
Patch for landing
Comment on attachment 345426 [details] Patch for landing Clearing flags on attachment: 345426 Committed r234024: <https://trac.webkit.org/changeset/234024> All reviewed patches have been landed. Closing bug. |
On Windows, this API test failure occurs: > TestWTF.WTF.StringConcatenate_Unsigned > > ..\..\Tools\TestWebKitAPI\Tests\WTF\StringConcatenate.cpp:84 > Value of: makeString("hello ", static_cast<unsigned short>(42) , " world") > Actual: hello 42 world > Expected: "hello * world" > Which is: 00007FFAF03F82E8 The issue seems to be a bad template deduction for StringTypeAdapter when UCHAR_TYPE is wchar_t.