JSSpecCompiler: Push ParseError out of SpecFunction

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/Lexer.cpp

@@ -110,7 +110,7 @@ ParseErrorOr<TokenizeTreeResult> tokenize_tree(XML::Node const* node, bool allow
                 }
 
                 if (element.name == tag_span) {
-                    auto element_class = TRY(get_attribute_by_name(child, attribute_class));
+                    auto element_class = TRY(deprecated_get_attribute_by_name(child, attribute_class));
                     if (element_class != class_secnum)
                         return ParseError::create(String::formatted("Expected 'secnum' as a class name of <span>, but found '{}'", element_class), child);
                     tokens.append({ TokenType::SectionNumber, TRY(get_text_contents(child)), child });

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/SpecParser.cpp

@@ -200,6 +200,12 @@ void SpecificationClause::parse(SpecificationParsingContext& ctx, XML::Node cons
                     else
                         ctx.current_logical_scope().section = MUST(String::from_utf8(m_header.section_number));
                 } else {
+                    if (element.name == tag_h1) {
+                        ctx.diag().error(ctx.location_from_xml_offset(child->offset),
+                            "<h1> can only be the first child of <emu-clause>");
+                        return;
+                    }
+
                     if (element.name == tag_emu_clause) {
                         m_subclauses.append(create(ctx, child));
                         return;
@@ -227,13 +233,65 @@ void SpecificationClause::parse(SpecificationParsingContext& ctx, XML::Node cons
 
 bool SpecFunction::post_initialize(SpecificationParsingContext& ctx, XML::Node const* element)
 {
-    auto initialization_result = do_post_initialize(ctx, element);
+    VERIFY(element->as_element().name == tag_emu_clause);
-    if (initialization_result.is_error()) {
+
-        // TODO: Integrate backtracing parser errors better
+    auto maybe_id = get_attribute_by_name(element, attribute_id);
-        ctx.diag().error(ctx.location_from_xml_offset(initialization_result.error()->offset()),
+    if (!maybe_id.has_value()) {
-            "{}", initialization_result.error()->to_string());
+        ctx.diag().error(ctx.location_from_xml_offset(element->offset),
+            "no id attribute");
+    } else {
+        m_id = maybe_id.value();
+    }
+
+    auto maybe_abstract_operation_id = get_attribute_by_name(element, attribute_aoid);
+    if (maybe_abstract_operation_id.has_value())
+        m_name = maybe_abstract_operation_id.value();
+
+    m_section_number = m_header.section_number;
+    auto const& [function_name, arguments] = m_header.header.get<ClauseHeader::FunctionDefinition>();
+    m_arguments = arguments;
+
+    if (m_name != function_name) {
+        ctx.diag().warn(ctx.location_from_xml_offset(element->offset),
+            "function name in header and <emu-clause>[aoid] do not match");
+    }
+
+    Vector<XML::Node const*> algorithm_nodes;
+
+    for (auto const& child : element->as_element().children) {
+        child->content.visit(
+            [&](XML::Node::Element const& element) {
+                if (element.name == tag_h1) {
+                    // Processed in SpecificationClause
+                } else if (element.name == tag_p) {
+                    ctx.diag().warn(ctx.location_from_xml_offset(child->offset),
+                        "prose is ignored");
+                } else if (element.name == tag_emu_alg) {
+                    algorithm_nodes.append(child);
+                } else {
+                    ctx.diag().error(ctx.location_from_xml_offset(child->offset),
+                        "<{}> should not be a child of <emu-clause> specifing function"sv, element.name);
+                }
+            },
+            [&](auto const&) {});
+    }
+
+    if (algorithm_nodes.size() != 1) {
+        ctx.diag().error(ctx.location_from_xml_offset(element->offset),
+            "<emu-clause> specifing function should have exactly one <emu-alg>"sv);
         return false;
     }
+
+    auto algorithm_creation_result = Algorithm::create(algorithm_nodes[0]);
+    if (algorithm_creation_result.is_error()) {
+        // TODO: Integrate backtracing parser errors better
+        ctx.diag().error(ctx.location_from_xml_offset(algorithm_creation_result.error()->offset()),
+            "{}", algorithm_creation_result.error()->to_string());
+        return false;
+    }
+
+    m_algorithm = algorithm_creation_result.release_value();
+
     return true;
 }
 
@@ -242,66 +300,6 @@ void SpecFunction::do_collect(TranslationUnitRef translation_unit)
     translation_unit->adopt_function(make_ref_counted<FunctionDefinition>(m_name, m_algorithm.m_tree, move(m_arguments)));
 }
 
-ParseErrorOr<void> SpecFunction::do_post_initialize(SpecificationParsingContext& ctx, XML::Node const* element)
-{
-    VERIFY(element->as_element().name == tag_emu_clause);
-
-    m_id = TRY(get_attribute_by_name(element, attribute_id));
-    m_name = TRY(get_attribute_by_name(element, attribute_aoid));
-
-    m_section_number = m_header.section_number;
-    auto const& [function_name, arguments] = m_header.header.get<ClauseHeader::FunctionDefinition>();
-
-    if (m_name != function_name) {
-        ctx.diag().warn(ctx.location_from_xml_offset(element->offset),
-            "function name in header and <emu-clause>[aoid] do not match");
-    }
-    m_arguments = arguments;
-
-    u32 children_count = 0;
-
-    XML::Node const* algorithm_node = nullptr;
-    XML::Node const* prose_node = nullptr;
-
-    for (auto const& child : element->as_element().children) {
-        TRY(child->content.visit(
-            [&](XML::Node::Element const& element) -> ParseErrorOr<void> {
-                if (element.name == tag_h1) {
-                    if (children_count != 0)
-                        return ParseError::create("<h1> can only be the first child of <emu-clause>"sv, child);
-                } else if (element.name == tag_p) {
-                    if (prose_node == nullptr)
-                        prose_node = child;
-                } else if (element.name == tag_emu_alg) {
-                    algorithm_node = child;
-                } else {
-                    return ParseError::create("Unknown child of <emu-clause>"sv, child);
-                }
-                ++children_count;
-                return {};
-            },
-            [&](XML::Node::Text const&) -> ParseErrorOr<void> {
-                if (!contains_empty_text(child)) {
-                    return ParseError::create("<emu-clause> should not have non-empty child text nodes"sv, child);
-                }
-                return {};
-            },
-            move(ignore_comments)));
-    }
-
-    if (algorithm_node == nullptr)
-        return ParseError::create("No <emu-alg>"sv, element);
-
-    if (prose_node) {
-        ctx.diag().warn(ctx.location_from_xml_offset(element->offset),
-            "prose is ignored");
-    }
-
-    m_algorithm = TRY(Algorithm::create(algorithm_node));
-
-    return {};
-}
-
 Specification Specification::create(SpecificationParsingContext& ctx, XML::Node const* element)
 {
     VERIFY(element->as_element().name == tag_specification);

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/SpecParser.h

@@ -105,8 +105,6 @@ protected:
     void do_collect(TranslationUnitRef translation_unit) override;
 
 private:
-    ParseErrorOr<void> do_post_initialize(SpecificationParsingContext& ctx, XML::Node const* element);
-
     StringView m_section_number;
     StringView m_id;
     StringView m_name;

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/XMLUtils.cpp

@@ -16,7 +16,7 @@ bool contains_empty_text(XML::Node const* node)
     return node->as_text().builder.string_view().trim_whitespace().is_empty();
 }
 
-ParseErrorOr<StringView> get_attribute_by_name(XML::Node const* node, StringView attribute_name)
+ParseErrorOr<StringView> deprecated_get_attribute_by_name(XML::Node const* node, StringView attribute_name)
 {
     auto const& attribute = node->as_element().attributes.get(attribute_name);
 
@@ -25,6 +25,15 @@ ParseErrorOr<StringView> get_attribute_by_name(XML::Node const* node, StringView
     return attribute.value();
 }
 
+Optional<StringView> get_attribute_by_name(XML::Node const* node, StringView attribute_name)
+{
+    auto const& attribute = node->as_element().attributes.get(attribute_name);
+
+    if (!attribute.has_value())
+        return {};
+    return attribute.value();
+}
+
 ParseErrorOr<StringView> get_text_contents(XML::Node const* node)
 {
     auto const& children = node->as_element().children;

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/XMLUtils.h

@@ -20,7 +20,8 @@ inline constexpr IgnoreComments ignore_comments {};
 
 bool contains_empty_text(XML::Node const* node);
 
-ParseErrorOr<StringView> get_attribute_by_name(XML::Node const* node, StringView attribute_name);
+ParseErrorOr<StringView> deprecated_get_attribute_by_name(XML::Node const* node, StringView attribute_name);
+Optional<StringView> get_attribute_by_name(XML::Node const* node, StringView attribute_name);
 
 ParseErrorOr<StringView> get_text_contents(XML::Node const* node);