Skip to content

Conversation

@Constellation
Copy link
Member

@Constellation Constellation commented Dec 2, 2022

e2c5cd5

[JSC] Implement JSON.parse source text access proposal
https://bugs.webkit.org/show_bug.cgi?id=248031
rdar://131579181

Reviewed by Yijia Huang.

This patch implements JSON.parse source text access proposal[1], which is now stage-3.
This patch implements two major things in the proposal.
The most important part of this patch is not regressing the existing
extremely optimized JSON implementation for the fast path. Thus this
patch adds template parameter to LiteralParser and disable all the
slight changes when we are using the fast path.

1. JSON.rawJSON mechaism. Now new object type is integrated, and JSON.rawJSON can wrap JSON text
   with this object, which is a tool to inject raw JSON text into JSON.stringify. JSON.stringify
   uses held string from rawJSON-created object.
2. JSON.parse collects source information during parsing, and offer substring of text as a third parameter
   of JSON.parse reviver function.

[1]: https://github.com/tc39/proposal-json-parse-with-source

* JSTests/stress/json-parse-source-text-access.js: Added.
(shouldBe):
* JSTests/stress/json-raw-json-stringify.js: Added.
(shouldBe):
(shouldBe.JSON.stringify):
* JSTests/stress/json-raw-json.js: Added.
(shouldBe):
(shouldThrow):
(SyntaxError.JSON.Parse.error.Single.quotes.shouldThrow):
* Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:
* Source/JavaScriptCore/Sources.txt:
* Source/JavaScriptCore/heap/Heap.cpp:
* Source/JavaScriptCore/heap/Heap.h:
* Source/JavaScriptCore/runtime/CommonIdentifiers.h:
* Source/JavaScriptCore/runtime/JSGlobalObject.cpp:
(JSC::createJSONProperty):
(JSC::JSGlobalObject::init):
* Source/JavaScriptCore/runtime/JSGlobalObject.h:
(JSC::JSGlobalObject::rawJSONObjectStructure const):
* Source/JavaScriptCore/runtime/JSONObject.cpp:
(JSC::JSONObject::finishCreation):
(JSC::Stringifier::appendStringifiedValue):
(JSC::Walker::Walker):
(JSC::Walker::callReviver):
(JSC::Walker::walk):
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/JSONObject.h:
* Source/JavaScriptCore/runtime/JSRawJSONObject.cpp: Added.
(JSC::JSRawJSONObject::JSRawJSONObject):
(JSC::JSRawJSONObject::finishCreation):
(JSC::JSRawJSONObject::createStructure):
(JSC::JSRawJSONObject::rawJSON):
* Source/JavaScriptCore/runtime/JSRawJSONObject.h: Copied from Source/JavaScriptCore/runtime/JSONObject.h.
* Source/JavaScriptCore/runtime/LiteralParser.cpp:
(JSC::LiteralParser<CharType>::tryJSONPParse):
(JSC::LiteralParser<CharType>::parse):
* Source/JavaScriptCore/runtime/LiteralParser.h:
(JSC::JSONRanges::JSONRanges):
(JSC::JSONRanges::root const):
(JSC::LiteralParser::tryLiteralParse):
(JSC::LiteralParser::tryLiteralParsePrimitiveValue):
(JSC::LiteralParser::Lexer::Lexer):
(JSC::LiteralParser::Lexer::start const):
* Source/JavaScriptCore/runtime/OptionsList.h:

Canonical link: https://commits.webkit.org/288223@main

41f8aef

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ❌ 🧪 mac-intel-wk2 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ❌ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@Constellation Constellation requested a review from a team as a code owner December 2, 2022 01:01
@Constellation Constellation self-assigned this Dec 2, 2022
@Constellation Constellation added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label Dec 2, 2022
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 2, 2022
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Dec 2, 2022
@Constellation Constellation force-pushed the eng/Implement-JSON-parse-source-text-access-proposal branch from a970078 to 45fdc27 Compare December 2, 2022 01:12
@Constellation
Copy link
Member Author

Large performance regression in SP2 and JS2. I should try to make the default path fast and succinct by adding this mode to the template parameter of LiteralParser.

@Constellation Constellation force-pushed the eng/Implement-JSON-parse-source-text-access-proposal branch from 45fdc27 to 8d3885f Compare December 2, 2022 19:32
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 2, 2022
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Dec 2, 2022
@Constellation Constellation marked this pull request as draft December 2, 2022 21:50
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 3, 2022
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Dec 14, 2022
@ljharb
Copy link
Contributor

ljharb commented Dec 27, 2023

@Constellation test262 tests are merged; any update?

@Constellation Constellation force-pushed the eng/Implement-JSON-parse-source-text-access-proposal branch from 8d3885f to a616664 Compare December 20, 2024 00:35
@Constellation Constellation force-pushed the eng/Implement-JSON-parse-source-text-access-proposal branch from a616664 to e92ce50 Compare December 20, 2024 00:37
@Constellation Constellation force-pushed the eng/Implement-JSON-parse-source-text-access-proposal branch from e92ce50 to b7c8969 Compare December 20, 2024 00:39
@Constellation Constellation marked this pull request as ready for review December 20, 2024 00:40
@Constellation Constellation requested a review from a team as a code owner December 20, 2024 00:40
@Constellation Constellation force-pushed the eng/Implement-JSON-parse-source-text-access-proposal branch from b7c8969 to 7c4843c Compare December 20, 2024 00:43
@Constellation Constellation force-pushed the eng/Implement-JSON-parse-source-text-access-proposal branch from 7c4843c to 6f3b774 Compare December 20, 2024 00:55
Copy link
Contributor

