-
-
Notifications
You must be signed in to change notification settings - Fork 406
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.
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
[Merged by Bors] - Implement optional chains #2390
Conversation
Test262 conformance changes
Fixed tests (56):
|
Codecov Report
@@ Coverage Diff @@
## main #2390 +/- ##
==========================================
- Coverage 40.03% 39.81% -0.23%
==========================================
Files 305 307 +2
Lines 23336 23547 +211
==========================================
+ Hits 9343 9375 +32
- Misses 13993 14172 +179
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking very good! Thanks for the implementation :) I added some comments that could be interesting to check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks perfect to me :)
bors r+ |
This Pull Request implements optional chains. Example: ```Javascript const adventurer = { name: 'Alice', cat: { name: 'Dinah' } }; console.log(adventurer.cat?.name); // Dinah console.log(adventurer.dog?.name); // undefined ``` Since I needed to implement `Opcode::RotateLeft`, and #2378 had an implementation for `Opcode::RotateRight`, I took the opportunity to integrate both ops into this PR (big thanks to @HalidOdat for the original implementation!). This PR almost has 100% conformance for the `optional-chaining` test suite. However, there's this one [test](https://github.com/tc39/test262/blob/85373b4ce12a908f8fc517093d95cf2ed2f5ee6a/test/language/expressions/optional-chaining/member-expression.js) that can't be solved until we properly set function names for function expressions in object and class definitions.
Pull request successfully merged into main. Build succeeded: |
Fixes #1917 (fixes the code example given with the hashes) Fixes the order dependent execution of assignment, so the following code works correctly: ```javascript function f(x) { console.log(x) } // used to check the order of execution let a = [[]] (f(1), a)[(f(2), 0)][(f(3), 0)] = (f(4), 123) // 🤮 ``` Prints out: ```bash 1 2 3 4 123 ``` As expected This introduces some opcodes: - ~~`Swap3`: currently used only to keep some previous code working that needs refactoring.~~ - ~~`RotateRight n`: Rotates the `n` top values from the top of the stack to the right by `1`.~~ Already added by #2390 ~~Besides the new opcodes,~~ Some opcodes pop and push order of values on the stack have been changed. To eliminate many swaps and to make this change easier. ~~This PR is still a WIP and needs more refactoring~~ This is now ready for review/merge :)
This Pull Request implements optional chains.
Example:
Since I needed to implement
Opcode::RotateLeft
, and #2378 had an implementation forOpcode::RotateRight
, I took the opportunity to integrate both ops into this PR (big thanks to @HalidOdat for the original implementation!).This PR almost has 100% conformance for the
optional-chaining
test suite. However, there's this one test that can't be solved until we properly set function names for function expressions in object and class definitions.