Bug 210954 - Use @isUndefinedOrNull instead of abstract equality with null
Summary: Use @isUndefinedOrNull instead of abstract equality with null
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Minor
Assignee: Alexey Shvayka
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-23 23:32 PDT by Alexey Shvayka
Modified: 2020-05-21 11:38 PDT (History)
10 users (show)

See Also:


Attachments
Patch (9.29 KB, patch)
2020-04-23 23:36 PDT, Alexey Shvayka
no flags Details | Formatted Diff | Diff
Patch (11.66 KB, patch)
2020-05-18 13:11 PDT, Alexey Shvayka
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Shvayka 2020-04-23 23:32:40 PDT
Use @isUndefinedOrNull instead of abstract equality with null
Comment 1 Alexey Shvayka 2020-04-23 23:36:59 PDT
Created attachment 397430 [details]
Patch
Comment 2 Yusuke Suzuki 2020-04-23 23:50:30 PDT
Comment on attachment 397430 [details]
Patch

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

r=me with comment.

> Source/JavaScriptCore/ChangeLog:14
> +        b) Replaces 3 `!= @undefined` and 7 `!= null` comparisons with @isUndefinedOrNull()
> +        as only the latter is correct with [[IsHTMLDDA]] aka MasqueradesAsUndefined objects [2].

Can you add a test with IsHTMLDDA? jsc shell has IsHTMLDDA mechanism so we can write a test for MasqueradesAsUndefined with JSC shell.
Comment 3 Alexey Shvayka 2020-05-18 13:11:00 PDT
Created attachment 399669 [details]
Patch

Rebase JSTests/test262/expectations.yaml and set reviewer.
Comment 4 Alexey Shvayka 2020-05-18 13:11:43 PDT
(In reply to Yusuke Suzuki from comment #2)
> Can you add a test with IsHTMLDDA? jsc shell has IsHTMLDDA mechanism so we
> can write a test for MasqueradesAsUndefined with JSC shell.

I've added test262 coverage for all the comparisons fixed by this patch (r259587 introduced proper $262.IsHTMLDDA).
These tests will be also beneficial for non-mainstream engines like Hermes, which, judging by source code
(https://github.com/facebook/hermes/blob/b6530ae7e25604839c33b60261fb2832f49ca464/lib/InternalBytecode/String.js#L673),
may have the same bug.
Comment 5 Alexey Shvayka 2020-05-18 13:13:15 PDT
Comment on attachment 399669 [details]
Patch

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

> JSTests/test262/expectations.yaml:11
>  test/annexB/built-ins/String/prototype/replaceAll/custom-replacer-emulates-undefined.js:

This test is wrong. Upstream fix: https://github.com/tc39/test262/pull/2626.
Comment 6 EWS 2020-05-21 11:37:54 PDT
Committed r262017: <https://trac.webkit.org/changeset/262017>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399669 [details].
Comment 7 Radar WebKit Bug Importer 2020-05-21 11:38:17 PDT
<rdar://problem/63502823>