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
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

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>