LibJS: Strict mode is now handled by Functions and Programs, not Blocks

Since blocks can't be strict by themselves, it makes no sense for them
to store whether or not they are strict. Strict-ness is now stored in
the Program and FunctionNode ASTNodes. Fixes issue #3641

Libraries/LibJS/AST.cpp

@@ -89,6 +89,11 @@ Value ScopeNode::execute(Interpreter& interpreter, GlobalObject& global_object)
     return interpreter.execute_statement(global_object, *this);
 }
 
+Value Program::execute(Interpreter& interpreter, GlobalObject& global_object) const
+{
+    return interpreter.execute_statement(global_object, *this, {}, ScopeType::Block, m_is_strict_mode);
+}
+
 Value FunctionDeclaration::execute(Interpreter&, GlobalObject&) const
 {
     return js_undefined();
@@ -96,7 +101,7 @@ Value FunctionDeclaration::execute(Interpreter&, GlobalObject&) const
 
 Value FunctionExpression::execute(Interpreter& interpreter, GlobalObject& global_object) const
 {
-    return ScriptFunction::create(global_object, name(), body(), parameters(), function_length(), interpreter.current_environment(), m_is_arrow_function);
+    return ScriptFunction::create(global_object, name(), body(), parameters(), function_length(), interpreter.current_environment(), is_strict_mode(), m_is_arrow_function);
 }
 
 Value ExpressionStatement::execute(Interpreter& interpreter, GlobalObject& global_object) const

Libraries/LibJS/AST.h

@@ -129,8 +129,6 @@ public:
     void add_functions(NonnullRefPtrVector<FunctionDeclaration>);
     const NonnullRefPtrVector<VariableDeclaration>& variables() const { return m_variables; }
     const NonnullRefPtrVector<FunctionDeclaration>& functions() const { return m_functions; }
-    bool in_strict_mode() const { return m_strict_mode; }
-    void set_strict_mode() { m_strict_mode = true; }
 
 protected:
     ScopeNode() { }
@@ -140,14 +138,20 @@ private:
     NonnullRefPtrVector<Statement> m_children;
     NonnullRefPtrVector<VariableDeclaration> m_variables;
     NonnullRefPtrVector<FunctionDeclaration> m_functions;
-    bool m_strict_mode { false };
 };
 
 class Program final : public ScopeNode {
 public:
     Program() { }
 
+    virtual Value execute(Interpreter&, GlobalObject&) const override;
+
+    bool is_strict_mode() const { return m_is_strict_mode; }
+    void set_strict_mode() { m_is_strict_mode = true; }
+
 private:
+    bool m_is_strict_mode { false };
+
     virtual bool is_program() const override { return true; }
     virtual const char* class_name() const override { return "Program"; }
 };
@@ -180,14 +184,16 @@ public:
     const Statement& body() const { return *m_body; }
     const Vector<Parameter>& parameters() const { return m_parameters; };
     i32 function_length() const { return m_function_length; }
+    bool is_strict_mode() const { return m_is_strict_mode; }
 
 protected:
-    FunctionNode(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables)
+    FunctionNode(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables, bool is_strict_mode)
         : m_name(name)
         , m_body(move(body))
         , m_parameters(move(parameters))
         , m_variables(move(variables))
         , m_function_length(function_length)
+        , m_is_strict_mode(is_strict_mode)
     {
     }
 
@@ -201,6 +207,7 @@ private:
     const Vector<Parameter> m_parameters;
     NonnullRefPtrVector<VariableDeclaration> m_variables;
     const i32 m_function_length;
+    bool m_is_strict_mode;
 };
 
 class FunctionDeclaration final
@@ -209,8 +216,8 @@ class FunctionDeclaration final
 public:
     static bool must_have_name() { return true; }
 
-    FunctionDeclaration(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables)
-        : FunctionNode(name, move(body), move(parameters), function_length, move(variables))
+    FunctionDeclaration(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables, bool is_strict_mode = false)
+        : FunctionNode(name, move(body), move(parameters), function_length, move(variables), is_strict_mode)
     {
     }
 
@@ -227,8 +234,8 @@ class FunctionExpression final
 public:
     static bool must_have_name() { return false; }
 
-    FunctionExpression(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables, bool is_arrow_function = false)
-        : FunctionNode(name, move(body), move(parameters), function_length, move(variables))
+    FunctionExpression(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables, bool is_strict_mode, bool is_arrow_function = false)
+        : FunctionNode(name, move(body), move(parameters), function_length, move(variables), is_strict_mode)
         , m_is_arrow_function(is_arrow_function)
     {
     }

