-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[JSC] Node's JSTokenLocation sometimes doesn't point to the start of an expression #22944
Conversation
EWS run on previous version of this PR (hash 3a30199) |
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.
I think you mean line, not like in the commit message.
r=me, but let's wait for #22880 to land first.
3a30199
to
b96cbf4
Compare
EWS run on previous version of this PR (hash b96cbf4) |
b96cbf4
to
76ad00b
Compare
EWS run on previous version of this PR (hash 76ad00b) |
76ad00b
to
fd3065e
Compare
EWS run on current version of this PR (hash fd3065e) |
β¦an expression https://bugs.webkit.org/show_bug.cgi?id=267728 <rdar://problem/121215280> Reviewed by Justin Michaud. Before this patch, if we tried to reparse at node->position(), we would fail with SyntaxError for object / array literals, async (arrow) functions, and member expressions with `new` / optional chaining, due to Node's JSTokenLocation not pointing to the start of an expression. This change fixes JSTokenLocation to be correct for above-mentioned cases, enabling us to redesign class field initializers reparsing, and also improves accuracy of expression divots by removing lastTokenEndPosition() usage from OPENBRACKET / DOT / BACKQUOTE cases of parseMemberExpression(). lastTokenEndPosition() makes sense in case of e.g. parseArguments() because after it exits, the current token is the next one after `)`, yet in the e.g. DOT case, lastTokenEndPosition() most likely points to previous identifier, which might be even on a different line. On top of that, this patch improves stack trace accuracy for `new new new C` expressions by storing position of each NEW token in a vector. Aligns stack traces (error locations) with V8 and SpiderMonkey, including for modified API test. * JSTests/stress/regress-267728.js: Added. * Source/JavaScriptCore/builtins/BuiltinExecutables.cpp: (JSC::BuiltinExecutables::createExecutable): * Source/JavaScriptCore/parser/ASTBuilder.h: (JSC::ASTBuilder::createNewExpr): * Source/JavaScriptCore/parser/Parser.cpp: (JSC::Parser<LexerType>::parseAssignmentExpression): (JSC::Parser<LexerType>::parseObjectLiteral): (JSC::Parser<LexerType>::parseArrayLiteral): (JSC::Parser<LexerType>::parseAsyncFunctionExpression): (JSC::Parser<LexerType>::parsePrimaryExpression): (JSC::Parser<LexerType>::parseMemberExpression): (JSC::Parser<LexerType>::parseArrowFunctionExpression): * Source/JavaScriptCore/parser/Parser.h: * Source/JavaScriptCore/parser/SyntaxChecker.h: (JSC::SyntaxChecker::createNewExpr): * Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewConfiguration.mm: Canonical link: https://commits.webkit.org/273255@main
fd3065e
to
e0132b8
Compare
Committed 273255@main (e0132b8): https://commits.webkit.org/273255@main Reviewed commits have been landed. Closing PR #22944 and removing active labels. |
e0132b8
fd3065e
π§ͺ wpe-wk2π π§ͺ jscπ§ͺ gtk-wk2π§ͺ jsc-armv7-tests