From 458180da5e61887cd9f820e573f307d0a640128d Mon Sep 17 00:00:00 2001 From: Spartan322 Date: Sat, 11 May 2024 14:37:50 -0400 Subject: Fix bugs in #37 Fix error handling dropping errors Fix error handling segfaults Improve error messages --- include/openvic-dataloader/AbstractSyntaxTree.hpp | 8 +--- include/openvic-dataloader/DiagnosticLogger.hpp | 55 ++++++++++++---------- include/openvic-dataloader/Error.hpp | 13 +++-- include/openvic-dataloader/Parser.hpp | 2 + include/openvic-dataloader/csv/Parser.hpp | 4 +- include/openvic-dataloader/detail/SymbolIntern.hpp | 14 ++++++ .../detail/utility/ErrorRange.hpp | 5 +- include/openvic-dataloader/v2script/Parser.hpp | 6 +-- src/openvic-dataloader/csv/Parser.cpp | 18 ++++--- src/openvic-dataloader/detail/Parser.cpp | 7 +++ src/openvic-dataloader/detail/dsl.hpp | 6 +-- .../v2script/DecisionGrammar.hpp | 2 +- .../v2script/ModifierGrammar.hpp | 4 +- src/openvic-dataloader/v2script/Parser.cpp | 23 ++++++--- src/openvic-dataloader/v2script/SimpleGrammar.hpp | 12 +++-- 15 files changed, 114 insertions(+), 65 deletions(-) create mode 100644 include/openvic-dataloader/detail/SymbolIntern.hpp diff --git a/include/openvic-dataloader/AbstractSyntaxTree.hpp b/include/openvic-dataloader/AbstractSyntaxTree.hpp index db81eeb..c6453e3 100644 --- a/include/openvic-dataloader/AbstractSyntaxTree.hpp +++ b/include/openvic-dataloader/AbstractSyntaxTree.hpp @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -17,12 +18,7 @@ #include namespace ovdl { - struct AbstractSyntaxTree { - struct SymbolId; - using index_type = std::uint32_t; - using symbol_type = dryad::symbol; - using symbol_interner_type = dryad::symbol_interner; - + struct AbstractSyntaxTree : SymbolIntern { symbol_type intern(const char* str, std::size_t length); symbol_type intern(std::string_view str); const char* intern_cstr(const char* str, std::size_t length); diff --git a/include/openvic-dataloader/DiagnosticLogger.hpp b/include/openvic-dataloader/DiagnosticLogger.hpp index 1fa2784..bd8f9cc 100644 --- a/include/openvic-dataloader/DiagnosticLogger.hpp +++ b/include/openvic-dataloader/DiagnosticLogger.hpp @@ -11,9 +11,14 @@ #include #include #include +#include #include #include +#include +#include +#include +#include #include #include #include @@ -26,18 +31,14 @@ #include -#include "openvic-dataloader/detail/CallbackOStream.hpp" -#include "openvic-dataloader/detail/utility/ErrorRange.hpp" -#include "openvic-dataloader/detail/utility/Utility.hpp" - #include namespace ovdl { - struct DiagnosticLogger { + struct DiagnosticLogger : SymbolIntern { using AnnotationKind = lexy_ext::annotation_kind; using DiagnosticKind = lexy_ext::diagnostic_kind; - using error_range = detail::error_range; + using error_range = detail::error_range; explicit operator bool() const; bool errored() const; @@ -57,28 +58,36 @@ namespace ovdl { using Reader = lexy::input_reader; error::Error* result; + std::string production_name = context.production(); + auto left_strip = production_name.find_first_of('<'); + if (left_strip != std::string::npos) { + auto right_strip = production_name.find_first_of('>', left_strip); + if (right_strip != std::string::npos) { + production_name.erase(left_strip, right_strip - left_strip + 1); + } + } + + auto production = _logger.intern_cstr(production_name); if constexpr (std::is_same_v) { auto string = lexy::_detail::make_literal_lexeme(error.string(), error.length()); - NodeLocation loc = NodeLocation::make_from(string.begin(), string.end()); + NodeLocation loc = NodeLocation::make_from(context.position(), error.position() - 1); auto message = _logger.intern_cstr(fmt::format("expected '{}'", string.data())); - result = _logger.template create(loc, message, context.production()); + result = _logger.template create(loc, message, production); } else if constexpr (std::is_same_v) { auto string = lexy::_detail::make_literal_lexeme(error.string(), error.length()); - NodeLocation loc = NodeLocation::make_from(string.begin(), string.end()); + NodeLocation loc = NodeLocation::make_from(context.position(), error.position() - 1); auto message = _logger.intern_cstr(fmt::format("expected keyword '{}'", string.data())); - result = _logger.template create(loc, message, context.production()); + result = _logger.template create(loc, message, production); } else if constexpr (std::is_same_v) { auto message = _logger.intern_cstr(fmt::format("expected {}", error.name())); - result = _logger.template create(error.position(), message, context.production()); + result = _logger.template create(error.position(), message, production); } else { NodeLocation loc = NodeLocation::make_from(error.begin(), error.end()); auto message = _logger.intern_cstr(error.message()); - result = _logger.template create(loc, message, context.production()); + result = _logger.template create(loc, message, production); } - if constexpr (requires { _logger.insert(result); }) { - _logger.insert(result); - } + _logger.insert(result); _count++; } @@ -119,12 +128,12 @@ namespace ovdl { dryad::node_map _map; dryad::tree _tree; - struct SymbolId; - using index_type = std::uint32_t; - using symbol_type = dryad::symbol; - using symbol_interner_type = dryad::symbol_interner; symbol_interner_type _symbol_interner; + void insert(error::Error* root) { + _tree.root()->insert_back(root); + } + public: symbol_type intern(const char* str, std::size_t length) { return _symbol_interner.intern(str, length); @@ -282,10 +291,10 @@ namespace ovdl { auto message = _logger.intern_cstr(output); switch (kind) { case AnnotationKind::primary: - _logger.create(loc, message); + annotation = _logger.create(loc, message); break; case AnnotationKind::secondary: - _logger.create(loc, message); + annotation = _logger.create(loc, message); break; default: detail::unreachable(); } @@ -381,10 +390,6 @@ namespace ovdl { } private: - void insert(error::Error* root) { - _tree.root()->insert_back(root); - } - const file_type* _file; }; } \ No newline at end of file diff --git a/include/openvic-dataloader/Error.hpp b/include/openvic-dataloader/Error.hpp index fd3254d..726079c 100644 --- a/include/openvic-dataloader/Error.hpp +++ b/include/openvic-dataloader/Error.hpp @@ -59,7 +59,7 @@ namespace ovdl::error { } struct Error : dryad::abstract_node_all { - const char* message() const { return _message; } + std::string_view message() const { return _message; } protected: DRYAD_ABSTRACT_NODE_CTOR(Error); @@ -79,7 +79,7 @@ namespace ovdl::error { struct Root : dryad::basic_node> { explicit Root(dryad::node_ctor ctor) : node_base(ctor) {} - DRYAD_CHILD_NODE_RANGE_GETTER(Error, errors, nullptr, this); + DRYAD_CHILD_NODE_RANGE_GETTER(Error, errors, nullptr, this->node_after(_last)); void insert_back(Error* error) { insert_child_after(_last, error); @@ -99,7 +99,7 @@ namespace ovdl::error { }; struct ParseError : dryad::abstract_node_range { - const char* production_name() const { return _production_name; } + std::string_view production_name() const { return _production_name; } protected: explicit ParseError(dryad::node_ctor ctor, @@ -126,7 +126,7 @@ namespace ovdl::error { using GenericParseError = _ParseError_t; struct Semantic : dryad::abstract_node_range, ErrorKind::FirstSemantic, ErrorKind::LastSemantic> { - DRYAD_CHILD_NODE_RANGE_GETTER(Annotation, annotations, nullptr, this); + DRYAD_CHILD_NODE_RANGE_GETTER(Annotation, annotations, nullptr, this->node_after(_last_annotation)); void push_back(Annotation* annotation); void push_back(AnnotationList p_annotations); @@ -146,6 +146,9 @@ namespace ovdl::error { insert_child_list_after(nullptr, annotations); _set_message(message); }; + + private: + Error* _last_annotation; }; template @@ -187,9 +190,11 @@ namespace ovdl::error { inline void Semantic::push_back(Annotation* annotation) { insert_child_after(annotations().end().deref(), annotation); + _last_annotation = annotation; } inline void Semantic::push_back(AnnotationList p_annotations) { insert_child_list_after(annotations().end().deref(), p_annotations); + _last_annotation = *p_annotations.end(); } } \ No newline at end of file diff --git a/include/openvic-dataloader/Parser.hpp b/include/openvic-dataloader/Parser.hpp index c1f266b..b885f3d 100644 --- a/include/openvic-dataloader/Parser.hpp +++ b/include/openvic-dataloader/Parser.hpp @@ -22,6 +22,8 @@ namespace ovdl::detail { std::string_view get_file_path() const; protected: + void set_file_path(std::string_view path); + std::reference_wrapper _error_stream; std::string _file_path; bool _has_fatal_error = false; diff --git a/include/openvic-dataloader/csv/Parser.hpp b/include/openvic-dataloader/csv/Parser.hpp index ccf732a..06e7251 100644 --- a/include/openvic-dataloader/csv/Parser.hpp +++ b/include/openvic-dataloader/csv/Parser.hpp @@ -31,7 +31,7 @@ namespace ovdl::csv { constexpr Parser& load_from_buffer(const char* data, std::size_t size); constexpr Parser& load_from_buffer(const char* start, const char* end); constexpr Parser& load_from_string(const std::string_view string); - constexpr Parser& load_from_file(const char* path); + Parser& load_from_file(const char* path); Parser& load_from_file(const std::filesystem::path& path); constexpr Parser& load_from_file(const detail::HasCstr auto& path) { @@ -42,7 +42,7 @@ namespace ovdl::csv { const std::vector& get_lines() const; - using error_range = ovdl::detail::error_range; + using error_range = ovdl::detail::error_range; Parser::error_range get_errors() const; const FilePosition get_error_position(const error::Error* error) const; diff --git a/include/openvic-dataloader/detail/SymbolIntern.hpp b/include/openvic-dataloader/detail/SymbolIntern.hpp new file mode 100644 index 0000000..8755887 --- /dev/null +++ b/include/openvic-dataloader/detail/SymbolIntern.hpp @@ -0,0 +1,14 @@ +#pragma once + +#include + +#include + +namespace ovdl { + struct SymbolIntern { + struct SymbolId; + using index_type = std::uint32_t; + using symbol_type = dryad::symbol; + using symbol_interner_type = dryad::symbol_interner; + }; +} \ No newline at end of file diff --git a/include/openvic-dataloader/detail/utility/ErrorRange.hpp b/include/openvic-dataloader/detail/utility/ErrorRange.hpp index a427f6c..7d5ca13 100644 --- a/include/openvic-dataloader/detail/utility/ErrorRange.hpp +++ b/include/openvic-dataloader/detail/utility/ErrorRange.hpp @@ -2,10 +2,9 @@ #include -#include - #include namespace ovdl::detail { - using error_range = decltype(std::declval()->errors()); + template + using error_range = decltype(std::declval()->errors()); } \ No newline at end of file diff --git a/include/openvic-dataloader/v2script/Parser.hpp b/include/openvic-dataloader/v2script/Parser.hpp index cef1faf..ea42aa2 100644 --- a/include/openvic-dataloader/v2script/Parser.hpp +++ b/include/openvic-dataloader/v2script/Parser.hpp @@ -33,7 +33,7 @@ namespace ovdl::v2script { constexpr Parser& load_from_buffer(const char* data, std::size_t size); constexpr Parser& load_from_buffer(const char* start, const char* end); constexpr Parser& load_from_string(const std::string_view string); - constexpr Parser& load_from_file(const char* path); + Parser& load_from_file(const char* path); Parser& load_from_file(const std::filesystem::path& path); constexpr Parser& load_from_file(const detail::HasCstr auto& path) { @@ -47,14 +47,14 @@ namespace ovdl::v2script { const FileTree* get_file_node() const; - std::string_view value(const ovdl::v2script::ast::FlatValue& node) const; + std::string_view value(const ovdl::v2script::ast::FlatValue* node) const; std::string make_native_string() const; std::string make_list_string() const; const FilePosition get_position(const ast::Node* node) const; - using error_range = ovdl::detail::error_range; + using error_range = ovdl::detail::error_range; Parser::error_range get_errors() const; const FilePosition get_error_position(const error::Error* error) const; diff --git a/src/openvic-dataloader/csv/Parser.cpp b/src/openvic-dataloader/csv/Parser.cpp index 849ea05..361f6ad 100644 --- a/src/openvic-dataloader/csv/Parser.cpp +++ b/src/openvic-dataloader/csv/Parser.cpp @@ -142,8 +142,8 @@ constexpr Parser& Parser::load_from_string(const std::string } template -constexpr Parser& Parser::load_from_file(const char* path) { - _file_path = path; +Parser& Parser::load_from_file(const char* path) { + set_file_path(path); // Type can be deduced?? _run_load_func(std::mem_fn(&ParseHandler::load_file), path); return *this; @@ -224,15 +224,21 @@ void Parser::print_errors_to(std::basic_ostream& stream) const { dryad::visit_tree( error, [&](const error::BufferError* buffer_error) { - stream << buffer_error->message() << '\n'; + stream << "buffer error: " << buffer_error->message() << '\n'; }, [&](const error::ParseError* parse_error) { - stream << parse_error->message() << '\n'; + auto position = get_error_position(parse_error); + std::string pos_str = fmt::format(":{}:{}: ", position.start_line, position.start_column); + stream << _file_path << pos_str << "parse error for '" << parse_error->production_name() << "': " << parse_error->message() << '\n'; }, [&](dryad::child_visitor visitor, const error::Semantic* semantic) { - stream << semantic->message() << '\n'; + auto position = get_error_position(semantic); + std::string pos_str = ": "; + if (!position.is_empty()) { + pos_str = fmt::format(":{}:{}: ", position.start_line, position.start_column); + } + stream << _file_path << pos_str << semantic->message() << '\n'; auto annotations = semantic->annotations(); - if (annotations.empty()) return; for (auto annotation : annotations) { visitor(annotation); } diff --git a/src/openvic-dataloader/detail/Parser.cpp b/src/openvic-dataloader/detail/Parser.cpp index fd87687..bb5c612 100644 --- a/src/openvic-dataloader/detail/Parser.cpp +++ b/src/openvic-dataloader/detail/Parser.cpp @@ -1,3 +1,4 @@ +#include #include #include @@ -40,4 +41,10 @@ bool BasicParser::has_warning() const { std::string_view BasicParser::get_file_path() const { return _file_path; +} + +void BasicParser::set_file_path(std::string_view path) { + std::error_code error; + std::filesystem::path file_path = std::filesystem::weakly_canonical(path, error); + _file_path = file_path.string(); } \ No newline at end of file diff --git a/src/openvic-dataloader/detail/dsl.hpp b/src/openvic-dataloader/detail/dsl.hpp index 9b544bc..ccc1af6 100644 --- a/src/openvic-dataloader/detail/dsl.hpp +++ b/src/openvic-dataloader/detail/dsl.hpp @@ -97,14 +97,14 @@ namespace ovdl::dsl { template< IsParseState ParseType, - typename Identifier, + auto Identifier, typename RuleValue, ovdl::detail::string_literal Keyword, auto Production, auto Value> struct keyword_rule { struct rule_t { - static constexpr auto keyword = ovdl::dsl::keyword(lexy::dsl::inline_); + static constexpr auto keyword = ovdl::dsl::keyword(Identifier); static constexpr auto rule = lexy::dsl::position(keyword) >> lexy::dsl::equal_sign; static constexpr auto value = Value; }; @@ -114,7 +114,7 @@ namespace ovdl::dsl { template< IsParseState ParseType, - typename Identifier, + auto Identifier, typename RuleValue, ovdl::detail::string_literal Keyword, auto Production, diff --git a/src/openvic-dataloader/v2script/DecisionGrammar.hpp b/src/openvic-dataloader/v2script/DecisionGrammar.hpp index 05cb8cc..b5010ff 100644 --- a/src/openvic-dataloader/v2script/DecisionGrammar.hpp +++ b/src/openvic-dataloader/v2script/DecisionGrammar.hpp @@ -40,7 +40,7 @@ namespace ovdl::v2script::grammar { struct DecisionList { static constexpr auto rule = - ovdl::dsl::keyword<"political_decisions">(lexy::dsl::inline_>) >> + ovdl::dsl::keyword<"political_decisions">(id) >> (lexy::dsl::equal_sign >> lexy::dsl::curly_bracketed.opt_list(lexy::dsl::p)); static constexpr auto value = lexy::as_list; diff --git a/src/openvic-dataloader/v2script/ModifierGrammar.hpp b/src/openvic-dataloader/v2script/ModifierGrammar.hpp index 5b937d5..d6dbb32 100644 --- a/src/openvic-dataloader/v2script/ModifierGrammar.hpp +++ b/src/openvic-dataloader/v2script/ModifierGrammar.hpp @@ -15,8 +15,8 @@ #include "detail/dsl.hpp" namespace ovdl::v2script::grammar { - constexpr auto modifier_keyword = LEXY_KEYWORD("modifier", lexy::dsl::inline_>); - constexpr auto factor_keyword = LEXY_KEYWORD("factor", lexy::dsl::inline_>); + constexpr auto modifier_keyword = LEXY_KEYWORD("modifier", id); + constexpr auto factor_keyword = LEXY_KEYWORD("factor", id); struct FactorStatement { static constexpr auto rule = lexy::dsl::position(factor_keyword) >> (lexy::dsl::equal_sign + lexy::dsl::p>); diff --git a/src/openvic-dataloader/v2script/Parser.cpp b/src/openvic-dataloader/v2script/Parser.cpp index 29e9e80..a4cad9d 100644 --- a/src/openvic-dataloader/v2script/Parser.cpp +++ b/src/openvic-dataloader/v2script/Parser.cpp @@ -1,5 +1,6 @@ #include "openvic-dataloader/v2script/Parser.hpp" +#include #include #include #include @@ -149,8 +150,8 @@ constexpr Parser& Parser::load_from_string(const std::string_view string) { return load_from_buffer(string.data(), string.size()); } -constexpr Parser& Parser::load_from_file(const char* path) { - _file_path = path; +Parser& Parser::load_from_file(const char* path) { + set_file_path(path); // Type can be deduced?? _run_load_func(std::mem_fn(&ParseHandler::load_file), path); return *this; @@ -179,6 +180,7 @@ bool Parser::simple_parse() { _has_error = _parse_handler->parse_state().logger().errored(); _has_warning = _parse_handler->parse_state().logger().warned(); if (!_parse_handler->root()) { + _has_error = true; _has_fatal_error = true; if (&_error_stream.get() != &detail::cnull) { print_errors_to(_error_stream); @@ -258,8 +260,8 @@ const FileTree* Parser::get_file_node() const { return _parse_handler->root(); } -std::string_view Parser::value(const ovdl::v2script::ast::FlatValue& node) const { - return node.value(_parse_handler->parse_state().ast().symbol_interner()); +std::string_view Parser::value(const ovdl::v2script::ast::FlatValue* node) const { + return node->value(_parse_handler->parse_state().ast().symbol_interner()); } std::string Parser::make_native_string() const { @@ -319,13 +321,20 @@ void Parser::print_errors_to(std::basic_ostream& stream) const { dryad::visit_tree( error, [&](const error::BufferError* buffer_error) { - stream << buffer_error->message() << '\n'; + stream << "buffer error: " << buffer_error->message() << '\n'; }, [&](const error::ParseError* parse_error) { - stream << parse_error->message() << '\n'; + auto position = get_error_position(parse_error); + std::string pos_str = fmt::format(":{}:{}: ", position.start_line, position.start_column); + stream << _file_path << pos_str << "parse error for '" << parse_error->production_name() << "': " << parse_error->message() << '\n'; }, [&](dryad::child_visitor visitor, const error::Semantic* semantic) { - stream << semantic->message() << '\n'; + auto position = get_error_position(semantic); + std::string pos_str = ": "; + if (!position.is_empty()) { + pos_str = fmt::format(":{}:{}: ", position.start_line, position.start_column); + } + stream << _file_path << pos_str << semantic->message() << '\n'; auto annotations = semantic->annotations(); for (auto annotation : annotations) { visitor(annotation); diff --git a/src/openvic-dataloader/v2script/SimpleGrammar.hpp b/src/openvic-dataloader/v2script/SimpleGrammar.hpp index bd4adaa..731a7f1 100644 --- a/src/openvic-dataloader/v2script/SimpleGrammar.hpp +++ b/src/openvic-dataloader/v2script/SimpleGrammar.hpp @@ -5,6 +5,8 @@ #include #include +#include +#include #include "detail/dsl.hpp" @@ -37,12 +39,14 @@ namespace ovdl::v2script::grammar { /* REQUIREMENTS: DAT-631 */ static constexpr auto comment_specifier = LEXY_LIT("#") >> lexy::dsl::until(lexy::dsl::newline).or_eof(); + static constexpr auto ascii = lexy::dsl::ascii::alpha_digit_underscore / LEXY_ASCII_ONE_OF("+:@%&'-."); + /* REQUIREMENTS: * DAT-632 * DAT-635 */ static constexpr auto windows_1252_data_specifier = - lexy::dsl::ascii::alpha_digit_underscore / LEXY_ASCII_ONE_OF("+:@%&'-.") / + ascii / lexy::dsl::lit_b<0x8A> / lexy::dsl::lit_b<0x8C> / lexy::dsl::lit_b<0x8E> / lexy::dsl::lit_b<0x92> / lexy::dsl::lit_b<0x97> / lexy::dsl::lit_b<0x9A> / lexy::dsl::lit_b<0x9C> / dsl::make_range<0x9E, 0x9F>() / @@ -74,6 +78,8 @@ namespace ovdl::v2script::grammar { .map<'r'>('\r') .map<'t'>('\t'); + static constexpr auto id = lexy::dsl::identifier(data_char_class); + template struct SimpleGrammar { struct StatementListBlock; @@ -194,14 +200,14 @@ namespace ovdl::v2script::grammar { template> using keyword_rule = dsl::keyword_rule< ast::ParseState, - Identifier, + id, ast::AssignStatement, Keyword, Production, Value>; template> using fkeyword_rule = dsl::fkeyword_rule< ast::ParseState, - Identifier, + id, ast::AssignStatement, Keyword, Production, Value>; -- cgit v1.2.3-56-ga3b1