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

Symbol fixes #1357

Merged
merged 3 commits into from
Aug 2, 2023
Merged

Symbol fixes #1357

merged 3 commits into from
Aug 2, 2023

Conversation

rbri
Copy link
Collaborator

@rbri rbri commented Jul 20, 2023 •
edited

  • fix an problem with typeof when used for symbols
  • take care of SymbolKey before casting - this fixes some problems when adding new symbols to java classes mapped into the js engine

@p-bakker
Copy link
Collaborator

Based on looking at the changed code and tests, I'm assuming there was an issue with the return value of the typeof opertor when called on a Symbol retrieved through .getOwnSymbolProperty/ies?

But if explanation of what was broken/is now fixed in these type of PR's would be handy I think for future reference (and to include in the release notes when time comes).

Other than that: lgtm. No test262 tests that got unbroken by this?

@rbri
Copy link
Collaborator Author

rbri commented Jul 23, 2023

@p-bakker comment updated - thanks

@gbrail
Copy link
Collaborator

gbrail commented Aug 2, 2023

Thanks -- I get what this is fixing, and I'll put together a more clear comment when I merge it.

@gbrail gbrail merged commit ff29b02 into mozilla:master Aug 2, 2023
3 checks passed
@rbri rbri deleted the symbol_fixes branch August 2, 2023 07:23
rbri added a commit to rbri/rhino that referenced this pull request Aug 18, 2023
@rbri
again (see mozilla#1357) take care of ScriptRuntime.isSymbol() == tru…
76fec0a 
…e; the object might be a NativeSymbol or a SymbolKey

(found while working on mozilla#1332)
@rbri rbri mentioned this pull request Aug 18, 2023
Merged
gbrail pushed a commit that referenced this pull request Aug 18, 2023
@rbri @gbrail
again (see #1357) take care of ScriptRuntime.isSymbol() == true; the …
04a2e13 
…object might be a NativeSymbol or a SymbolKey

(found while working on #1332)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rbri @p-bakker @gbrail