Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

instanceof with bound function should call [Symbol.hasInstance] accessor #5918

Merged
merged 1 commit into from
May 21, 2019
Merged

instanceof with bound function should call [Symbol.hasInstance] accessor #5918

merged 1 commit into from
May 21, 2019

Conversation

boingoing
Copy link
Contributor

Fixes #5883

akroshg
akroshg reviewed Jan 23, 2019
View reviewed changes
test/Bugs/bug_5883.js
}
const B = A.bind();

({} instanceof B);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is not fast path for instanceOf, will it always going to call the OP_IsInst ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked and we have a cache of instanceof results which we use in the JIT but I think we always fall back to the helper to actually compute instanceof in the first place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to this PR but could that instanceof cache be the cause of #5915?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fatcerberus Hmm I'm not sure about #5915. Doesn't seem like the cached value is getting used incorrectly but the issue could be related. I'll see if I can find time to dig in this week.

@boingoing boingoing force-pushed the fix_5883 branch 3 times, most recently from 1304fbf to 4cc8bbb Compare May 21, 2019 19:10
@chakrabot chakrabot merged commit 8f73cae into chakra-core:master May 21, 2019
chakrabot pushed a commit that referenced this pull request May 21, 2019
@boingoing
[MERGE #5918 @boingoing] instanceof with bound function should call […
d2d9d28 
…Symbol.hasInstance] accessor

Merge pull request #5918 from boingoing:fix_5883

Fixes #5883
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fatcerberus fatcerberus fatcerberus left review comments

@akroshg akroshg akroshg left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

instanceof with bound function should call [Symbol.hasInstance]
4 participants
@boingoing @fatcerberus @akroshg @chakrabot