Browse Source

LibJS: Don't use null DFS for break/continue statements without a label

Dan Klishch 1 year ago
parent
commit
78491204d9

+ 6 - 6
Userland/Libraries/LibJS/AST.h

@@ -2118,22 +2118,22 @@ private:
 
 class BreakStatement final : public Statement {
 public:
-    BreakStatement(SourceRange source_range, DeprecatedFlyString target_label)
+    BreakStatement(SourceRange source_range, Optional<DeprecatedFlyString> target_label)
         : Statement(move(source_range))
         , m_target_label(move(target_label))
     {
     }
 
-    DeprecatedFlyString const& target_label() const { return m_target_label; }
+    Optional<DeprecatedFlyString> const& target_label() const { return m_target_label; }
     virtual Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> generate_bytecode(Bytecode::Generator&, Optional<Bytecode::Operand> preferred_dst = {}) const override;
 
 private:
-    DeprecatedFlyString m_target_label;
+    Optional<DeprecatedFlyString> m_target_label;
 };
 
 class ContinueStatement final : public Statement {
 public:
-    ContinueStatement(SourceRange source_range, DeprecatedFlyString target_label)
+    ContinueStatement(SourceRange source_range, Optional<DeprecatedFlyString> target_label)
         : Statement(move(source_range))
         , m_target_label(move(target_label))
     {
@@ -2141,10 +2141,10 @@ public:
 
     virtual Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> generate_bytecode(Bytecode::Generator&, Optional<Bytecode::Operand> preferred_dst = {}) const override;
 
-    DeprecatedFlyString const& target_label() const { return m_target_label; }
+    Optional<DeprecatedFlyString> const& target_label() const { return m_target_label; }
 
 private:
-    DeprecatedFlyString m_target_label;
+    Optional<DeprecatedFlyString> m_target_label;
 };
 
 class DebuggerStatement final : public Statement {

+ 4 - 4
Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp

@@ -2199,12 +2199,12 @@ Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> ContinueStatement::
     // FIXME: Handle finally blocks in a graceful manner
     //        We need to execute the finally block, but tell it to resume
     //        execution at the designated block
-    if (m_target_label.is_null()) {
+    if (!m_target_label.has_value()) {
         generator.generate_continue();
         return Optional<Bytecode::Operand> {};
     }
 
-    generator.generate_continue(m_target_label);
+    generator.generate_continue(m_target_label.value());
     return Optional<Bytecode::Operand> {};
 }
 
@@ -2415,12 +2415,12 @@ Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> BreakStatement::gen
     // FIXME: Handle finally blocks in a graceful manner
     //        We need to execute the finally block, but tell it to resume
     //        execution at the designated block
-    if (m_target_label.is_null()) {
+    if (!m_target_label.has_value()) {
         generator.generate_break();
         return Optional<Bytecode::Operand> {};
     }
 
-    generator.generate_break(m_target_label);
+    generator.generate_break(m_target_label.value());
     return Optional<Bytecode::Operand> {};
 }
 

+ 7 - 7
Userland/Libraries/LibJS/Parser.cpp

@@ -3397,21 +3397,21 @@ NonnullRefPtr<BreakStatement const> Parser::parse_break_statement()
 {
     auto rule_start = push_start();
     consume(TokenType::Break);
-    DeprecatedFlyString target_label;
+    Optional<DeprecatedFlyString> target_label;
     if (match(TokenType::Semicolon)) {
         consume();
     } else {
         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);
+            auto label = m_state.labels_in_scope.find(target_label.value());
             if (label == m_state.labels_in_scope.end())
-                syntax_error(ByteString::formatted("Label '{}' not found", target_label));
+                syntax_error(ByteString::formatted("Label '{}' not found", target_label.value()));
         }
         consume_or_insert_semicolon();
     }
 
-    if (target_label.is_null() && !m_state.in_break_context)
+    if (!target_label.has_value() && !m_state.in_break_context)
         syntax_error("Unlabeled 'break' not allowed outside of a loop or switch statement");
 
     return create_ast_node<BreakStatement>({ m_source_code, rule_start.position(), position() }, target_label);
@@ -3424,7 +3424,7 @@ NonnullRefPtr<ContinueStatement const> Parser::parse_continue_statement()
         syntax_error("'continue' not allow outside of a loop");
 
     consume(TokenType::Continue);
-    DeprecatedFlyString target_label;
+    Optional<DeprecatedFlyString> target_label;
     if (match(TokenType::Semicolon)) {
         consume();
         return create_ast_node<ContinueStatement>({ m_source_code, rule_start.position(), position() }, target_label);
@@ -3433,9 +3433,9 @@ NonnullRefPtr<ContinueStatement const> Parser::parse_continue_statement()
         auto label_position = position();
         target_label = consume().value();
 
-        auto label = m_state.labels_in_scope.find(target_label);
+        auto label = m_state.labels_in_scope.find(target_label.value());
         if (label == m_state.labels_in_scope.end())
-            syntax_error(ByteString::formatted("Label '{}' not found or invalid", target_label));
+            syntax_error(ByteString::formatted("Label '{}' not found or invalid", target_label.value()));
         else
             label->value = label_position;
     }