Libraries/LibJS/Interpreter.cpp

@@ -91,15 +91,15 @@ const GlobalObject& Interpreter::global_object() const
     return static_cast<const GlobalObject&>(*m_global_object.cell());
 }
 
-void Interpreter::enter_scope(const ScopeNode& scope_node, ArgumentVector arguments, ScopeType scope_type, GlobalObject& global_object)
+void Interpreter::enter_scope(const ScopeNode& scope_node, ArgumentVector arguments, ScopeType scope_type, GlobalObject& global_object, bool is_strict)
 {
     for (auto& declaration : scope_node.functions()) {
-        auto* function = ScriptFunction::create(global_object, declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), current_environment());
+        auto* function = ScriptFunction::create(global_object, declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), current_environment(), is_strict);
         vm().set_variable(declaration.name(), function, global_object);
     }
 
     if (scope_type == ScopeType::Function) {
-        m_scope_stack.append({ scope_type, scope_node, false });
+        push_scope({ scope_type, scope_node, false, is_strict });
         return;
     }
 
@@ -130,7 +130,7 @@ void Interpreter::enter_scope(const ScopeNode& scope_node, ArgumentVector argume
         pushed_lexical_environment = true;
     }
 
-    m_scope_stack.append({ scope_type, scope_node, pushed_lexical_environment });
+    push_scope({ scope_type, scope_node, pushed_lexical_environment, is_strict });
 }
 
 void Interpreter::exit_scope(const ScopeNode& scope_node)
@@ -148,13 +148,20 @@ void Interpreter::exit_scope(const ScopeNode& scope_node)
         vm().unwind(ScopeType::None);
 }
 
-Value Interpreter::execute_statement(GlobalObject& global_object, const Statement& statement, ArgumentVector arguments, ScopeType scope_type)
+void Interpreter::push_scope(ScopeFrame frame)
+{
+    if (in_strict_mode())
+        frame.is_strict_mode = true;
+    m_scope_stack.append(move(frame));
+}
+
+Value Interpreter::execute_statement(GlobalObject& global_object, const Statement& statement, ArgumentVector arguments, ScopeType scope_type, bool is_strict)
 {
     if (!statement.is_scope_node())
         return statement.execute(*this, global_object);
 
     auto& block = static_cast<const ScopeNode&>(statement);
-    enter_scope(block, move(arguments), scope_type, global_object);
+    enter_scope(block, move(arguments), scope_type, global_object, is_strict);
 
     if (block.children().is_empty())
         vm().set_last_value({}, js_undefined());

Libraries/LibJS/Interpreter.h

@@ -88,10 +88,9 @@ public:
 
     bool in_strict_mode() const
     {
-        // FIXME: This implementation is bogus; strict mode is per-file or per-function, not per-block!
         if (m_scope_stack.is_empty())
-            return true;
-        return m_scope_stack.last().scope_node->in_strict_mode();
+            return false;
+        return m_scope_stack.last().is_strict_mode;
     }
 
     size_t argument_count() const { return vm().argument_count(); }
@@ -100,10 +99,10 @@ public:
     LexicalEnvironment* current_environment() { return vm().current_environment(); }
     const CallFrame& call_frame() { return vm().call_frame(); }
 
-    void enter_scope(const ScopeNode&, ArgumentVector, ScopeType, GlobalObject&);
+    void enter_scope(const ScopeNode&, ArgumentVector, ScopeType, GlobalObject&, bool is_strict = false);
     void exit_scope(const ScopeNode&);
 
-    Value execute_statement(GlobalObject&, const Statement&, ArgumentVector = {}, ScopeType = ScopeType::Block);
+    Value execute_statement(GlobalObject&, const Statement&, ArgumentVector = {}, ScopeType = ScopeType::Block, bool is_strict = false);
 
 private:
     explicit Interpreter(VM&);
@@ -113,11 +112,13 @@ private:
         return vm().call(function, this_value, move(arguments));
     }
 
+    void push_scope(ScopeFrame frame);
+
+    Vector<ScopeFrame> m_scope_stack;
+
     NonnullRefPtr<VM> m_vm;
 
     Handle<Object> m_global_object;
-
-    Vector<ScopeFrame> m_scope_stack;
 };
 
 template<>

Libraries/LibJS/Parser.cpp

