[webkit-changes] [186997] trunk/Source/JavaScriptCore

Sat, 18 Jul 2015 14:22:24 -0700

Title: [186997] trunk/Source/_javascript_Core
Revision
186997
Author
[email protected]
Date
2015-07-18 14:21:48 -0700 (Sat, 18 Jul 2015)

Log Message

Unreviewed, rolling out r186996.
https://bugs.webkit.org/show_bug.cgi?id=147070

Broke JSC tests (Requested by smfr on #webkit).

Reverted changeset:

"lexical scoping is broken with respect to "break" and
"continue""
https://bugs.webkit.org/show_bug.cgi?id=147063
http://trac.webkit.org/changeset/186996

Modified Paths

Removed Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (186996 => 186997)


--- trunk/Source/_javascript_Core/ChangeLog	2015-07-18 20:12:14 UTC (rev 186996)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-07-18 21:21:48 UTC (rev 186997)
@@ -1,3 +1,17 @@
+2015-07-18  Commit Queue  <[email protected]>
+
+        Unreviewed, rolling out r186996.
+        https://bugs.webkit.org/show_bug.cgi?id=147070
+
+        Broke JSC tests (Requested by smfr on #webkit).
+
+        Reverted changeset:
+
+        "lexical scoping is broken with respect to "break" and
+        "continue""
+        https://bugs.webkit.org/show_bug.cgi?id=147063
+        http://trac.webkit.org/changeset/186996
+
 2015-07-18  Saam barati  <[email protected]>
 
         lexical scoping is broken with respect to "break" and "continue"

Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (186996 => 186997)


--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2015-07-18 20:12:14 UTC (rev 186996)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2015-07-18 21:21:48 UTC (rev 186997)
@@ -522,8 +522,6 @@
         instructions().append(0);
     }
 
-    if (m_lexicalEnvironmentRegister)
-        pushScopedControlFlowContext();
     m_symbolTableStack.append(SymbolTableStackEntry{ Strong<SymbolTable>(*m_vm, m_symbolTable), m_lexicalEnvironmentRegister, false, symbolTableConstantIndex });
     m_TDZStack.append(std::make_pair(*parentScopeTDZVariables, false));
 }
@@ -629,7 +627,7 @@
         m_labelScopes.removeLast();
 
     // Allocate new label scope.
-    LabelScope scope(type, name, labelScopeDepth(), newLabel(), type == LabelScope::Loop ? newLabel() : PassRefPtr<Label>()); // Only loops have continue targets.
+    LabelScope scope(type, name, scopeDepth(), newLabel(), type == LabelScope::Loop ? newLabel() : PassRefPtr<Label>()); // Only loops have continue targets.
     m_labelScopes.append(scope);
     return LabelScopePtr(m_labelScopes, m_labelScopes.size() - 1);
 }
@@ -1212,7 +1210,7 @@
     // The format of this instruction is: op_profile_type regToProfile, TypeLocation*, flag, identifier?, resolveType?
     emitOpcode(op_profile_type);
     instructions().append(registerToProfile->index());
-    instructions().append(localScopeDepth());
+    instructions().append(currentScopeDepth());
     instructions().append(flag);
     instructions().append(identifier ? addConstant(*identifier) : 0);
     instructions().append(resolveType());
@@ -1320,8 +1318,6 @@
         instructions().append(addConstantValue(jsTDZValue())->index());
 
         emitMove(scopeRegister(), newScope);
-
-        pushScopedControlFlowContext();
     }
 
     m_symbolTableStack.append(SymbolTableStackEntry{ symbolTable, newScope, false, symbolTableConstantIndex });
@@ -1369,8 +1365,8 @@
 
     if (hasCapturedVariables) {
         RELEASE_ASSERT(stackEntry.m_scope);
-        emitPopScope(scopeRegister(), stackEntry.m_scope);
-        popScopedControlFlowContext();
+        RefPtr<RegisterID> parentScope = emitGetParentScope(newTemporary(), scopeRegister());
+        emitMove(scopeRegister(), parentScope.get());
         stackEntry.m_scope->deref();
     }
 
@@ -1427,7 +1423,7 @@
     // as the previous scope because the loop body is compiled under
     // the assumption that the scope's register index is constant even
     // though the value in that register will change on each loop iteration.
