LibJS: Allow multiple labels on the same statement

Since there are only a number of statements where labels can actually be
used we now also only store labels when necessary.
Also now tracks the first continue usage of a label since this might not
be valid but that can only be determined after we have parsed the
statement.
Also ensures the correct error does not get wiped by load_state.

Userland/Libraries/LibJS/AST.cpp

@@ -2326,6 +2326,11 @@ void ThrowStatement::dump(int indent) const
     argument().dump(indent + 1);
 }
 
+void TryStatement::add_label(FlyString string)
+{
+    m_block->add_label(move(string));
+}
+
 Value TryStatement::execute(Interpreter& interpreter, GlobalObject& global_object) const
 {
     InterpreterNodeScope node_scope { interpreter, *this };

Userland/Libraries/LibJS/AST.h

@@ -80,13 +80,22 @@ public:
         : ASTNode(source_range)
     {
     }
+};
 
-    HashTable<FlyString> const& labels() const { return m_labels; }
-    void add_label(FlyString label) { m_labels.set(move(label)); }
-    bool has_label(FlyString const& label) const { return m_labels.contains(label); }
+class LabelableStatement : public Statement {
+public:
+    using Statement::Statement;
+
+    Vector<FlyString> const& labels() const { return m_labels; }
+    virtual void add_label(FlyString string) { m_labels.append(move(string)); }
 
 protected:
-    HashTable<FlyString> m_labels;
+    Vector<FlyString> m_labels;
+};
+
+class IterationStatement : public LabelableStatement {
+public:
+    using LabelableStatement::LabelableStatement;
 };
 
 class EmptyStatement final : public Statement {
@@ -128,7 +137,7 @@ private:
     NonnullRefPtr<Expression> m_expression;
 };
 