@@ -390,10 +390,12 @@ RefPtr<FunctionExpression> Parser::try_parse_arrow_function_expression(bool expe
     if (function_length == -1)
         function_length = parameters.size();
 
-    auto function_body_result = [this]() -> RefPtr<BlockStatement> {
+    bool is_strict = false;
+
+    auto function_body_result = [&]() -> RefPtr<BlockStatement> {
         if (match(TokenType::CurlyOpen)) {
             // Parse a function body with statements
-            return parse_block_statement();
+            return parse_block_statement(is_strict);
         }
         if (match_expression()) {
             // Parse a function body which returns a single expression
@@ -414,7 +416,7 @@ RefPtr<FunctionExpression> Parser::try_parse_arrow_function_expression(bool expe
     if (!function_body_result.is_null()) {
         state_rollback_guard.disarm();
         auto body = function_body_result.release_nonnull();
-        return create_ast_node<FunctionExpression>("", move(body), move(parameters), function_length, m_parser_state.m_var_scopes.take_last(), true);
+        return create_ast_node<FunctionExpression>("", move(body), move(parameters), function_length, m_parser_state.m_var_scopes.take_last(), is_strict, true);
     }
 
     return nullptr;
@@ -562,9 +564,9 @@ NonnullRefPtr<ClassExpression> Parser::parse_class_expression(bool expect_class_
             constructor_body->append(create_ast_node<ExpressionStatement>(move(super_call)));
             constructor_body->add_variables(m_parser_state.m_var_scopes.last());
 
-            constructor = create_ast_node<FunctionExpression>(class_name, move(constructor_body), Vector { FunctionNode::Parameter { "args", nullptr, true } }, 0, NonnullRefPtrVector<VariableDeclaration>());
+            constructor = create_ast_node<FunctionExpression>(class_name, move(constructor_body), Vector { FunctionNode::Parameter { "args", nullptr, true } }, 0, NonnullRefPtrVector<VariableDeclaration>(), true);
         } else {
-            constructor = create_ast_node<FunctionExpression>(class_name, move(constructor_body), Vector<FunctionNode::Parameter> {}, 0, NonnullRefPtrVector<VariableDeclaration>());
+            constructor = create_ast_node<FunctionExpression>(class_name, move(constructor_body), Vector<FunctionNode::Parameter> {}, 0, NonnullRefPtrVector<VariableDeclaration>(), true);
         }
     }
 
@@ -1203,6 +1205,12 @@ NonnullRefPtr<ReturnStatement> Parser::parse_return_statement()
 }
 
 NonnullRefPtr<BlockStatement> Parser::parse_block_statement()
+{
+    bool dummy = false;
+    return parse_block_statement(dummy);
+}
+
+NonnullRefPtr<BlockStatement> Parser::parse_block_statement(bool& is_strict)
 {
     ScopePusher scope(*this, ScopePusher::Let);
     auto block = create_ast_node<BlockStatement>();
@@ -1212,7 +1220,7 @@ NonnullRefPtr<BlockStatement> Parser::parse_block_statement()
     bool initial_strict_mode_state = m_parser_state.m_strict_mode;
     if (initial_strict_mode_state) {
         m_parser_state.m_use_strict_directive = UseStrictDirectiveState::None;
-        block->set_strict_mode();
+        is_strict = true;
     } else {
         m_parser_state.m_use_strict_directive = UseStrictDirectiveState::Looking;
     }
@@ -1225,7 +1233,7 @@ NonnullRefPtr<BlockStatement> Parser::parse_block_statement()
 
             if (first && !initial_strict_mode_state) {
                 if (m_parser_state.m_use_strict_directive == UseStrictDirectiveState::Found) {
-                    block->set_strict_mode();
+                    is_strict = true;
                     m_parser_state.m_strict_mode = true;
                 }
                 m_parser_state.m_use_strict_directive = UseStrictDirectiveState::None;
@@ -1292,10 +1300,11 @@ NonnullRefPtr<FunctionNodeType> Parser::parse_function_node(bool check_for_funct
     if (function_length == -1)
         function_length = parameters.size();
 
-    auto body = parse_block_statement();
+    bool is_strict = false;
+    auto body = parse_block_statement(is_strict);
     body->add_variables(m_parser_state.m_var_scopes.last());
     body->add_functions(m_parser_state.m_function_scopes.last());
-    return create_ast_node<FunctionNodeType>(name, move(body), move(parameters), function_length, NonnullRefPtrVector<VariableDeclaration>());
+    return create_ast_node<FunctionNodeType>(name, move(body), move(parameters), function_length, NonnullRefPtrVector<VariableDeclaration>(), is_strict);
 }
 
 NonnullRefPtr<VariableDeclaration> Parser::parse_variable_declaration(bool with_semicolon)

Libraries/LibJS/Parser.h

@@ -50,6 +50,7 @@ public:
 
     NonnullRefPtr<Statement> parse_statement();
     NonnullRefPtr<BlockStatement> parse_block_statement();
+    NonnullRefPtr<BlockStatement> parse_block_statement(bool& is_strict);
     NonnullRefPtr<ReturnStatement> parse_return_statement();
     NonnullRefPtr<VariableDeclaration> parse_variable_declaration(bool with_semicolon = true);
     NonnullRefPtr<Statement> parse_for_statement();

Libraries/LibJS/Runtime/ScriptFunction.cpp

@@ -47,18 +47,19 @@ static ScriptFunction* typed_this(VM& vm, GlobalObject& global_object)
     return static_cast<ScriptFunction*>(this_object);
 }
 
-ScriptFunction* ScriptFunction::create(GlobalObject& global_object, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, bool is_arrow_function)
+ScriptFunction* ScriptFunction::create(GlobalObject& global_object, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, bool is_strict, bool is_arrow_function)
 {
-    return global_object.heap().allocate<ScriptFunction>(global_object, global_object, name, body, move(parameters), m_function_length, parent_environment, *global_object.function_prototype(), is_arrow_function);
+    return global_object.heap().allocate<ScriptFunction>(global_object, global_object, name, body, move(parameters), m_function_length, parent_environment, *global_object.function_prototype(), is_strict, is_arrow_function);
 }
 
-ScriptFunction::ScriptFunction(GlobalObject& global_object, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, Object& prototype, bool is_arrow_function)
+ScriptFunction::ScriptFunction(GlobalObject& global_object, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, Object& prototype, bool is_strict, bool is_arrow_function)
     : Function(prototype, is_arrow_function ? vm().this_value(global_object) : Value(), {})
     , m_name(name)
     , m_body(body)
     , m_parameters(move(parameters))
     , m_parent_environment(parent_environment)
     , m_function_length(m_function_length)
+    , m_is_strict(is_strict)
     , m_is_arrow_function(is_arrow_function)
 {
 }
@@ -140,7 +141,7 @@ Value ScriptFunction::call()
         arguments.append({ parameter.name, value });
         vm().current_environment()->set(global_object(), parameter.name, { value, DeclarationKind::Var });
     }
-    return interpreter->execute_statement(global_object(), m_body, arguments, ScopeType::Function);
+    return interpreter->execute_statement(global_object(), m_body, arguments, ScopeType::Function, m_is_strict);
 }
 
 Value ScriptFunction::construct(Function&)

Libraries/LibJS/Runtime/ScriptFunction.h

@@ -35,9 +35,9 @@ class ScriptFunction final : public Function {
     JS_OBJECT(ScriptFunction, Function);
 
 public:
-    static ScriptFunction* create(GlobalObject&, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, bool is_arrow_function = false);
+    static ScriptFunction* create(GlobalObject&, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, bool is_strict, bool is_arrow_function = false);
 
-    ScriptFunction(GlobalObject&, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, Object& prototype, bool is_arrow_function = false);
+    ScriptFunction(GlobalObject&, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, Object& prototype, bool is_strict, bool is_arrow_function = false);
     virtual void initialize(GlobalObject&) override;
     virtual ~ScriptFunction();
 
@@ -63,6 +63,7 @@ private:
     const Vector<FunctionNode::Parameter> m_parameters;
     LexicalEnvironment* m_parent_environment { nullptr };
     i32 m_function_length;
+    bool m_is_strict;
     bool m_is_arrow_function;
 };
 

Libraries/LibJS/Runtime/VM.h

@@ -50,6 +50,7 @@ struct ScopeFrame {
     ScopeType type;
     NonnullRefPtr<ScopeNode> scope_node;
     bool pushed_environment { false };
+    bool is_strict_mode { false };
 };
 
 struct CallFrame {

Libraries/LibJS/Tests/strict-mode-blocks.js

@@ -0,0 +1,22 @@
+test("Issue #3641, strict mode should be function- or program-level, not block-level", () => {
+    function func() {
+        expect(isStrictMode()).toBeFalse();
+
+        {
+            "use strict";
+            expect(isStrictMode()).toBeFalse();
+        }
+
+        if (true) {
+            "use strict";
+            expect(isStrictMode()).toBeFalse();
+        }
+
+        do {
+            "use strict";
+            expect(isStrictMode()).toBeFalse();
+        } while (false);
+    }
+
+    func();
+});