We allow assignments to const variables when in a for-in/for-of loop
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Aug 2016 23:57:03 +0000 (23:57 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Aug 2016 23:57:03 +0000 (23:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=156673

Patch by JF Bastien <jfbastien@apple.com> on 2016-08-17
Reviewed by Filip Pizlo.

JSTests:

* stress/for-in-of-const.js: Added.
(expect_nothrow):
(expect_throw):
(capture):

Source/JavaScriptCore:

for-in and for-of weren't checking whether iteration variable from
parent scopes were const. Assigning to such variables should
throw, but used not to.

* bytecompiler/NodesCodegen.cpp:
(JSC::ForInNode::emitLoopHeader):
(JSC::ForOfNode::emitBytecode):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@204586 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/for-in-of-const.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

index 7dceec6..8c7042d 100644 (file)
@@ -1,3 +1,15 @@
+2016-08-17  JF Bastien  <jfbastien@apple.com>
+
+        We allow assignments to const variables when in a for-in/for-of loop
+        https://bugs.webkit.org/show_bug.cgi?id=156673
+
+        Reviewed by Filip Pizlo.
+
+        * stress/for-in-of-const.js: Added.
+        (expect_nothrow):
+        (expect_throw):
+        (capture):
+
 2016-08-17  Mark Lam  <mark.lam@apple.com>
 
         Remove an invalid assertion in the DFG backend's GetById emitter.
diff --git a/JSTests/stress/for-in-of-const.js b/JSTests/stress/for-in-of-const.js
new file mode 100644 (file)
index 0000000..cead58f
--- /dev/null
@@ -0,0 +1,49 @@
+// Check that const variables can't be assigned to from for-in/for-of.
+// https://bugs.webkit.org/show_bug.cgi?id=156673
+
+expect_nothrow = function(why, f) {
+    f();
+}
+
+expect_throw = function(why, f) {
+    threw = false;
+    try {
+        f();
+    } catch (e) {
+    if (e.toString() != "TypeError: Attempted to assign to readonly property.")
+        throw Error("expected a TypeError, got " + e.toString());
+        threw = true;
+    }
+    if (!threw)
+        throw Error("expected to throw");
+}
+
+// for-in
+
+expect_nothrow("regular for-in",              function() { for (x in [1,2,3]) x; });
+expect_nothrow("var for-in",                  function() { for (var x in [1,2,3]) x; });
+expect_nothrow("let for-in",                  function() { for (let x in [1,2,3]) x; });
+expect_nothrow("for-in with const variable",  function() { for (const x in [1,2,3]) x; });
+expect_nothrow("for-in which never iterates", function() { const x = 20; for (x in []) x; });
+
+expect_throw("for-in on const from func's scope", function() { const x = 20; for (x in [1,2,3]) x; });
+expect_throw("same, with intervening capture",    function() { const x = 20; capture = function() { x; }; for (x in [1,2,3]) x; });
+expect_throw("same, iterating in capture",        function() { const x = 20; capture = function() { for (x in [1,2,3]) x; }; capture(); });
+
+// for-of
+
+expect_nothrow("regular for-of",              function() { for (x of [1,2,3]) x; });
+expect_nothrow("var for-of",                  function() { for (var x of [1,2,3]) x; });
+expect_nothrow("let for-of",                  function() { for (let x of [1,2,3]) x; });
+expect_nothrow("for-of with const variable",  function() { for (const x of [1,2,3]) x; });
+expect_nothrow("for-of which never iterates", function() { const x = 20; for (x of []) x; });
+
+expect_throw("for-of on const from func's scope", function() { const x = 20; for (x of [1,2,3]) x; });
+expect_throw("same, with intervening capture",    function() { const x = 20; capture = function() { x; }; for (x of [1,2,3]) x; });
+expect_throw("same, iterating in capture",        function() { const x = 20; capture = function() { for (x of [1,2,3]) x; }; capture(); });
+
+expect_throw("bad destructuring",          function() { let arr = [{x:20}]; const x = 50; for ({x} of arr) x; });
+expect_nothrow("good destructuring",       function() { let arr = [{x:20}]; const x = 50; for ({x : foo} of arr) x; });
+expect_nothrow("const good destructuring", function() { let arr = [{x:20}]; const x = 50; for (const {x} of arr) x; });
+expect_nothrow("let good destructuring",   function() { let arr = [{x:20}]; const x = 50; for (let {x} of arr) x; });
+// Note: `var {x}` would shadow `const x` and therefore fail.
index 18556ee..a97f5ff 100644 (file)
@@ -1,3 +1,18 @@
+2016-08-17  JF Bastien  <jfbastien@apple.com>
+
+        We allow assignments to const variables when in a for-in/for-of loop
+        https://bugs.webkit.org/show_bug.cgi?id=156673
+
+        Reviewed by Filip Pizlo.
+
+        for-in and for-of weren't checking whether iteration variable from
+        parent scopes were const. Assigning to such variables should
+        throw, but used not to.
+
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::ForInNode::emitLoopHeader):
+        (JSC::ForOfNode::emitBytecode):
+
 2016-08-17  Geoffrey Garen  <ggaren@apple.com>
 
         Fixed a potential bug in MarkedArgumentBuffer.
index c5a217a..4b2d5ba 100644 (file)
@@ -2551,11 +2551,15 @@ void ForInNode::emitLoopHeader(BytecodeGenerator& generator, RegisterID* propert
     if (m_lexpr->isResolveNode()) {
         const Identifier& ident = static_cast<ResolveNode*>(m_lexpr)->identifier();
         Variable var = generator.variable(ident);
-        if (RegisterID* local = var.local())
+        if (RegisterID* local = var.local()) {
+            if (var.isReadOnly())
+                generator.emitReadOnlyExceptionIfNeeded(var);
             generator.emitMove(local, propertyName);
-        else {
+        else {
             if (generator.isStrictMode())
                 generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
+            if (var.isReadOnly())
+                generator.emitReadOnlyExceptionIfNeeded(var);
             RegisterID* scope = generator.emitResolveScope(nullptr, var);
             generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
             generator.emitPutToScope(scope, var, propertyName, generator.isStrictMode() ? ThrowIfNotFound : DoNotThrowIfNotFound, InitializationMode::NotInitialization);
@@ -2780,11 +2784,15 @@ void ForOfNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
         if (m_lexpr->isResolveNode()) {
             const Identifier& ident = static_cast<ResolveNode*>(m_lexpr)->identifier();
             Variable var = generator.variable(ident);
-            if (RegisterID* local = var.local())
+            if (RegisterID* local = var.local()) {
+                if (var.isReadOnly())
+                    generator.emitReadOnlyExceptionIfNeeded(var);
                 generator.emitMove(local, value);
-            else {
+            else {
                 if (generator.isStrictMode())
                     generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
+                if (var.isReadOnly())
+                    generator.emitReadOnlyExceptionIfNeeded(var);
                 RegisterID* scope = generator.emitResolveScope(nullptr, var);
                 generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
                 generator.emitPutToScope(scope, var, value, generator.isStrictMode() ? ThrowIfNotFound : DoNotThrowIfNotFound, InitializationMode::NotInitialization);