Tests for Symbols as WeakMap keys proposal #3678
Merged
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
Ms2ger
previously approved these changes
Oct 11, 2022
test/built-ins/FinalizationRegistry/prototype/register/heldValue-same-as-target.js
Outdated
Show resolved
Hide resolved
ptomato
force-pushed
the
symbols-as-weakmap-keys
branch
from
ac19716 to
778db61
Compare
Ms2ger
approved these changes
Oct 12, 2022
This test is duplicated by WeakSet/prototype/add/value-not-object-throw.js Originally taken from Rick's draft commit for symbols-as-weakmap-keys tests.
…map-keys Originally taken from Rick's draft commit for symbols-as-weakmap-keys tests.
WeakMap, WeakSet, WeakRef, and FinalizationRegistry all had tests verifying what would happen if they were called with a value that wasn't allowed as a weak value: before this proposal, that was a non-Object. Now, allowed weak values are Objects, well-known Symbols, and unregistered Symbols. That leaves registered Symbols that are still not allowed as weak values. This commit updates those tests to use a registered Symbol instead of an unregistered Symbol; they should still pass, regardless of whether the implementation has implemented symbols-as-weakmap-keys yet. The tests are renamed as appropriate. Also updates the frontmatter to the most current spec text, including the CanBeHeldWeakly abstract operation. See: tc39#2850
There are many existing tests for WeakMap, WeakSet, WeakRef, and FinalizationRegistry using Objects as weak values. For symbols-as-weakmap- keys, we'll want to write tests that parallel these for Symbol keys. Change the descriptions and filenames of these tests to describe their new scope of only Object weak values. Update the front matter of these tests while we're at it, to reflect the changes to the spec text that the symbols-as-weakmap-keys brings in. (In some cases, remove irrelevant bits of the front matter.) See: tc39#2850
This adds tests to WeakMap, WeakSet, WeakRef, and FinalizationRegistry for Symbols as weakly-held values. Regular symbols and well-known symbols are both tested. These tests correspond to existing tests for Objects as weakly-held values, but are put in separate files so that they can be filtered out with the "symbols-as-weakmap-keys" feature flag. Registered symbols are not allowed; this is already tested in the "cannot- be-held-weakly" tests. See: tc39#2850
It's possible for an async test to throw a Symbol (harness/async-gc.js does this.) The Symbol ends up getting passed to $DONE in a .then($DONE, $DONE) call. Previously, $DONE would then throw an exception due to not being able to convert the Symbol to a string.
This looks like a bug: resolveAsyncGC() is supposed to succeed if the thrown value is asyncGC.notCollected, but instead it would call $DONE() twice. An added "return" prevents that.
Ms2ger
force-pushed
the
symbols-as-weakmap-keys
branch
from
778db61 to
5ec189d
Compare
webkit-commit-queue
pushed a commit
to sosukesuzuki/WebKit
that referenced
this pull request
May 6, 2024
…iles https://bugs.webkit.org/show_bug.cgi?id=273746 Reviewed by Yusuke Suzuki. We had been skipping several test262 test cases that were broken due to the implementation of the Symbols as WeakMap keys proposal[1]. However, those tests have been fixed or removed in tc39/test262#3678[2]. This patch removes those tests from the skipped files. [1]: https://github.com/tc39/proposal-symbols-as-weakmap-keys [2]: tc39/test262#3678 * JSTests/test262/config.yaml: Canonical link: https://commits.webkit.org/278395@main
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.
Tests for the Symbols as WeakMap keys proposal which reached Stage 3 in June 2022.
Note that because I updated the frontmatter to reflect the changes in the proposal's spec text, rename detection is not working on some of these files.
Closes: #2850