Title: [291577] trunk
Revision
291577
Author
ysuz...@apple.com
Date
2022-03-21 12:57:19 -0700 (Mon, 21 Mar 2022)

Log Message

[JSC] ReferenceError when using extra parens in class fields
https://bugs.webkit.org/show_bug.cgi?id=236843

Reviewed by Saam Barati.

JSTests:

* stress/class-field-initializer-should-have-variable-scope.js: Added.
(shouldBe):
(test1.const.a.x.B):
(test1):
(test2.const.a.x.B):
(test2):
(test3.B.prototype.b):
(test3.B):
(test3):

Source/_javascript_Core:

class field initializer should create its own used-variables set
to capture used variables separately from the other variables since
it becomes independent CodeBlock internally later. The current code
was wrong since,

    1. Incorrectly using the current set of class-scope.
    2. Incorrectly marking only the last set while parseAssignmentExpression can create a new set inside it.

* parser/Parser.cpp:
(JSC::Parser<LexerType>::parseClass):
* parser/Parser.h:
(JSC::Scope::markLastUsedVariablesSetAsCaptured):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (291576 => 291577)


--- trunk/JSTests/ChangeLog	2022-03-21 19:42:26 UTC (rev 291576)
+++ trunk/JSTests/ChangeLog	2022-03-21 19:57:19 UTC (rev 291577)
@@ -1,3 +1,20 @@
+2022-03-21  Yusuke Suzuki  <ysuz...@apple.com>
+
+        [JSC] ReferenceError when using extra parens in class fields
+        https://bugs.webkit.org/show_bug.cgi?id=236843
+
+        Reviewed by Saam Barati.
+
+        * stress/class-field-initializer-should-have-variable-scope.js: Added.
+        (shouldBe):
+        (test1.const.a.x.B):
+        (test1):
+        (test2.const.a.x.B):
+        (test2):
+        (test3.B.prototype.b):
+        (test3.B):
+        (test3):
+
 2022-03-08  Mark Lam  <mark....@apple.com>
 
         Remove invalid ASSERT in LocaleIDBuilder::overrideLanguageScriptRegion().

Added: trunk/JSTests/stress/class-field-initializer-should-have-variable-scope.js (0 => 291577)


--- trunk/JSTests/stress/class-field-initializer-should-have-variable-scope.js	                        (rev 0)
+++ trunk/JSTests/stress/class-field-initializer-should-have-variable-scope.js	2022-03-21 19:57:19 UTC (rev 291577)
@@ -0,0 +1,37 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+(function test1() {
+    const a = (x) => x
+
+    class B {
+        c = a('OK');
+    }
+
+    shouldBe(new B().c, "OK");
+})();
+
+(function test2() {
+    const a = (x) => x
+
+    class B {
+        c = a(('OK'));
+    }
+
+    shouldBe(new B().c, "OK");
+})();
+
+(function test3() {
+    const a = (x) => x;
+    const b = 'ok';
+
+    class B {
+        [b]() { return 42; }
+        c = a('OK');
+    }
+
+    shouldBe(new B().c, "OK");
+    shouldBe(new B().ok(), 42);
+})();

Modified: trunk/Source/_javascript_Core/ChangeLog (291576 => 291577)


--- trunk/Source/_javascript_Core/ChangeLog	2022-03-21 19:42:26 UTC (rev 291576)
+++ trunk/Source/_javascript_Core/ChangeLog	2022-03-21 19:57:19 UTC (rev 291577)
@@ -1,3 +1,23 @@
+2022-03-21  Yusuke Suzuki  <ysuz...@apple.com>
+
+        [JSC] ReferenceError when using extra parens in class fields
+        https://bugs.webkit.org/show_bug.cgi?id=236843
+
+        Reviewed by Saam Barati.
+
+        class field initializer should create its own used-variables set
+        to capture used variables separately from the other variables since
+        it becomes independent CodeBlock internally later. The current code
+        was wrong since,
+
+            1. Incorrectly using the current set of class-scope.
+            2. Incorrectly marking only the last set while parseAssignmentExpression can create a new set inside it.
+
+        * parser/Parser.cpp:
+        (JSC::Parser<LexerType>::parseClass):
+        * parser/Parser.h:
+        (JSC::Scope::markLastUsedVariablesSetAsCaptured):
+
 2022-03-21  Jonathan Bedard  <jbed...@apple.com>
 
         Unreviewed, reverting r291558.

Modified: trunk/Source/_javascript_Core/parser/Parser.cpp (291576 => 291577)


--- trunk/Source/_javascript_Core/parser/Parser.cpp	2022-03-21 19:42:26 UTC (rev 291576)
+++ trunk/Source/_javascript_Core/parser/Parser.cpp	2022-03-21 19:57:19 UTC (rev 291577)
@@ -3110,12 +3110,14 @@
 
             TreeExpression initializer = 0;
             if (consume(EQUAL)) {
+                size_t usedVariablesSize = currentScope()->currentUsedVariablesSize();
+                currentScope()->pushUsedVariableSet();
                 SetForScope overrideParsingClassFieldInitializer(m_parserState.isParsingClassFieldInitializer, true);
                 classScope->setExpectedSuperBinding(SuperBinding::Needed);
                 initializer = parseAssignmentExpression(context);
                 classScope->setExpectedSuperBinding(SuperBinding::NotNeeded);
                 failIfFalse(initializer, "Cannot parse initializer for class field");
-                classScope->markLastUsedVariablesSetAsCaptured();
+                classScope->markLastUsedVariablesSetAsCaptured(usedVariablesSize);
             }
             failIfFalse(autoSemiColon(), "Expected a ';' following a class field");
             auto inferName = initializer ? InferName::Allowed : InferName::Disallowed;

Modified: trunk/Source/_javascript_Core/parser/Parser.h (291576 => 291577)


--- trunk/Source/_javascript_Core/parser/Parser.h	2022-03-21 19:42:26 UTC (rev 291576)
+++ trunk/Source/_javascript_Core/parser/Parser.h	2022-03-21 19:57:19 UTC (rev 291577)
@@ -663,10 +663,12 @@
         m_closedVariableCandidates.add(impl);
     }
 
-    void markLastUsedVariablesSetAsCaptured()
+    void markLastUsedVariablesSetAsCaptured(unsigned from)
     {
-        for (UniquedStringImpl* impl : m_usedVariables.last())
-            m_closedVariableCandidates.add(impl);
+        for (unsigned index = from; index < m_usedVariables.size(); ++index) {
+            for (UniquedStringImpl* impl : m_usedVariables[index])
+                m_closedVariableCandidates.add(impl);
+        }
     }
     
     void collectFreeVariables(Scope* nestedScope, bool shouldTrackClosedVariables)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to