-    RefPtr<RegisterID> parentScope = emitGetParentScope(newTemporary(), loopScope);
+    RefPtr<RegisterID> parentScope = emitGetParentScope(newTemporary(), scopeRegister());
     emitMove(scopeRegister(), parentScope.get());
 
     emitOpcode(op_create_lexical_environment);
@@ -1576,11 +1572,8 @@
 // will start with this ResolveType and compute the least upper bound including intercepting scopes.
 ResolveType BytecodeGenerator::resolveType()
 {
-    for (unsigned i = m_symbolTableStack.size(); i--; ) {
-        if (m_symbolTableStack[i].m_isWithOrCatch)
-            return Dynamic;
-    }
-
+    if (m_localScopeDepth)
+        return Dynamic;
     if (m_symbolTable && m_symbolTable->usesNonStrictEval())
         return GlobalPropertyWithVarInjectionChecks;
     return GlobalProperty;
@@ -1639,7 +1632,7 @@
         instructions().append(scopeRegister()->index());
         instructions().append(addConstant(variable.ident()));
         instructions().append(resolveType());
-        instructions().append(localScopeDepth());
+        instructions().append(currentScopeDepth());
         instructions().append(0);
         return dst;
     }
@@ -1673,7 +1666,7 @@
         instructions().append(scope->index());
         instructions().append(addConstant(variable.ident()));
         instructions().append(ResolveModeAndType(resolveMode, variable.offset().isScope() ? LocalClosureVar : resolveType()).operand());
-        instructions().append(localScopeDepth());
+        instructions().append(currentScopeDepth());
         instructions().append(variable.offset().isScope() ? variable.offset().scopeOffset().offset() : 0);
         instructions().append(profile);
         return dst;
@@ -1713,7 +1706,7 @@
         } else {
             ASSERT(resolveType() != LocalClosureVar);
             instructions().append(ResolveModeAndType(resolveMode, resolveType()).operand());
-            instructions().append(localScopeDepth());
+            instructions().append(currentScopeDepth());
         }
         instructions().append(!!offset ? offset.offset() : 0);
         return value;
@@ -2481,7 +2474,10 @@
 
 RegisterID* BytecodeGenerator::emitPushWithScope(RegisterID* dst, RegisterID* scope)
 {
-    pushScopedControlFlowContext();
+    ControlFlowContext context;
+    context.isFinallyBlock = false;
+    m_scopeContextStack.append(context);
+    m_localScopeDepth++;
 
     RegisterID* result = emitUnaryOp(op_push_with_scope, dst, scope);
     m_symbolTableStack.append(SymbolTableStackEntry{ Strong<SymbolTable>(), nullptr, true, 0 });
@@ -2496,16 +2492,16 @@
     return dst;
 }
 
-void BytecodeGenerator::emitPopScope(RegisterID* dst, RegisterID* scope)
+void BytecodeGenerator::emitPopScope(RegisterID* srcDst)
 {
-    RefPtr<RegisterID> parentScope = emitGetParentScope(newTemporary(), scope);
-    emitMove(dst, parentScope.get());
-}
+    ASSERT(m_scopeContextStack.size());
+    ASSERT(!m_scopeContextStack.last().isFinallyBlock);
 
-void BytecodeGenerator::emitPopWithOrCatchScope(RegisterID* srcDst)
-{
-    emitPopScope(srcDst, srcDst);
-    popScopedControlFlowContext();
+    RefPtr<RegisterID> parentScope = emitGetParentScope(newTemporary(), srcDst);
+    emitMove(srcDst, parentScope.get());
+
+    m_scopeContextStack.removeLast();
+    m_localScopeDepth--;
     SymbolTableStackEntry stackEntry = m_symbolTableStack.takeLast();
     RELEASE_ASSERT(stackEntry.m_isWithOrCatch);
 }
