Bug 185374

Summary: [BigInt] Simplifying JSBigInt by using bool addition
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, ews-watchlist, keith_miller, mark.lam, msaboff, saam, ticaiolima, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
achristensen: review+, ews-watchlist: commit-queue-
Archive of layout-test-results from ews204 for win-future none

Yusuke Suzuki
Reported 2018-05-07 03:52:53 PDT
[BigInt] Simplifying JSBigInt by using bool addition
Attachments
Patch (5.46 KB, patch)
2018-05-07 03:54 PDT, Yusuke Suzuki 		achristensen: review+
ews-watchlist: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from ews204 for win-future (12.80 MB, application/zip)
2018-05-07 07:31 PDT, EWS Watchlist 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2018-05-07 03:54:35 PDT
Created attachment 339711 [details] Patch
Caio Lima
Comment 2 2018-05-07 04:04:57 PDT
Informal Review here. This LGTM. Thx for the Patch!
EWS Watchlist
Comment 3 2018-05-07 07:31:07 PDT
Comment on attachment 339711 [details] Patch Attachment 339711 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/7594268 New failing tests: http/tests/preload/onload_event.html
EWS Watchlist
Comment 4 2018-05-07 07:31:19 PDT
Created attachment 339719 [details] Archive of layout-test-results from ews204 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Alex Christensen
Comment 5 2018-05-08 10:27:24 PDT
Comment on attachment 339711 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339711&action=review > Source/JavaScriptCore/runtime/JSBigInt.cpp:263 > + carry += static_cast<bool>(result < a); I'm not sure if the static_cast<bool>s are necessary
Yusuke Suzuki
Comment 6 2018-05-08 23:00:43 PDT
Comment on attachment 339711 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339711&action=review Thanks! >> Source/JavaScriptCore/runtime/JSBigInt.cpp:263 >> + carry += static_cast<bool>(result < a); > > I'm not sure if the static_cast<bool>s are necessary This is not necessary. In C++, bool to unsigned value is guaranteed (true -> 1, false -> 0). But to make this intent explicit, I think having `static_cast<bool>` is nice.
Yusuke Suzuki
Comment 7 2018-05-08 23:01:24 PDT
Committed r231546: <https://trac.webkit.org/changeset/231546>
Radar WebKit Bug Importer
Comment 8 2018-05-08 23:03:15 PDT
<rdar://problem/40083720>
Note You need to log in before you can comment on or make changes to this bug.