From d339ad01bb55e081014b351fff2d4e0e44340a83 Mon Sep 17 00:00:00 2001 From: Dan Klishch Date: Tue, 16 Jan 2024 21:24:55 -0500 Subject: [PATCH] JSSpecCompiler: Push ParseError out of SpecFunction --- .../JSSpecCompiler/Parser/Lexer.cpp | 2 +- .../JSSpecCompiler/Parser/SpecParser.cpp | 128 +++++++++--------- .../JSSpecCompiler/Parser/SpecParser.h | 2 - .../JSSpecCompiler/Parser/XMLUtils.cpp | 11 +- .../JSSpecCompiler/Parser/XMLUtils.h | 3 +- 5 files changed, 76 insertions(+), 70 deletions(-) diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/Lexer.cpp b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/Lexer.cpp index e0678f30b8..7a06fa6dd2 100644 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/Lexer.cpp +++ b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/Lexer.cpp @@ -110,7 +110,7 @@ ParseErrorOr 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 , but found '{}'", element_class), child); tokens.append({ TokenType::SectionNumber, TRY(get_text_contents(child)), child }); diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/SpecParser.cpp b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/SpecParser.cpp index ed640d117c..631240a2f8 100644 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/SpecParser.cpp +++ b/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), + "

can only be the first child of "); + 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); - if (initialization_result.is_error()) { - // TODO: Integrate backtracing parser errors better - ctx.diag().error(ctx.location_from_xml_offset(initialization_result.error()->offset()), - "{}", initialization_result.error()->to_string()); + VERIFY(element->as_element().name == tag_emu_clause); + + auto maybe_id = get_attribute_by_name(element, attribute_id); + if (!maybe_id.has_value()) { + 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(); + m_arguments = arguments; + + if (m_name != function_name) { + ctx.diag().warn(ctx.location_from_xml_offset(element->offset), + "function name in header and [aoid] do not match"); + } + + Vector 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 specifing function"sv, element.name); + } + }, + [&](auto const&) {}); + } + + if (algorithm_nodes.size() != 1) { + ctx.diag().error(ctx.location_from_xml_offset(element->offset), + " specifing function should have exactly one "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(m_name, m_algorithm.m_tree, move(m_arguments))); } -ParseErrorOr 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(); - - if (m_name != function_name) { - ctx.diag().warn(ctx.location_from_xml_offset(element->offset), - "function name in header and [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 { - if (element.name == tag_h1) { - if (children_count != 0) - return ParseError::create("

can only be the first child of "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 "sv, child); - } - ++children_count; - return {}; - }, - [&](XML::Node::Text const&) -> ParseErrorOr { - if (!contains_empty_text(child)) { - return ParseError::create(" should not have non-empty child text nodes"sv, child); - } - return {}; - }, - move(ignore_comments))); - } - - if (algorithm_node == nullptr) - return ParseError::create("No "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); diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/SpecParser.h b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/SpecParser.h index a252b251e1..34dd86d15d 100644 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/SpecParser.h +++ b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/SpecParser.h @@ -105,8 +105,6 @@ protected: void do_collect(TranslationUnitRef translation_unit) override; private: - ParseErrorOr do_post_initialize(SpecificationParsingContext& ctx, XML::Node const* element); - StringView m_section_number; StringView m_id; StringView m_name; diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/XMLUtils.cpp b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/XMLUtils.cpp index a97d1eb5c3..283eb14b2e 100644 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/XMLUtils.cpp +++ b/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 get_attribute_by_name(XML::Node const* node, StringView attribute_name) +ParseErrorOr 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 get_attribute_by_name(XML::Node const* node, StringView return attribute.value(); } +Optional 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 get_text_contents(XML::Node const* node) { auto const& children = node->as_element().children; diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/XMLUtils.h b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/XMLUtils.h index fdf5d4bc90..bad64a7cc8 100644 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/XMLUtils.h +++ b/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 get_attribute_by_name(XML::Node const* node, StringView attribute_name); +ParseErrorOr deprecated_get_attribute_by_name(XML::Node const* node, StringView attribute_name); +Optional get_attribute_by_name(XML::Node const* node, StringView attribute_name); ParseErrorOr get_text_contents(XML::Node const* node);