Implement changes for "avoid mostly-redundant await in async yield*"
#3619
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jul 28, 2022
jugglinmike
previously requested changes
Aug 6, 2022
| throw new Test262Error(".return should not be accessed"); | ||
| }, | ||
| get throw() { | ||
| throw new Test262Error(".throw should not be accessed"); |
There was a problem hiding this comment.
In cases like this, I prefer to observe execution with "flag" variables that are then asserted at the end of the test. Reason being: the exception handling mechanisms are themselves being validated, so there's a risk that whatever bug causes this statement to be evaluated would also cause the failure not to be reported.
That said, async tests are fundamentally dependent on asynchronous exception handling semantics, so I don't feel this risk is large enough to warrant another round of review. (I'm sharing my perspective anyway in case it's relevant for future tests and in case anyone out there can explain why I should reconsider.)
test/language/statements/async-generator/yield-star-promise-not-unwrapped.js
Outdated
Show resolved
Hide resolved
Ms2ger
approved these changes
Sep 6, 2022
Ms2ger
force-pushed
the
remove-yield-star-await
branch
from
a0679bb
to
b7f9d1b
Compare
Ms2ger
approved these changes
Sep 6, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These are updates for tc39/ecma262#2819.
I made the relevant modification to engine262 in a local branch, ran all of test262, and only found one test whose behavior needed to change, which was exercising the number of ticks implied by
yield*in an async generator. This PR modifies that test and also adds an explicit test for thatyield*does not perform unwrapping of the inner value.I recommend reviewing commits individually - in particular, the first commit only updates the existing test's
infoto match the current spec, which is then modified to match tc39/ecma262#2819 in the second commit. It's hard to see the change otherwise.cc @anba who wrote the test which is being updated.