@@ -2819,9 +2815,9 @@
 
 void BytecodeGenerator::emitPopScopes(RegisterID* scope, int targetScopeDepth)
 {
-    ASSERT(labelScopeDepth() - targetScopeDepth >= 0);
+    ASSERT(scopeDepth() - targetScopeDepth >= 0);
 
-    size_t scopeDelta = labelScopeDepth() - targetScopeDepth;
+    size_t scopeDelta = scopeDepth() - targetScopeDepth;
     ASSERT(scopeDelta <= m_scopeContextStack.size());
     if (!scopeDelta)
         return;
@@ -2880,10 +2876,16 @@
 
 int BytecodeGenerator::calculateTargetScopeDepthForExceptionHandler() const
 {
-    int depth = localScopeDepth();
+    int depth = m_localScopeDepth;
 
+    for (unsigned i = m_symbolTableStack.size(); i--; ) {
+        RegisterID* scope = m_symbolTableStack[i].m_scope;
+        if (scope)
+            depth++;
+    }
+
     // Currently, we're maintaing compatibility with how things are done and letting the exception handling
-    // code take into consideration the base activation of the function. There is no reason we shouldn't
+    // code take into consideration the base activation of the function. There is no reason we shouldn't 
     // be able to calculate the exact depth here and let the exception handler not worry if there is a base
     // activation or not.
     if (m_lexicalEnvironmentRegister)
@@ -2893,16 +2895,18 @@
     return depth;
 }
 
-int BytecodeGenerator::localScopeDepth() const
+int BytecodeGenerator::currentScopeDepth() const
 {
-    return m_localScopeDepth;
+    // This is the current number of JSScope descendents that would be allocated
+    // in this function/program if this code were running.
+    int depth = 0;
+    for (unsigned i = m_symbolTableStack.size(); i--; ) {
+        if (m_symbolTableStack[i].m_scope || m_symbolTableStack[i].m_isWithOrCatch)
+            depth++;
+    }
+    return depth;
 }
 
-int BytecodeGenerator::labelScopeDepth() const
-{ 
-    return localScopeDepth() + m_finallyDepth;
-}
-
 void BytecodeGenerator::emitThrowReferenceError(const String& message)
 {
     emitOpcode(op_throw_static_error);
@@ -2926,26 +2930,13 @@
     instructions().append(JSNameScope::FunctionNameScope);
 }
 
-void BytecodeGenerator::pushScopedControlFlowContext()
+void BytecodeGenerator::emitPushCatchScope(RegisterID* dst, const Identifier& property, RegisterID* value, unsigned attributes)
 {
     ControlFlowContext context;
     context.isFinallyBlock = false;
     m_scopeContextStack.append(context);
     m_localScopeDepth++;
-}
 
-void BytecodeGenerator::popScopedControlFlowContext()
-{
-    ASSERT(m_scopeContextStack.size());
-    ASSERT(!m_scopeContextStack.last().isFinallyBlock);
-    m_scopeContextStack.removeLast();
-    m_localScopeDepth--;
-}
-
-void BytecodeGenerator::emitPushCatchScope(RegisterID* dst, const Identifier& property, RegisterID* value, unsigned attributes)
-{
-    pushScopedControlFlowContext();
-
     emitOpcode(op_push_name_scope);
     instructions().append(dst->index());
     instructions().append(value->index());

Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (186996 => 186997)


--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h	2015-07-18 20:12:14 UTC (rev 186996)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h	2015-07-18 21:21:48 UTC (rev 186997)
@@ -585,12 +585,12 @@
 
         void emitGetScope();
         RegisterID* emitPushWithScope(RegisterID* dst, RegisterID* scope);
-        void emitPopScope(RegisterID* dst, RegisterID* scope);
-        void emitPopWithOrCatchScope(RegisterID* srcDst);
+        void emitPopScope(RegisterID* srcDst);
         RegisterID* emitGetParentScope(RegisterID* dst, RegisterID* scope);
 
         void emitDebugHook(DebugHookID, unsigned line, unsigned charOffset, unsigned lineStart);
 
+        int scopeDepth() { return m_localScopeDepth + m_finallyDepth; }
         bool hasFinaliser() { return m_finallyDepth != 0; }
 
         void pushFinallyContext(StatementNode* finallyBlock);
@@ -624,7 +624,6 @@
         void pushLexicalScope(VariableEnvironmentNode*, bool canOptimizeTDZChecks, RegisterID** constantSymbolTableResult = nullptr);
         void popLexicalScope(VariableEnvironmentNode*);
         void prepareLexicalScopeForNextForLoopIteration(VariableEnvironmentNode*, RegisterID* loopSymbolTable);
-        int labelScopeDepth() const;
 
     private:
         void reclaimFreeRegisters();
@@ -762,9 +761,7 @@
         const CodeType m_codeType;
 
         int calculateTargetScopeDepthForExceptionHandler() const;
-        int localScopeDepth() const;
-        void pushScopedControlFlowContext();
-        void popScopedControlFlowContext();
+        int currentScopeDepth() const;
 
         Vector<ControlFlowContext, 0, UnsafeVectorOverflow> m_scopeContextStack;
         Vector<SwitchInfo> m_switchContextStack;

Modified: trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (186996 => 186997)


--- trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2015-07-18 20:12:14 UTC (rev 186996)
+++ trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2015-07-18 21:21:48 UTC (rev 186997)
@@ -2627,7 +2627,7 @@
     LabelScopePtr scope = generator.continueTarget(m_ident);
     ASSERT(scope);
 
-    if (generator.labelScopeDepth() != scope->scopeDepth())
+    if (generator.scopeDepth() != scope->scopeDepth())
         return 0;
 
     return scope->continueTarget();
@@ -2656,7 +2656,7 @@
     LabelScopePtr scope = generator.breakTarget(m_ident);
     ASSERT(scope);
 
-    if (generator.labelScopeDepth() != scope->scopeDepth())
+    if (generator.scopeDepth() != scope->scopeDepth())
         return 0;
 
     return scope->breakTarget();
@@ -2690,7 +2690,7 @@
         generator.emitProfileType(returnRegister.get(), ProfileTypeBytecodeFunctionReturnStatement, nullptr);
         generator.emitTypeProfilerExpressionInfo(divotStart(), divotEnd());
     }
-    if (generator.labelScopeDepth()) {
+    if (generator.scopeDepth()) {
         returnRegister = generator.emitMove(generator.newTemporary(), returnRegister.get());
         generator.emitPopScopes(generator.scopeRegister(), 0);
     }
@@ -2714,7 +2714,7 @@
     generator.emitExpressionInfo(m_divot, m_divot - m_expressionLength, m_divot);
     generator.emitPushWithScope(generator.scopeRegister(), scope.get());
     generator.emitNode(dst, m_statement);
-    generator.emitPopWithOrCatchScope(generator.scopeRegister());
+    generator.emitPopScope(generator.scopeRegister());
 }
 
 // ------------------------------ CaseClauseNode --------------------------------
@@ -2967,7 +2967,7 @@
         generator.emitPushCatchScope(generator.scopeRegister(), m_thrownValueIdent, thrownValueRegister.get(), DontDelete);
         generator.emitProfileControlFlow(m_tryBlock->endOffset() + 1);
         generator.emitNode(dst, m_catchBlock);
-        generator.emitPopWithOrCatchScope(generator.scopeRegister());
+        generator.emitPopScope(generator.scopeRegister());
         generator.emitLabel(catchEndLabel.get());
     }