@kmiller68 kmiller68 left a comment

Choose a reason for hiding this comment

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

I’ll look more later but at least copyright update has a typo

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be 2024 not 2014

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@Constellation Constellation force-pushed the eng/Implement-JSON-parse-source-text-access-proposal branch from 6f3b774 to 7bc565f Compare December 20, 2024 02:13
@Constellation Constellation requested review from a team and kmiller68 December 20, 2024 02:13
@webkit-ews-buildbot
Copy link
Collaborator

Safer C++ Build #14724 (7bc565f)

❌ Found 3 new failures. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

⚠️ Found 1 fixed file! Please update expectations in Source/[Project]/SaferCPPExpectations by running the following command and update your pull request:

  • Tools/Scripts/update-safer-cpp-expectations -p JavaScriptCore --NoUncountedMemberChecker runtime/Watchdog.h

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 20, 2024
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Dec 20, 2024
@Constellation Constellation force-pushed the eng/Implement-JSON-parse-source-text-access-proposal branch from 7bc565f to beda5fb Compare December 20, 2024 18:17
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't see context.index and context.input. It seems they are not supported in this patch.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@hyjorc1 hyjorc1 left a comment

Choose a reason for hiding this comment

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

r=me

@webkit-ews-buildbot
Copy link
Collaborator

Safer C++ Build #14802 (beda5fb)

❌ Found 2 new failures. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 20, 2024
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Dec 21, 2024
@Constellation Constellation force-pushed the eng/Implement-JSON-parse-source-text-access-proposal branch from beda5fb to 81bb9ce Compare December 21, 2024 06:26
@Constellation Constellation force-pushed the eng/Implement-JSON-parse-source-text-access-proposal branch from 81bb9ce to 41f8aef Compare December 21, 2024 19:12
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 21, 2024
@Constellation Constellation added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged labels Dec 21, 2024
https://bugs.webkit.org/show_bug.cgi?id=248031
rdar://131579181

Reviewed by Yijia Huang.

This patch implements JSON.parse source text access proposal[1], which is now stage-3.
This patch implements two major things in the proposal.
The most important part of this patch is not regressing the existing
extremely optimized JSON implementation for the fast path. Thus this
patch adds template parameter to LiteralParser and disable all the
slight changes when we are using the fast path.

1. JSON.rawJSON mechaism. Now new object type is integrated, and JSON.rawJSON can wrap JSON text
   with this object, which is a tool to inject raw JSON text into JSON.stringify. JSON.stringify
   uses held string from rawJSON-created object.
2. JSON.parse collects source information during parsing, and offer substring of text as a third parameter
   of JSON.parse reviver function.

[1]: https://github.com/tc39/proposal-json-parse-with-source

* JSTests/stress/json-parse-source-text-access.js: Added.
(shouldBe):
* JSTests/stress/json-raw-json-stringify.js: Added.
(shouldBe):
(shouldBe.JSON.stringify):
* JSTests/stress/json-raw-json.js: Added.
(shouldBe):
(shouldThrow):
(SyntaxError.JSON.Parse.error.Single.quotes.shouldThrow):
* Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:
* Source/JavaScriptCore/Sources.txt:
* Source/JavaScriptCore/heap/Heap.cpp:
* Source/JavaScriptCore/heap/Heap.h:
* Source/JavaScriptCore/runtime/CommonIdentifiers.h:
* Source/JavaScriptCore/runtime/JSGlobalObject.cpp:
(JSC::createJSONProperty):
(JSC::JSGlobalObject::init):
* Source/JavaScriptCore/runtime/JSGlobalObject.h:
(JSC::JSGlobalObject::rawJSONObjectStructure const):
* Source/JavaScriptCore/runtime/JSONObject.cpp:
(JSC::JSONObject::finishCreation):
(JSC::Stringifier::appendStringifiedValue):
(JSC::Walker::Walker):
(JSC::Walker::callReviver):
(JSC::Walker::walk):
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/JSONObject.h:
* Source/JavaScriptCore/runtime/JSRawJSONObject.cpp: Added.
(JSC::JSRawJSONObject::JSRawJSONObject):
(JSC::JSRawJSONObject::finishCreation):
(JSC::JSRawJSONObject::createStructure):
(JSC::JSRawJSONObject::rawJSON):
* Source/JavaScriptCore/runtime/JSRawJSONObject.h: Copied from Source/JavaScriptCore/runtime/JSONObject.h.
* Source/JavaScriptCore/runtime/LiteralParser.cpp:
(JSC::LiteralParser<CharType>::tryJSONPParse):
(JSC::LiteralParser<CharType>::parse):
* Source/JavaScriptCore/runtime/LiteralParser.h:
(JSC::JSONRanges::JSONRanges):
(JSC::JSONRanges::root const):
(JSC::LiteralParser::tryLiteralParse):
(JSC::LiteralParser::tryLiteralParsePrimitiveValue):
(JSC::LiteralParser::Lexer::Lexer):
(JSC::LiteralParser::Lexer::start const):
* Source/JavaScriptCore/runtime/OptionsList.h:

Canonical link: https://commits.webkit.org/288223@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Implement-JSON-parse-source-text-access-proposal branch from 41f8aef to e2c5cd5 Compare December 21, 2024 21:43
@webkit-commit-queue
Copy link
Collaborator

Committed 288223@main (e2c5cd5): https://commits.webkit.org/288223@main

Reviewed commits have been landed. Closing PR #7057 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit e2c5cd5 into WebKit:main Dec 21, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Dec 21, 2024
@Constellation Constellation deleted the eng/Implement-JSON-parse-source-text-access-proposal branch December 22, 2024 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants