LibJS: Use Identifier to represent CatchClause parameter names

By doing that we consistently use Identifier node for identifiers and
also enable mechanism that registers identifiers in a corresponding
ScopePusher for catch parameters, which is necessary for work in the
upcoming changes.

Libraries/LibJS/AST.cpp

@@ -1378,19 +1378,17 @@ void TryStatement::dump(int indent) const
 void CatchClause::dump(int indent) const
 {
     print_indent(indent);
-    m_parameter.visit(
-        [&](FlyString const& parameter) {
-            if (parameter.is_empty())
     outln("CatchClause");
-            else
+    m_parameter.visit(
-                outln("CatchClause ({})", parameter);
+        [&](NonnullRefPtr<Identifier const> const& parameter) {
+            parameter->dump(indent + 1);
         },
         [&](NonnullRefPtr<BindingPattern const> const& pattern) {
-            outln("CatchClause");
             print_indent(indent);
             outln("(Parameter)");
             pattern->dump(indent + 2);
-        });
+        },
+        [&](Empty) {});
 
     body().dump(indent + 1);
 }

Libraries/LibJS/AST.h

@@ -2075,7 +2075,7 @@ private:
 
 class CatchClause final : public ASTNode {
 public:
-    CatchClause(SourceRange source_range, FlyString parameter, NonnullRefPtr<BlockStatement const> body)
+    CatchClause(SourceRange source_range, NonnullRefPtr<Identifier const> parameter, NonnullRefPtr<BlockStatement const> body)
         : ASTNode(move(source_range))
         , m_parameter(move(parameter))
         , m_body(move(body))
@@ -2089,13 +2089,20 @@ public:
     {
     }
 
+    CatchClause(SourceRange source_range, NonnullRefPtr<BlockStatement const> body)
+        : ASTNode(move(source_range))
+        , m_parameter(Empty {})
+        , m_body(move(body))
+    {
+    }
+
     auto& parameter() const { return m_parameter; }
     BlockStatement const& body() const { return m_body; }
 
     virtual void dump(int indent) const override;
 
 private:
-    Variant<FlyString, NonnullRefPtr<BindingPattern const>> m_parameter;
+    Variant<NonnullRefPtr<Identifier const>, NonnullRefPtr<BindingPattern const>, Empty> m_parameter;
     NonnullRefPtr<BlockStatement const> m_body;
 };
 

Libraries/LibJS/Bytecode/ASTCodegen.cpp

@@ -2660,14 +2660,12 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> TryStatement::generate_
         bool did_create_variable_scope_for_catch_clause = false;
 
         TRY(m_handler->parameter().visit(
-            [&](FlyString const& parameter) -> Bytecode::CodeGenerationErrorOr<void> {
+            [&](NonnullRefPtr<Identifier const> const& parameter) -> Bytecode::CodeGenerationErrorOr<void> {
-                if (!parameter.is_empty()) {
                 generator.begin_variable_scope();
                 did_create_variable_scope_for_catch_clause = true;
-                    auto parameter_identifier = generator.intern_identifier(parameter);
+                auto parameter_identifier = generator.intern_identifier(parameter->string());
                 generator.emit<Bytecode::Op::CreateVariable>(parameter_identifier, Bytecode::Op::EnvironmentMode::Lexical, false);
                 generator.emit<Bytecode::Op::InitializeLexicalBinding>(parameter_identifier, caught_value);
-                }
                 return {};
             },
             [&](NonnullRefPtr<BindingPattern const> const& binding_pattern) -> Bytecode::CodeGenerationErrorOr<void> {
@@ -2675,6 +2673,9 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> TryStatement::generate_
                 did_create_variable_scope_for_catch_clause = true;
                 TRY(binding_pattern->generate_bytecode(generator, Bytecode::Op::BindingInitializationMode::Initialize, caught_value, true));
                 return {};
+            },
+            [](Empty) -> Bytecode::CodeGenerationErrorOr<void> {
+                return {};
             }));
 
         auto handler_result = TRY(m_handler->body().generate_bytecode(generator));

Libraries/LibJS/Parser.cpp

@@ -105,7 +105,7 @@ public:
         return scope_pusher;
     }
 
-    static ScopePusher catch_scope(Parser& parser, RefPtr<BindingPattern const> const& pattern, FlyString const& parameter)
+    static ScopePusher catch_scope(Parser& parser, RefPtr<BindingPattern const> const& pattern, RefPtr<Identifier const> const& parameter)
     {
         ScopePusher scope_pusher(parser, nullptr, ScopeLevel::NotTopLevel, ScopeType::Catch);
         if (pattern) {
@@ -114,9 +114,9 @@ public:
                 scope_pusher.m_forbidden_var_names.set(identifier.string());
                 scope_pusher.m_bound_names.set(identifier.string());
             }));
-        } else if (!parameter.is_empty()) {
+        } else if (parameter) {
-            scope_pusher.m_var_names.set(parameter);
+            scope_pusher.m_var_names.set(parameter->string());
-            scope_pusher.m_bound_names.set(parameter);
+            scope_pusher.m_bound_names.set(parameter->string());
         }
 
         return scope_pusher;
@@ -3777,7 +3777,7 @@ NonnullRefPtr<CatchClause const> Parser::parse_catch_clause()
     auto rule_start = push_start();
     consume(TokenType::Catch);
 
-    FlyString parameter;
+    RefPtr<Identifier const> parameter;
     RefPtr<BindingPattern const> pattern_parameter;
     auto should_expect_parameter = false;
     if (match(TokenType::ParenOpen)) {
@@ -3787,14 +3787,15 @@ NonnullRefPtr<CatchClause const> Parser::parse_catch_clause()
         if (match_identifier_name()
             && (!match(TokenType::Yield) || !m_state.in_generator_function_context)
             && (!match(TokenType::Async) || !m_state.await_expression_is_valid)
-            && (!match(TokenType::Await) || !m_state.in_class_static_init_block))
+            && (!match(TokenType::Await) || !m_state.in_class_static_init_block)) {
-            parameter = consume().fly_string_value();
+            parameter = parse_identifier();
-        else
+        } else {
             pattern_parameter = parse_binding_pattern(AllowDuplicates::No, AllowMemberExpressions::No);
+        }
         consume(TokenType::ParenClose);
     }
 
-    if (should_expect_parameter && parameter.is_empty() && !pattern_parameter)
+    if (should_expect_parameter && !parameter && !pattern_parameter)
         expected("an identifier or a binding pattern");
 
     HashTable<FlyString> bound_names;
@@ -3808,9 +3809,9 @@ NonnullRefPtr<CatchClause const> Parser::parse_catch_clause()
             }));
     }
 
-    if (!parameter.is_empty()) {
+    if (parameter) {
-        check_identifier_name_for_assignment_validity(parameter);
+        check_identifier_name_for_assignment_validity(parameter->string());
-        bound_names.set(parameter);
+        bound_names.set(parameter->string());
     }
 
     ScopePusher catch_scope = ScopePusher::catch_scope(*this, pattern_parameter, parameter);
@@ -3829,9 +3830,15 @@ NonnullRefPtr<CatchClause const> Parser::parse_catch_clause()
             move(body));
     }
 
+    if (parameter) {
+        return create_ast_node<CatchClause>(
+            { m_source_code, rule_start.position(), position() },
+            parameter.release_nonnull(),
+            move(body));
+    }
+
     return create_ast_node<CatchClause>(
         { m_source_code, rule_start.position(), position() },
-        move(parameter),
         move(body));
 }