Bug 179903 - [ESNext][BigInt] Support logic operations
Summary: [ESNext][BigInt] Support logic operations
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Caio Lima
URL:
Keywords: InRadar
: 182215 186230 186231 (view as bug list)
Depends on:
Blocks: 179001
  Show dependency treegraph
 
Reported: 2017-11-20 16:11 PST by Caio Lima
Modified: 2018-12-04 10:55 PST (History)
10 users (show)

See Also:


Attachments
WIP - Patch (15.35 KB, patch)
2018-10-16 12:16 PDT, Caio Lima
no flags Details | Formatted Diff | Diff
WIP - Patch (15.49 KB, patch)
2018-10-16 17:51 PDT, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (15.52 KB, patch)
2018-11-01 17:33 PDT, Caio Lima
no flags Details | Formatted Diff | Diff
WIP - Patch (15.48 KB, patch)
2018-11-08 06:56 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-sierra (2.57 MB, application/zip)
2018-11-08 07:59 PST, Build Bot
no flags Details
Patch (15.79 KB, patch)
2018-11-13 15:46 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (15.84 KB, patch)
2018-11-14 08:35 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (15.75 KB, patch)
2018-11-18 16:36 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (15.75 KB, patch)
2018-11-26 03:59 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (15.73 KB, patch)
2018-11-28 05:35 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (15.78 KB, patch)
2018-12-03 16:20 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (15.78 KB, patch)
2018-12-04 03:33 PST, Caio Lima
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews117 for mac-sierra (2.04 MB, application/zip)
2018-12-04 06:11 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Caio Lima 2017-11-20 16:11:35 PST
Support operations like "&&" and "||" with BigInt.
Comment 1 Caio Lima 2018-10-16 12:16:43 PDT
Created attachment 352488 [details]
WIP - Patch
Comment 2 Caio Lima 2018-10-16 17:51:26 PDT
Created attachment 352527 [details]
WIP - Patch
Comment 3 Caio Lima 2018-10-22 11:33:28 PDT
*** Bug 186231 has been marked as a duplicate of this bug. ***
Comment 4 Caio Lima 2018-10-22 11:33:56 PDT
*** Bug 186230 has been marked as a duplicate of this bug. ***
Comment 5 Caio Lima 2018-10-22 11:44:32 PDT
*** Bug 182215 has been marked as a duplicate of this bug. ***
Comment 6 Caio Lima 2018-11-01 17:33:43 PDT
Created attachment 353670 [details]
Patch
Comment 7 Caio Lima 2018-11-08 06:56:22 PST
Created attachment 354236 [details]
WIP - Patch
Comment 8 Build Bot 2018-11-08 07:59:19 PST
Comment on attachment 354236 [details]
WIP - Patch

Attachment 354236 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9909202

New failing tests:
http/wpt/css/css-animations/start-animation-001.html
Comment 9 Build Bot 2018-11-08 07:59:21 PST
Created attachment 354240 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 10 Caio Lima 2018-11-13 15:46:48 PST
Created attachment 354714 [details]
Patch
Comment 11 Caio Lima 2018-11-14 08:35:34 PST
Created attachment 354812 [details]
Patch
Comment 12 Caio Lima 2018-11-18 16:36:17 PST
Created attachment 355244 [details]
Patch
Comment 13 Caio Lima 2018-11-26 03:59:38 PST
Created attachment 355634 [details]
Patch
Comment 14 Caio Lima 2018-11-28 05:35:20 PST
Created attachment 355865 [details]
Patch
Comment 15 Caio Lima 2018-11-28 13:35:42 PST
Ping Review
Comment 16 Yusuke Suzuki 2018-12-02 00:40:00 PST
Comment on attachment 355865 [details]
Patch

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

r=me with nits.

> Source/JavaScriptCore/runtime/JSBigInt.cpp:1680
> +bool JSBigInt::toBoolean() const
> +{
> +    return !isZero();
> +}

This function is very very small. I think we should define `isZero` and `toBoolean` as inline functions in JSBigInt.h (inside the class JSBigInt definition).
Comment 17 Caio Lima 2018-12-03 16:20:37 PST
Created attachment 356431 [details]
Patch
Comment 18 Build Bot 2018-12-03 16:22:38 PST
Attachment 356431 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/JSBigInt.h:202:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 19 Caio Lima 2018-12-03 16:22:55 PST
Comment on attachment 356431 [details]
Patch

Thank you very much for the review!
Comment 20 WebKit Commit Bot 2018-12-03 17:12:12 PST
The commit-queue encountered the following flaky tests while processing attachment 356431 [details]:

webgl/2.0.0/conformance/more/functions/copyTexImage2D.html bug 192343 (author: justin_fan@apple.com)
The commit-queue is continuing to process your patch.
Comment 21 WebKit Commit Bot 2018-12-03 17:13:10 PST
Comment on attachment 356431 [details]
Patch

Clearing flags on attachment: 356431

Committed r238833: <https://trac.webkit.org/changeset/238833>
Comment 22 WebKit Commit Bot 2018-12-03 17:13:12 PST
All reviewed patches have been landed.  Closing bug.
Comment 23 Radar WebKit Bug Importer 2018-12-03 17:14:33 PST
<rdar://problem/46438082>
Comment 24 Ryan Haddad 2018-12-03 17:22:50 PST
As EWS indicated in https://webkit-queues.webkit.org/results/10255942, this change broke macOS Debug builds:

ERROR: JavaScriptCore has a weak external symbol in it (/Volumes/Data/slave/mojave-debug/build/WebKitBuild/Debug/JavaScriptCore.framework/Versions/A/JavaScriptCore)
ERROR: A weak external symbol is generated when a symbol is defined in multiple compilation units and is also marked as being exported from the library.
ERROR: A common cause of weak external symbols is when an inline function is listed in the linker export file.
ERROR: symbol __ZNK3JSC8JSBigInt9toBooleanEv
Comment 25 Ryan Haddad 2018-12-03 17:34:34 PST
Reverted r238833 for reason:

Breaks macOS and iOS debug builds.

Committed r238835: <https://trac.webkit.org/changeset/238835>
Comment 26 Caio Lima 2018-12-04 03:33:28 PST
Created attachment 356482 [details]
Patch
Comment 27 Build Bot 2018-12-04 03:36:41 PST
Attachment 356482 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/JSBigInt.h:202:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 28 Build Bot 2018-12-04 06:11:06 PST
Comment on attachment 356482 [details]
Patch

Attachment 356482 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10262081

New failing tests:
http/wpt/mediarecorder/MediaRecorder-AV-video-only-dataavailable.html
Comment 29 Build Bot 2018-12-04 06:11:07 PST
Created attachment 356490 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 30 Caio Lima 2018-12-04 10:29:30 PST
(In reply to Build Bot from comment #28)
> Comment on attachment 356482 [details]
> Patch
> 
> Attachment 356482 [details] did not pass mac-debug-ews (mac):
> Output: https://webkit-queues.webkit.org/results/10262081
> 
> New failing tests:
> http/wpt/mediarecorder/MediaRecorder-AV-video-only-dataavailable.html

Regression not related with this Patch.
Comment 31 WebKit Commit Bot 2018-12-04 10:55:33 PST
Comment on attachment 356482 [details]
Patch

Clearing flags on attachment: 356482

Committed r238861: <https://trac.webkit.org/changeset/238861>
Comment 32 WebKit Commit Bot 2018-12-04 10:55:35 PST
All reviewed patches have been landed.  Closing bug.