Deleted: trunk/Source/_javascript_Core/tests/stress/lexical-scoping-break-continue.js (186996 => 186997)


--- trunk/Source/_javascript_Core/tests/stress/lexical-scoping-break-continue.js	2015-07-18 20:12:14 UTC (rev 186996)
+++ trunk/Source/_javascript_Core/tests/stress/lexical-scoping-break-continue.js	2015-07-18 21:21:48 UTC (rev 186997)
@@ -1,216 +0,0 @@
-function assert(b) {
-    if (!b)
-        throw new Error("bad assertion");
-}
-noInline(assert);
-
-;(function() {
-    function test1() {
-        let x = 20;
-        function foo() {
-            label: {
-                let y = 21;
-                let capY = function () { return y; }
-                assert(x === 20);
-                break label;
-            }
-            assert(x === 20);
-        }
-        foo();
-    }
-
-    function test2() {
-        let x = 20;
-        function capX() { return x; }
-        function foo() {
-            label1: {
-                label2: {
-                    let y = 21;
-                    let capY = function () { return y; }
-                    break label2;
-                }
-                assert(x === 20);
-            }
-            assert(x === 20);
-
-            label1: {
-                label2: {
-                    let y = 21;
-                    let capY = function () { return y; }
-                    assert(x === 20);
-                    assert(y === 21);
-                    break label1;
-                }
-            }
-            assert(x === 20);
-
-            label1: {
-                let y = 21;
-                let capY = function () { return y; }
-                label2: {
-                    let y = 21;
-                    let capY = function () { return y; }
-                    assert(x === 20);
-                    assert(y === 21);
-                    break label1;
-                }
-            }
-            assert(x === 20);
-        }
-        foo()
-    }
-
-    function test3() {
-        let x = 20;
-        function capX() { return x; }
-        function foo() {
-            loop1: for (var i = 0; i++ < 1000; ) {
-                //assert(x === 20);
-                loop2: for (var j = 0; j++ < 1000; ) {
-                    let y = 21;
-                    let capY = function() { return y; }
-                    assert(x === 20);
-                    assert(y === 21);
-                    continue loop1;
-                    //break loop1;
-                }
-            }
-            assert(x === 20);
-        }
-        foo()
-    }
-
-    function test4() {
-        let x = 20;
-        function capX() { return x; }
-        function foo() {
-            loop1: for (var i = 0; i++ < 1000; ) {
-                loop2: for (var j = 0; j++ < 1000; ) {
-                    let y = 21;
-                    let capY = function() { return y; }
-                    assert(x === 20);
-                    assert(y === 21);
-                    break loop1;
-                }
-            }
-            assert(x === 20);
-        }
-        foo()
-    }
-
-    function test5() {
-        let x = 20;
-        function capX() { return x; }
-        function foo() {
-            loop1: for (var i = 0; i++ < 1000; ) {
-                let y = 21;
-                let capY = function() { return y; }
-                loop2: for (var j = 0; j++ < 1000; ) {
-                    let y = 21;
-                    let capY = function() { return y; }
-                    assert(x === 20);
-                    assert(y === 21);
-                    break loop1;
-                }
-            }
-            assert(x === 20);
-        }
-        foo()
-    }
-
-    function test6() {
-        let x = 20;
-        function capX() { return x; }
-        function foo() {
-            loop1: for (var i = 0; i++ < 1000; ) {
-                assert(x === 20);
-                let y = 21;
-                let capY = function() { return y; }
-                loop2: for (var j = 0; j++ < 1000; ) {
-                    let y = 21;
-                    let capY = function() { return y; }
-                    assert(x === 20);
-                    assert(y === 21);
-                    try {
-                        throw new Error();            
-                    } catch(e) {
-                    } finally {
-                        assert(x === 20);
-                        continue loop1;    
-                    }
-                }
-            }
-            assert(x === 20);
-        }
-        foo()
-    }
-
-    function test7() {
-        let x = 20;
-        function capX() { return x; }
-        function foo() {
-            loop1: for (var i = 0; i++ < 1000; ) {
-                assert(x === 20);
-                let y = 21;
-                let capY = function() { return y; }
-                loop2: for (var j = 0; j++ < 1000; ) {
-                    let y = 21;
-                    let capY = function() { return y; }
-                    assert(x === 20);
-                    assert(y === 21);
-                    try {
-                        throw new Error();            
-                    } catch(e) {
-                        continue loop1;
-                    } finally {
-                        let x = 40;
-                        let capX = function() { return x; }
-                        assert(x === 40);
-                    }
-                }
-            }
-            assert(x === 20);
-        }
-        foo()
-    }
-
-    function test8() {
-        let x = 20;
-        function capX() { return x; }
-        function foo() {
-            loop1: for (var i = 0; i++ < 1000; ) {
-                assert(x === 20);
-                let y = 21;
-                let capY = function() { return y; }
-                loop2: for (var j = 0; j++ < 1000; ) {
-                    let y = 21;
-                    let capY = function() { return y; }
-                    assert(x === 20);
-                    assert(y === 21);
-                    try {
-                        throw new Error();            
-                    } catch(e) {
-                        break loop1;
-                    } finally {
-                        let x = 40;
-                        let capX = function() { return x; }
-                        assert(x === 40);
-                    }
-                }
-            }
-            assert(x === 20);
-        }
-        foo()
-    }
-
-    for (var i = 0; i < 1000; i++) {
-        test1();
-        test2();
-        test3();
-        test4();
-        test5();
-        test6();
-        test7();
-        test8();
-    }
-})();

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to