-class ScopeNode : public Statement {
+class ScopeNode : public LabelableStatement {
 public:
     template<typename T, typename... Args>
     T& append(SourceRange range, Args&&... args)
@@ -156,7 +165,7 @@ public:
 
 protected:
     explicit ScopeNode(SourceRange source_range)
-        : Statement(source_range)
+        : LabelableStatement(source_range)
     {
     }
 
@@ -514,10 +523,10 @@ private:
     RefPtr<Statement> m_alternate;
 };
 
-class WhileStatement final : public Statement {
+class WhileStatement final : public IterationStatement {
 public:
     WhileStatement(SourceRange source_range, NonnullRefPtr<Expression> test, NonnullRefPtr<Statement> body)
-        : Statement(source_range)
+        : IterationStatement(source_range)
         , m_test(move(test))
         , m_body(move(body))
     {
@@ -535,10 +544,10 @@ private:
     NonnullRefPtr<Statement> m_body;
 };
 
-class DoWhileStatement final : public Statement {
+class DoWhileStatement final : public IterationStatement {
 public:
     DoWhileStatement(SourceRange source_range, NonnullRefPtr<Expression> test, NonnullRefPtr<Statement> body)
-        : Statement(source_range)
+        : IterationStatement(source_range)
         , m_test(move(test))
         , m_body(move(body))
     {
@@ -576,10 +585,10 @@ private:
     NonnullRefPtr<Statement> m_body;
 };
 
-class ForStatement final : public Statement {
+class ForStatement final : public IterationStatement {
 public:
     ForStatement(SourceRange source_range, RefPtr<ASTNode> init, RefPtr<Expression> test, RefPtr<Expression> update, NonnullRefPtr<Statement> body)
-        : Statement(source_range)
+        : IterationStatement(source_range)
         , m_init(move(init))
         , m_test(move(test))
         , m_update(move(update))
@@ -603,10 +612,10 @@ private:
     NonnullRefPtr<Statement> m_body;
 };
 
-class ForInStatement final : public Statement {
+class ForInStatement final : public IterationStatement {
 public:
     ForInStatement(SourceRange source_range, NonnullRefPtr<ASTNode> lhs, NonnullRefPtr<Expression> rhs, NonnullRefPtr<Statement> body)
-        : Statement(source_range)
+        : IterationStatement(source_range)
         , m_lhs(move(lhs))
         , m_rhs(move(rhs))
         , m_body(move(body))
@@ -626,10 +635,10 @@ private:
     NonnullRefPtr<Statement> m_body;
 };
 
-class ForOfStatement final : public Statement {
+class ForOfStatement final : public IterationStatement {
 public:
     ForOfStatement(SourceRange source_range, NonnullRefPtr<ASTNode> lhs, NonnullRefPtr<Expression> rhs, NonnullRefPtr<Statement> body)
-        : Statement(source_range)
+        : IterationStatement(source_range)
         , m_lhs(move(lhs))
         , m_rhs(move(rhs))
         , m_body(move(body))
@@ -1498,10 +1507,10 @@ private:
     NonnullRefPtr<BlockStatement> m_body;
 };
 
-class TryStatement final : public Statement {
+class TryStatement final : public LabelableStatement {
 public:
     TryStatement(SourceRange source_range, NonnullRefPtr<BlockStatement> block, RefPtr<CatchClause> handler, RefPtr<BlockStatement> finalizer)
-        : Statement(source_range)
+        : LabelableStatement(source_range)
         , m_block(move(block))
         , m_handler(move(handler))
         , m_finalizer(move(finalizer))
@@ -1515,6 +1524,7 @@ public:
     virtual void dump(int indent) const override;
     virtual Value execute(Interpreter&, GlobalObject&) const override;
     virtual void generate_bytecode(Bytecode::Generator&) const override;
+    void add_label(FlyString string) override;
 
 private:
     NonnullRefPtr<BlockStatement> m_block;
@@ -1560,10 +1570,10 @@ private:
     NonnullRefPtrVector<Statement> m_consequent;
 };
 
-class SwitchStatement final : public Statement {
+class SwitchStatement final : public LabelableStatement {
 public:
     SwitchStatement(SourceRange source_range, NonnullRefPtr<Expression> discriminant, NonnullRefPtrVector<SwitchCase> cases)
-        : Statement(source_range)
+        : LabelableStatement(source_range)
         , m_discriminant(move(discriminant))
         , m_cases(move(cases))
     {

Userland/Libraries/LibJS/Interpreter.cpp

@@ -187,6 +187,14 @@ Value Interpreter::execute_statement(GlobalObject& global_object, const Statemen
         return statement.execute(*this, global_object);
 
     auto& block = static_cast<const ScopeNode&>(statement);
+    Vector<FlyString> const& labels = [&] {
+        if (is<BlockStatement>(block)) {
+            return static_cast<BlockStatement const&>(block).labels();
+        } else {
+            return Vector<FlyString>();
+        }
+    }();
+
     enter_scope(block, scope_type, global_object);
 
     Value last_value;
@@ -195,7 +203,7 @@ Value Interpreter::execute_statement(GlobalObject& global_object, const Statemen
         if (!value.is_empty())
             last_value = value;
         if (vm().should_unwind()) {
-            if (!block.labels().is_empty() && vm().should_unwind_until(ScopeType::Breakable, block.labels()))
+            if (!labels.is_empty() && vm().should_unwind_until(ScopeType::Breakable, labels))
                 vm().stop_unwind();
             break;
         }

Userland/Libraries/LibJS/Parser.cpp

@@ -586,7 +586,12 @@ RefPtr<Statement> Parser::try_parse_labelled_statement(AllowLabelledFunction all
         return {};
     }
 
-    auto identifier = consume_identifier_reference().value();
+    auto identifier = [&] {
+        if (m_state.current_token.value() == "await"sv) {
+            return consume().value();
+        }
+        return consume_identifier_reference().value();
+    }();
     if (!match(TokenType::Colon))
         return {};
     consume(TokenType::Colon);
@@ -594,6 +599,14 @@ RefPtr<Statement> Parser::try_parse_labelled_statement(AllowLabelledFunction all
     if (!match_statement())
         return {};
 
+    state_rollback_guard.disarm();
+    discard_saved_state();
+
+    if (m_state.strict_mode && identifier == "let"sv) {
+        syntax_error("Strict mode reserved word 'let' is not allowed in label", rule_start.position());
+        return {};
+    }
+
     if (match(TokenType::Function) && (allow_function == AllowLabelledFunction::No || m_state.strict_mode)) {
         syntax_error("Not allowed to declare a function here");
         return {};
@@ -604,8 +617,10 @@ RefPtr<Statement> Parser::try_parse_labelled_statement(AllowLabelledFunction all
 
     RefPtr<Statement> labelled_statement;
 
+    auto is_iteration_statement = false;
+
     if (match(TokenType::Function)) {
-        m_state.labels_in_scope.set(identifier, false);
+        m_state.labels_in_scope.set(identifier, {});
         auto function_declaration = parse_function_node<FunctionDeclaration>();
         m_state.current_scope->function_declarations.append(function_declaration);
         auto hoisting_target = m_state.current_scope->get_current_function_scope();
@@ -615,16 +630,23 @@ RefPtr<Statement> Parser::try_parse_labelled_statement(AllowLabelledFunction all
 
         labelled_statement = move(function_declaration);
     } else {
-        auto is_iteration_statement = match(TokenType::For) || match(TokenType::Do) || match(TokenType::While);
-        m_state.labels_in_scope.set(identifier, is_iteration_statement);
-        labelled_statement = parse_statement();
+        m_state.labels_in_scope.set(identifier, {});
+        labelled_statement = parse_statement(allow_function);
+        if (is<IterationStatement>(*labelled_statement)) {
+            is_iteration_statement = true;
+            static_cast<IterationStatement&>(*labelled_statement).add_label(identifier);
+        } else if (is<LabelableStatement>(*labelled_statement)) {
+            static_cast<LabelableStatement&>(*labelled_statement).add_label(identifier);
+        }
+    }
+
+    if (!is_iteration_statement) {
+        if (auto entry = m_state.labels_in_scope.find(identifier); entry != m_state.labels_in_scope.end() && entry->value.has_value())
+            syntax_error("labelled continue statement cannot use non iterating statement", m_state.labels_in_scope.get(identifier).value());
     }
 
     m_state.labels_in_scope.remove(identifier);
 
-    labelled_statement->add_label(identifier);
-    state_rollback_guard.disarm();
-    discard_saved_state();
     return labelled_statement.release_nonnull();
 }
 
@@ -2376,7 +2398,7 @@ NonnullRefPtr<BreakStatement> Parser::parse_break_statement()
     if (match(TokenType::Semicolon)) {
         consume();
     } else {
-        if (match(TokenType::Identifier) && !m_state.current_token.trivia_contains_line_terminator()) {
+        if (!m_state.current_token.trivia_contains_line_terminator() && match_identifier()) {
             target_label = consume().value();
 
             auto label = m_state.labels_in_scope.find(target_label);
@@ -2404,12 +2426,15 @@ NonnullRefPtr<ContinueStatement> Parser::parse_continue_statement()
         consume();
         return create_ast_node<ContinueStatement>({ m_state.current_token.filename(), rule_start.position(), position() }, target_label);
     }
-    if (match(TokenType::Identifier) && !m_state.current_token.trivia_contains_line_terminator()) {
+    if (!m_state.current_token.trivia_contains_line_terminator() && match_identifier()) {
+        auto label_position = position();
         target_label = consume().value();
 
         auto label = m_state.labels_in_scope.find(target_label);
-        if (label == m_state.labels_in_scope.end() || !label->value)
+        if (label == m_state.labels_in_scope.end())
             syntax_error(String::formatted("Label '{}' not found or invalid", target_label));
+        else
+            label->value = label_position;
     }
     consume_or_insert_semicolon();
     return create_ast_node<ContinueStatement>({ m_state.current_token.filename(), rule_start.position(), position() }, target_label);

Userland/Libraries/LibJS/Parser.h

@@ -259,7 +259,7 @@ private:
 
         Vector<Vector<FunctionNode::Parameter>&> function_parameters;
 
-        HashMap<StringView, bool> labels_in_scope;
+        HashMap<StringView, Optional<Position>> labels_in_scope;
         bool strict_mode { false };
         bool allow_super_property_lookup { false };
         bool allow_super_constructor_call { false };

Userland/Libraries/LibJS/Runtime/VM.h

@@ -193,11 +193,13 @@ public:
         m_unwind_until = ScopeType::None;
         m_unwind_until_label = {};
     }
-    bool should_unwind_until(ScopeType type, HashTable<FlyString> const& labels) const
+    bool should_unwind_until(ScopeType type, Vector<FlyString> const& labels) const
     {
         if (m_unwind_until_label.is_null())
             return m_unwind_until == type;
-        return m_unwind_until == type && labels.contains(m_unwind_until_label);
+        return m_unwind_until == type && any_of(labels.begin(), labels.end(), [&](FlyString const& label) {
+            return m_unwind_until_label == label;
+        });
     }
     bool should_unwind() const { return m_unwind_until != ScopeType::None; }
 

Userland/Libraries/LibJS/Tests/labels.js

@@ -1,16 +1,16 @@
 test("labeled plain scope", () => {
-    test: {
+    notused: test: alsonotused: {
         let o = 1;
         expect(o).toBe(1);
-        break test;
+        unused: break test;
         expect().fail();
     }
 });
 
 test("break on plain scope from inner scope", () => {
-    outer: {
+    notused: outer: alsonotused: {
         {
-            break outer;
+            unused: break outer;
         }
         expect().fail();
     }
@@ -18,7 +18,7 @@ test("break on plain scope from inner scope", () => {
 
 test("labeled for loop with break", () => {
     let counter = 0;
-    outer: for (a of [1, 2, 3]) {
+    notused: outer: alsonotused: for (a of [1, 2, 3]) {
         for (b of [4, 5, 6]) {
             if (a === 2 && b === 5) break outer;
             counter++;
@@ -29,7 +29,7 @@ test("labeled for loop with break", () => {
 
 test("labeled for loop with continue", () => {
     let counter = 0;
-    outer: for (a of [1, 2, 3]) {
+    notused: outer: alsonotused: for (a of [1, 2, 3]) {
         for (b of [4, 5, 6]) {
             if (b === 6) continue outer;
             counter++;
@@ -38,10 +38,107 @@ test("labeled for loop with continue", () => {
     expect(counter).toBe(6);
 });
 
-test("invalid label across scope", () => {
-    expect(`
-        label: {
-            (() => { break label; });
-        }
-    `).not.toEval();
+test("break on try catch statement", () => {
+    let entered = false;
+    label1: label2: label3: try {
+        entered = true;
+        break label2;
+        expect().fail();
+    } catch (e) {
+        expect().fail();
+    }
+    expect(entered).toBeTrue();
+});
+
+test("can break on every label", () => {
+    let i = 0;
+    label0: label1: label2: for (; i < 3; i++) {
+        block: {
+            break block;
+            expect().fail();
+        }
+        if (i === 0) continue label0;
+        if (i === 1) continue label1;
+        if (i === 2) continue label2;
+        expect().fail();
+    }
+    expect(i).toBe(3);
+});
+
+test("can use certain 'keywords' as labels", () => {
+    let i = 0;
+
+    yield: {
+        i++;
+        break yield;
+        expect().fail();
+    }
+
+    await: {
+        i++;
+        break await;
+        expect().fail();
+    }
+
+    async: {
+        i++;
+        break async;
+        expect().fail();
+    }
+
+    let: {
+        i++;
+        break let;
+        expect().fail();
+    }
+
+    // prettier-ignore
+    l\u0065t: {
+        i++;
+        break let;
+        expect().fail();
+    }
+
+    private: {
+        i++;
+        break private;
+        expect().fail();
+    }
+
+    expect(i).toBe(6);
+
+    expect(`const: { break const; }`).not.toEval();
+});
+
+test("invalid label usage", () => {
+    expect(() =>
+        eval(`
+            label: {
+                (() => {
+                    break label;
+                });
+            }
+        `)
+    ).toThrowWithMessage(SyntaxError, "Label 'label' not found");
+
+    expect(() =>
+        eval(`
+            label: {
+                while (false) {
+                    continue label;
+                }
+            }
+        `)
+    ).toThrowWithMessage(
+        SyntaxError,
+        "labelled continue statement cannot use non iterating statement"
+    );
+
+    expect(() =>
+        eval(`
+            label: label: {
+                break label;
+            }
+        `)
+    ).toThrowWithMessage(SyntaxError, "Label 'label' has already been declared");
 });