From dcb5ebe57cb93922a9ea7861084dbabd7235294f Mon Sep 17 00:00:00 2001 From: Slavomir Kucera Date: Tue, 20 Jul 2021 16:26:11 +0200 Subject: [PATCH 1/9] test to pass --- .../test/processing/ca_instr_test.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/parser_library/test/processing/ca_instr_test.cpp b/parser_library/test/processing/ca_instr_test.cpp index e7c5edf1f..2242c5647 100644 --- a/parser_library/test/processing/ca_instr_test.cpp +++ b/parser_library/test/processing/ca_instr_test.cpp @@ -314,6 +314,23 @@ TEST(AGO, extended_fail) EXPECT_TRUE(ctx.get_var_sym(it3)); } +TEST(AGO, skip_invalid) +{ + std::string input(R"( + MACRO + MAC + AGO .SKIP + 2 a +.SKIP ANOP + MEND +)"); + analyzer a(input); + a.analyze(); + a.collect_diags(); + + EXPECT_TRUE(a.diags().empty()); +} + TEST(AIF, extended) { std::string input(R"( From 5f473f512a0747572da0b86df6ef52294ffcb1e9 Mon Sep 17 00:00:00 2001 From: Slavomir Kucera Date: Tue, 20 Jul 2021 16:28:17 +0200 Subject: [PATCH 2/9] refactor related diagnostics --- parser_library/src/CMakeLists.txt | 1 + .../src/checking/diagnostic_collector.cpp | 2 +- parser_library/src/diagnosable_ctx.cpp | 49 +++++++++++++++++++ parser_library/src/diagnosable_ctx.h | 34 ++----------- parser_library/src/parsing/parser_impl.cpp | 4 +- 5 files changed, 58 insertions(+), 32 deletions(-) create mode 100644 parser_library/src/diagnosable_ctx.cpp diff --git a/parser_library/src/CMakeLists.txt b/parser_library/src/CMakeLists.txt index aaec97c9c..aaf67fcf7 100644 --- a/parser_library/src/CMakeLists.txt +++ b/parser_library/src/CMakeLists.txt @@ -18,6 +18,7 @@ target_sources(parser_library PRIVATE analyzing_context.h compiler_options.h diagnosable.h + diagnosable_ctx.cpp diagnosable_ctx.h diagnosable_impl.h diagnostic.cpp diff --git a/parser_library/src/checking/diagnostic_collector.cpp b/parser_library/src/checking/diagnostic_collector.cpp index 60da17171..6e0ca6277 100644 --- a/parser_library/src/checking/diagnostic_collector.cpp +++ b/parser_library/src/checking/diagnostic_collector.cpp @@ -35,7 +35,7 @@ void diagnostic_collector::operator()(diagnostic_op diagnostic) const { if (!diagnoser_) return; - diagnoser_->add_diagnostic_inner(std::move(diagnostic), get_location_stack()); + diagnoser_->diagnosable_impl::add_diagnostic(add_stack_details(std::move(diagnostic), get_location_stack())); } context::processing_stack_t diagnostic_collector::get_location_stack() const diff --git a/parser_library/src/diagnosable_ctx.cpp b/parser_library/src/diagnosable_ctx.cpp new file mode 100644 index 000000000..641cdd053 --- /dev/null +++ b/parser_library/src/diagnosable_ctx.cpp @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2021 Broadcom. + * The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. + * + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Broadcom, Inc. - initial API and implementation + */ + +#include "diagnosable_ctx.h" + +namespace hlasm_plugin::parser_library { + +void diagnosable_ctx::add_diagnostic(diagnostic_s diagnostic) const +{ + diagnosable_impl::add_diagnostic(add_stack_details( + diagnostic_op( + diagnostic.severity, std::move(diagnostic.code), std::move(diagnostic.message), diagnostic.diag_range), + ctx_.processing_stack())); +} + +void diagnosable_ctx::add_diagnostic(diagnostic_op diagnostic) const +{ + diagnosable_impl::add_diagnostic(add_stack_details(std::move(diagnostic), ctx_.processing_stack())); +} + +diagnostic_s add_stack_details(diagnostic_op diagnostic, context::processing_stack_t stack) +{ + diagnostic_s diag(std::move(stack.back().proc_location.file), std::move(diagnostic)); + + diag.related.reserve(stack.size() - 1); + for (auto frame = ++stack.rbegin(); frame != stack.rend(); ++frame) + { + auto& file_name = frame->proc_location.file; + auto message = "While compiling " + file_name + '(' + std::to_string(frame->proc_location.pos.line + 1) + ")"; + diag.related.emplace_back( + range_uri_s(std::move(file_name), range(frame->proc_location.pos, frame->proc_location.pos)), + std::move(message)); + } + + return diag; +} + +} // namespace hlasm_plugin::parser_library diff --git a/parser_library/src/diagnosable_ctx.h b/parser_library/src/diagnosable_ctx.h index b81134a8c..9fbc855a2 100644 --- a/parser_library/src/diagnosable_ctx.h +++ b/parser_library/src/diagnosable_ctx.h @@ -28,45 +28,21 @@ class diagnosable_ctx : public diagnosable_impl, public diagnostic_op_consumer context::hlasm_context& ctx_; public: - void add_diagnostic(diagnostic_s diagnostic) const override - { - add_diagnostic_inner( - diagnostic_op( - diagnostic.severity, std::move(diagnostic.code), std::move(diagnostic.message), diagnostic.diag_range), - ctx_.processing_stack()); - } - - void add_diagnostic(diagnostic_op diagnostic) const override - { - add_diagnostic_inner(std::move(diagnostic), ctx_.processing_stack()); - } + void add_diagnostic(diagnostic_s diagnostic) const override; + void add_diagnostic(diagnostic_op diagnostic) const override; protected: diagnosable_ctx(context::hlasm_context& ctx) : ctx_(ctx) {} - virtual ~diagnosable_ctx() {}; - -private: - void add_diagnostic_inner(diagnostic_op diagnostic, const context::processing_stack_t& stack) const - { - diagnostic_s diag(stack.back().proc_location.file, diagnostic); - - for (auto frame = ++stack.rbegin(); frame != stack.rend(); ++frame) - { - auto& file_name = frame->proc_location.file; - range r = range(frame->proc_location.pos, frame->proc_location.pos); - diagnostic_related_info_s s = diagnostic_related_info_s(range_uri_s(file_name, r), - "While compiling " + file_name + '(' + std::to_string(frame->proc_location.pos.line + 1) + ")"); - diag.related.push_back(std::move(s)); - } - diagnosable_impl::add_diagnostic(std::move(diag)); - } + virtual ~diagnosable_ctx() = default; friend diagnostic_collector; }; +diagnostic_s add_stack_details(diagnostic_op diagnostic, context::processing_stack_t stack); + } // namespace hlasm_plugin::parser_library diff --git a/parser_library/src/parsing/parser_impl.cpp b/parser_library/src/parsing/parser_impl.cpp index b2b9476e0..3c709f8ae 100644 --- a/parser_library/src/parsing/parser_impl.cpp +++ b/parser_library/src/parsing/parser_impl.cpp @@ -149,14 +149,14 @@ void parser_impl::resolve_expression(expressions::ca_expr_ptr& expr) const else if (opcode.value == hlasm_ctx->ids().well_known.SETB) { if (!expr->is_compatible(ca_expression_compatibility::setb)) - expr->diags().push_back(diagnostic_op::error_CE016_logical_expression_parenthesis(expr->expr_range)); + expr->add_diagnostic(diagnostic_op::error_CE016_logical_expression_parenthesis(expr->expr_range)); resolve_expression(expr, context::SET_t_enum::B_TYPE); } else if (opcode.value == hlasm_ctx->ids().well_known.AIF) { if (!expr->is_compatible(ca_expression_compatibility::aif)) - expr->diags().push_back(diagnostic_op::error_CE016_logical_expression_parenthesis(expr->expr_range)); + expr->add_diagnostic(diagnostic_op::error_CE016_logical_expression_parenthesis(expr->expr_range)); resolve_expression(expr, context::SET_t_enum::B_TYPE); } From e3c95394717f4ecb008e75d014e76086154843e3 Mon Sep 17 00:00:00 2001 From: Slavomir Kucera Date: Thu, 22 Jul 2021 18:40:31 +0200 Subject: [PATCH 3/9] the first (ugly) version that passes tests --- parser_library/src/context/hlasm_statement.h | 6 +++ parser_library/src/parsing/parser_impl.h | 6 +++ .../src/processing/opencode_provider.cpp | 42 ++++++++++----- .../src/processing/opencode_provider.h | 7 +-- parser_library/src/processing/statement.h | 12 +++++ .../members_statement_provider.cpp | 4 ++ parser_library/src/semantics/collector.cpp | 16 ++++-- parser_library/src/semantics/collector.h | 5 ++ parser_library/src/semantics/statement.h | 24 ++++++--- parser_library/test/context/macro_test.cpp | 54 +++++++++++++++++++ .../test/processing/ca_instr_test.cpp | 17 ------ .../test/workspace/workspace_test.cpp | 6 +-- 12 files changed, 153 insertions(+), 46 deletions(-) diff --git a/parser_library/src/context/hlasm_statement.h b/parser_library/src/context/hlasm_statement.h index 0a3289973..1c01325fe 100644 --- a/parser_library/src/context/hlasm_statement.h +++ b/parser_library/src/context/hlasm_statement.h @@ -16,10 +16,14 @@ #define CONTEXT_HLASM_STATEMENT_H #include +#include #include #include "range.h" +namespace hlasm_plugin::parser_library { +struct diagnostic_op; +} // namespace hlasm_plugin::parser_library namespace hlasm_plugin::parser_library::semantics { struct deferred_statement; } // namespace hlasm_plugin::parser_library::semantics @@ -54,6 +58,8 @@ struct hlasm_statement virtual position statement_position() const = 0; + virtual std::pair diagnostics() const { return {}; } + virtual ~hlasm_statement() = default; protected: diff --git a/parser_library/src/parsing/parser_impl.h b/parser_library/src/parsing/parser_impl.h index 4d73e4f6d..1efc78efd 100644 --- a/parser_library/src/parsing/parser_impl.h +++ b/parser_library/src/parsing/parser_impl.h @@ -53,6 +53,12 @@ class parser_impl : public antlr4::Parser processing::processing_status proc_stat, diagnostic_op_consumer* diagnoser); + void set_diagnoser(diagnostic_op_consumer* diagnoser) + { + diagnoser_ = diagnoser; + err_listener_.diagnoser = diagnoser; + } + semantics::collector& get_collector() { return collector; } protected: diff --git a/parser_library/src/processing/opencode_provider.cpp b/parser_library/src/processing/opencode_provider.cpp index ab60c029f..969a99438 100644 --- a/parser_library/src/processing/opencode_provider.cpp +++ b/parser_library/src/processing/opencode_provider.cpp @@ -185,14 +185,14 @@ void opencode_provider::process_comment() } } -void opencode_provider::generate_continuation_error_messages() const +void opencode_provider::generate_continuation_error_messages(diagnostic_op_consumer* diags) const { auto line_no = m_current_logical_line_source.begin_line; for (const auto& s : m_current_logical_line.segments) { if (s.continuation_error) { - m_diagnoser->add_diagnostic( + diags->add_diagnostic( diagnostic_op::error_E001(range { { line_no, 0 }, { line_no, s.code_offset_utf16 } })); break; @@ -215,7 +215,7 @@ std::shared_ptr opencode_provider::process_looka || std::get(collector.current_instruction().value) == m_ctx->hlasm_ctx->ids().well_known.COPY)) { - const auto& h = prepare_operand_parser(*op_text, *m_ctx->hlasm_ctx, false, {}, op_range, proc_status, true); + const auto& h = prepare_operand_parser(*op_text, *m_ctx->hlasm_ctx, nullptr, {}, op_range, proc_status, true); h.parser->lookahead_operands_and_remarks(); @@ -235,7 +235,8 @@ std::shared_ptr opencode_provider::process_looka std::shared_ptr opencode_provider::process_ordinary(const statement_processor& proc, semantics::collector& collector, const std::optional& op_text, - const range& op_range) + const range& op_range, + diagnostic_op_consumer* diags) { if (proc.kind == processing::processing_kind::ORDINARY && try_trigger_attribute_lookahead( @@ -247,7 +248,7 @@ std::shared_ptr opencode_provider::process_ordin if (op_text) { - const auto& h = prepare_operand_parser(*op_text, *m_ctx->hlasm_ctx, true, {}, op_range, proc_status, false); + const auto& h = prepare_operand_parser(*op_text, *m_ctx->hlasm_ctx, diags, {}, op_range, proc_status, false); const auto& [format, opcode] = proc_status; if (format.occurence == processing::operand_occurence::ABSENT @@ -278,7 +279,7 @@ std::shared_ptr opencode_provider::process_ordin semantics::range_provider tmp_provider(r, ranges, semantics::adjusting_state::MACRO_REPARSE); const auto& h_second = prepare_operand_parser( - to_parse, *m_ctx->hlasm_ctx, true, std::move(tmp_provider), r, proc_status, true); + to_parse, *m_ctx->hlasm_ctx, diags, std::move(tmp_provider), r, proc_status, true); line.operands = std::move(h_second.parser->macro_ops()->list); } @@ -408,6 +409,8 @@ bool opencode_provider::try_running_preprocessor() context::shared_stmt_ptr opencode_provider::get_next(const statement_processor& proc) { const bool lookahead = proc.kind == processing::processing_kind::LOOKAHEAD; + const bool nested = + proc.kind == processing::processing_kind::MACRO || proc.kind == processing::processing_kind::COPY; auto& ph = lookahead ? *m_lookahead_parser : *m_parser; @@ -415,9 +418,12 @@ context::shared_stmt_ptr opencode_provider::get_next(const statement_processor& if (feed_line_res == extract_next_logical_line_result::failed) return nullptr; + auto& collector = ph.parser->get_collector(); + auto* diag_target = nested ? collector.diag_collector() : static_cast(m_diagnoser); + // lookahead may read something that will be removed from the input stream later on if (m_current_logical_line.continuation_error && !lookahead) - generate_continuation_error_messages(); + generate_continuation_error_messages(diag_target); if (feed_line_res != extract_next_logical_line_result::process && is_comment()) { @@ -426,9 +432,8 @@ context::shared_stmt_ptr opencode_provider::get_next(const statement_processor& return nullptr; } - auto& collector = ph.parser->get_collector(); - - collector.prepare_for_next_statement(); + if (!lookahead) + ph.parser->set_diagnoser(diag_target); const auto& [op_text, op_range] = [parser = ph.parser.get(), lookahead]() { if (lookahead) @@ -444,14 +449,23 @@ context::shared_stmt_ptr opencode_provider::get_next(const statement_processor& }(); if (!collector.has_instruction()) - return nullptr; + { + if (lookahead || !nested) + return nullptr; + else + return std::make_shared(range {}, + semantics::label_si { {} }, + semantics::instruction_si { {} }, + semantics::deferred_operands_si { {}, {}, {} }, + std::move(collector.diag_container().diags)); + } m_ctx->hlasm_ctx->set_source_indices(m_current_logical_line_source.begin_offset, m_current_logical_line_source.end_offset, m_current_logical_line_source.end_line); return lookahead ? process_lookahead(proc, collector, op_text, op_range) - : process_ordinary(proc, collector, op_text, op_range); + : process_ordinary(proc, collector, op_text, op_range, diag_target); } bool opencode_provider::finished() const @@ -672,7 +686,7 @@ extract_next_logical_line_result opencode_provider::extract_next_logical_line() const parsing::parser_holder& opencode_provider::prepare_operand_parser(const std::string& text, context::hlasm_context& hlasm_ctx, - bool do_collect_diags, + diagnostic_op_consumer* diags, semantics::range_provider range_prov, range text_range, const processing_status& proc_status, @@ -689,7 +703,7 @@ const parsing::parser_holder& opencode_provider::prepare_operand_parser(const st h.stream->reset(); - h.parser->reinitialize(&hlasm_ctx, std::move(range_prov), proc_status, do_collect_diags ? m_diagnoser : nullptr); + h.parser->reinitialize(&hlasm_ctx, std::move(range_prov), proc_status, diags); h.parser->reset(); diff --git a/parser_library/src/processing/opencode_provider.h b/parser_library/src/processing/opencode_provider.h index 69af1f649..60f117f83 100644 --- a/parser_library/src/processing/opencode_provider.h +++ b/parser_library/src/processing/opencode_provider.h @@ -154,14 +154,14 @@ class opencode_provider final : public statement_provider void generate_aread_highlighting(std::string_view text, size_t line_no) const; bool is_next_line_ictl() const; bool is_next_line_process() const; - void generate_continuation_error_messages() const; + void generate_continuation_error_messages(diagnostic_op_consumer* diags) const; extract_next_logical_line_result extract_next_logical_line_from_ainsert_buffer(); extract_next_logical_line_result extract_next_logical_line_from_copy_buffer(); extract_next_logical_line_result extract_next_logical_line(); void apply_pending_line_changes(); const parsing::parser_holder& prepare_operand_parser(const std::string& text, context::hlasm_context& hlasm_ctx, - bool do_collect_diags, + diagnostic_op_consumer* diag_collector, semantics::range_provider range_prov, range text_range, const processing_status& proc_status, @@ -174,7 +174,8 @@ class opencode_provider final : public statement_provider std::shared_ptr process_ordinary(const statement_processor& proc, semantics::collector& collector, const std::optional& op_text, - const range& op_range); + const range& op_range, + diagnostic_op_consumer* diags); bool fill_copy_buffer_for_aread(); bool try_running_preprocessor(); diff --git a/parser_library/src/processing/statement.h b/parser_library/src/processing/statement.h index 4949b73b3..46392a91c 100644 --- a/parser_library/src/processing/statement.h +++ b/parser_library/src/processing/statement.h @@ -44,9 +44,17 @@ struct resolved_statement_impl : public resolved_statement : base_stmt(std::move(base_stmt)) , status(std::move(status)) {} + resolved_statement_impl(std::shared_ptr base_stmt, + processing_status status, + std::vector&& diags) + : base_stmt(std::move(base_stmt)) + , status(std::move(status)) + , statement_diagnostics(std::make_move_iterator(diags.begin()), std::make_move_iterator(diags.end())) + {} std::shared_ptr base_stmt; processing_status status; + std::vector statement_diagnostics; const semantics::label_si& label_ref() const override { return base_stmt->label_ref(); } const semantics::instruction_si& instruction_ref() const override { return base_stmt->instruction_ref(); } @@ -55,6 +63,10 @@ struct resolved_statement_impl : public resolved_statement const range& stmt_range_ref() const override { return base_stmt->stmt_range_ref(); } const op_code& opcode_ref() const override { return status.second; } processing_format format_ref() const override { return status.first; } + std::pair diagnostics() const override + { + return { statement_diagnostics.data(), statement_diagnostics.data() + statement_diagnostics.size() }; + } }; // statement used for preprocessing of resolved statements diff --git a/parser_library/src/processing/statement_providers/members_statement_provider.cpp b/parser_library/src/processing/statement_providers/members_statement_provider.cpp index 8632dee63..02dd7fb45 100644 --- a/parser_library/src/processing/statement_providers/members_statement_provider.cpp +++ b/parser_library/src/processing/statement_providers/members_statement_provider.cpp @@ -88,6 +88,10 @@ void members_statement_provider::fill_cache( context::statement_cache::cached_statement_t reparsed_stmt; auto def_impl = std::dynamic_pointer_cast(cache.get_base()); + auto diags = def_impl->diagnostics(); + for (auto i = diags.first; i != diags.second; ++i) + reparsed_stmt.diags.push_back(*i); + if (status.first.occurence == operand_occurence::ABSENT || status.first.form == processing_form::UNKNOWN || status.first.form == processing_form::IGNORED) { diff --git a/parser_library/src/semantics/collector.cpp b/parser_library/src/semantics/collector.cpp index 6d4a4f452..523fd1e26 100644 --- a/parser_library/src/semantics/collector.cpp +++ b/parser_library/src/semantics/collector.cpp @@ -178,6 +178,10 @@ void collector::append_operand_field(collector&& c) for (auto& symbol : c.hl_symbols_) hl_symbols_.push_back(std::move(symbol)); + + statement_diagnostics.diags.insert(statement_diagnostics.diags.end(), + std::make_move_iterator(c.statement_diagnostics.diags.begin()), + std::make_move_iterator(c.statement_diagnostics.diags.end())); } const instruction_si& collector::peek_instruction() { return *instr_; } @@ -199,8 +203,11 @@ context::shared_stmt_ptr collector::extract_statement(processing::processing_sta { if (!def_) def_.emplace(instr_->field_range, "", std::vector()); - return std::make_shared( - union_range(lbl_->field_range, def_->field_range), std::move(*lbl_), std::move(*instr_), std::move(*def_)); + return std::make_shared(union_range(lbl_->field_range, def_->field_range), + std::move(*lbl_), + std::move(*instr_), + std::move(*def_), + std::move(statement_diagnostics.diags)); } else { @@ -217,7 +224,8 @@ context::shared_stmt_ptr collector::extract_statement(processing::processing_sta statement_range = union_range(lbl_->field_range, op_->field_range); auto stmt_si = std::make_shared( statement_range, std::move(*lbl_), std::move(*instr_), std::move(*op_), std::move(*rem_)); - return std::make_shared(std::move(stmt_si), std::move(status)); + return std::make_shared( + std::move(stmt_si), std::move(status), std::move(statement_diagnostics.diags)); } } @@ -241,4 +249,6 @@ void collector::prepare_for_next_statement() hl_symbols_.clear(); lsp_symbols_extracted_ = false; hl_symbols_extracted_ = false; + + statement_diagnostics.diags.clear(); } diff --git a/parser_library/src/semantics/collector.h b/parser_library/src/semantics/collector.h index 3ce115314..c53998716 100644 --- a/parser_library/src/semantics/collector.h +++ b/parser_library/src/semantics/collector.h @@ -64,6 +64,9 @@ class collector std::vector extract_hl_symbols(); void prepare_for_next_statement(); + diagnostic_op_consumer* diag_collector() { return &statement_diagnostics; } + diagnostic_op_consumer_container& diag_container() { return statement_diagnostics; } + private: std::optional lbl_; std::optional instr_; @@ -74,6 +77,8 @@ class collector bool lsp_symbols_extracted_; bool hl_symbols_extracted_; + diagnostic_op_consumer_container statement_diagnostics; + void add_operand_remark_hl_symbols(); }; diff --git a/parser_library/src/semantics/statement.h b/parser_library/src/semantics/statement.h index 03a9ce135..9c47bfea1 100644 --- a/parser_library/src/semantics/statement.h +++ b/parser_library/src/semantics/statement.h @@ -16,6 +16,7 @@ #define SEMANTICS_STATEMENT_H #include "context/hlasm_statement.h" +#include "diagnostic.h" #include "statement_fields.h" // this file contains inherited structures from hlasm_statement that are used during the parsing @@ -56,12 +57,16 @@ struct deferred_statement : public core_statement, public context::hlasm_stateme // struct holding deferred semantic information (si) about whole instruction statement, whole logical line struct statement_si_deferred : public deferred_statement { - statement_si_deferred( - range stmt_range, label_si label, instruction_si instruction, deferred_operands_si deferred_operands) + statement_si_deferred(range stmt_range, + label_si label, + instruction_si instruction, + deferred_operands_si deferred_operands, + std::vector&& diags) : stmt_range(std::move(stmt_range)) , label(std::move(label)) , instruction(std::move(instruction)) , deferred_operands(std::move(deferred_operands)) + , statement_diagnostics(std::make_move_iterator(diags.begin()), std::make_move_iterator(diags.end())) {} range stmt_range; @@ -70,10 +75,17 @@ struct statement_si_deferred : public deferred_statement instruction_si instruction; deferred_operands_si deferred_operands; - const label_si& label_ref() const override { return label; }; - const instruction_si& instruction_ref() const override { return instruction; }; - const deferred_operands_si& deferred_ref() const override { return deferred_operands; }; - const range& stmt_range_ref() const override { return stmt_range; }; + std::vector statement_diagnostics; + + const label_si& label_ref() const override { return label; } + const instruction_si& instruction_ref() const override { return instruction; } + const deferred_operands_si& deferred_ref() const override { return deferred_operands; } + const range& stmt_range_ref() const override { return stmt_range; } + + std::pair diagnostics() const override + { + return { statement_diagnostics.data(), statement_diagnostics.data() + statement_diagnostics.size() }; + } }; // struct holding full semantic information (si) about whole instruction statement, whole logical line diff --git a/parser_library/test/context/macro_test.cpp b/parser_library/test/context/macro_test.cpp index 0474d4e3d..e9f3439d0 100644 --- a/parser_library/test/context/macro_test.cpp +++ b/parser_library/test/context/macro_test.cpp @@ -757,3 +757,57 @@ TEST(macro, macro_call_reparse_range) EXPECT_EQ(a.diags()[0].code, "S0003"); EXPECT_EQ(a.diags()[0].diag_range, range({ 6, 16 }, { 6, 16 })); } + +TEST(macro, skip_invalid) +{ + std::string input(R"( + MACRO + MAC + AGO .SKIP + 2 a +.SKIP ANOP + MEND + MAC +)"); + analyzer a(input); + a.analyze(); + a.collect_diags(); + + EXPECT_TRUE(a.diags().empty()); +} + +TEST(macro, invalid_not_invoked) +{ + std::string input(R"( + MACRO + MAC + 2 a + MEND +)"); + analyzer a(input); + a.analyze(); + a.collect_diags(); + + EXPECT_TRUE(a.diags().empty()); +} + +TEST(macro, invalid_invoked) +{ + std::string input(R"( + MACRO + MAC + 2 a + MEND + MAC +)"); + analyzer a(input); + a.analyze(); + a.collect_diags(); + const auto& d = a.diags(); + + const std::array expected = { std::string("S0002"), std::string("E049") }; + std::vector codes; + std::transform(d.begin(), d.end(), std::back_inserter(codes), [](const auto& diag) { return diag.code; }); + + EXPECT_TRUE(std::is_permutation(codes.begin(), codes.end(), expected.begin(), expected.end())); +} diff --git a/parser_library/test/processing/ca_instr_test.cpp b/parser_library/test/processing/ca_instr_test.cpp index 2242c5647..e7c5edf1f 100644 --- a/parser_library/test/processing/ca_instr_test.cpp +++ b/parser_library/test/processing/ca_instr_test.cpp @@ -314,23 +314,6 @@ TEST(AGO, extended_fail) EXPECT_TRUE(ctx.get_var_sym(it3)); } -TEST(AGO, skip_invalid) -{ - std::string input(R"( - MACRO - MAC - AGO .SKIP - 2 a -.SKIP ANOP - MEND -)"); - analyzer a(input); - a.analyze(); - a.collect_diags(); - - EXPECT_TRUE(a.diags().empty()); -} - TEST(AIF, extended) { std::string input(R"( diff --git a/parser_library/test/workspace/workspace_test.cpp b/parser_library/test/workspace/workspace_test.cpp index 246e13237..4d9acc137 100644 --- a/parser_library/test/workspace/workspace_test.cpp +++ b/parser_library/test/workspace/workspace_test.cpp @@ -323,11 +323,11 @@ TEST_F(workspace_test, did_close_file) // 3 files are open // - open codes source1 and source2 with syntax errors using macro ERROR // - macro file lib/ERROR with syntax error - // on first reparse, there should be 3 diagnotics from sources and lib/ERROR file + // on first reparse, there should be 2x2=4 diagnotics from sources and lib/ERROR file ws.did_open_file("source1"); ws.did_open_file("source2"); - ASSERT_EQ(collect_and_get_diags_size(ws, file_manager), (size_t)3); - EXPECT_TRUE(match_strings({ faulty_macro_path, "source2", "source1" })); + ASSERT_EQ(collect_and_get_diags_size(ws, file_manager), (size_t)4); + EXPECT_TRUE(match_strings({ faulty_macro_path, faulty_macro_path, "source2", "source1" })); // when we close source1, only its diagnostics should disapear // macro's and source2's diagnostics should stay as it is still open From 5639203d5a74f74dd57269cfeedbadbb6e5650df Mon Sep 17 00:00:00 2001 From: Slavomir Kucera Date: Fri, 23 Jul 2021 18:08:56 +0200 Subject: [PATCH 4/9] test for invalid macro prototype --- parser_library/test/context/macro_test.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/parser_library/test/context/macro_test.cpp b/parser_library/test/context/macro_test.cpp index e9f3439d0..1252dae59 100644 --- a/parser_library/test/context/macro_test.cpp +++ b/parser_library/test/context/macro_test.cpp @@ -811,3 +811,19 @@ TEST(macro, invalid_invoked) EXPECT_TRUE(std::is_permutation(codes.begin(), codes.end(), expected.begin(), expected.end())); } + +TEST(macro, invalid_prototype) +{ + std::string input = R"( + MACRO +& &LABEL &a=15 + + MEND +)"; + analyzer a(input); + a.analyze(); + a.collect_diags(); + + ASSERT_FALSE(a.diags().empty()); + EXPECT_EQ(a.diags()[0].code, "E071"); +} From db4cc02a6ed901a6d651b1502abd6152c5092bed Mon Sep 17 00:00:00 2001 From: Slavomir Kucera Date: Fri, 23 Jul 2021 18:12:11 +0200 Subject: [PATCH 5/9] enhance statement type checks --- parser_library/src/debugging/debugger.cpp | 9 ++- parser_library/src/diagnostic.cpp | 39 +++++++----- parser_library/src/diagnostic.h | 2 + .../instruction_sets/asm_processor.cpp | 2 +- .../instruction_sets/asm_processor.h | 2 +- .../instruction_sets/ca_processor.cpp | 7 +-- .../instruction_sets/ca_processor.h | 2 +- .../instruction_sets/instruction_processor.h | 2 +- .../low_language_processor.cpp | 2 +- .../instruction_sets/low_language_processor.h | 2 +- .../instruction_sets/mach_processor.cpp | 4 +- .../instruction_sets/mach_processor.h | 2 +- .../instruction_sets/macro_processor.cpp | 7 +-- .../instruction_sets/macro_processor.h | 2 +- .../statement_analyzers/lsp_analyzer.cpp | 63 ++++++++----------- .../statement_analyzers/lsp_analyzer.h | 6 +- .../lookahead_processor.cpp | 25 ++++---- .../macrodef_processor.cpp | 22 ++++--- .../ordinary_processor.cpp | 17 +++-- .../members_statement_provider.cpp | 22 ++++--- .../members_statement_provider.h | 2 +- .../statement_provider.cpp | 8 +-- .../processing/occurence_collector_test.cpp | 4 +- 23 files changed, 134 insertions(+), 119 deletions(-) diff --git a/parser_library/src/debugging/debugger.cpp b/parser_library/src/debugging/debugger.cpp index 3d7217aa2..d4a483fc1 100644 --- a/parser_library/src/debugging/debugger.cpp +++ b/parser_library/src/debugging/debugger.cpp @@ -170,11 +170,16 @@ class debugger::impl final : public processing::statement_analyzer // lookahead and copy/macro definitions) if (proc_kind != processing::processing_kind::ORDINARY) return; + + const auto* resolved_stmt = statement.access_resolved(); + if (!resolved_stmt) + return; + // Continue only for non-empty statements - if (statement.access_resolved()->opcode_ref().value == context::id_storage::empty_id) + if (resolved_stmt->opcode_ref().value == context::id_storage::empty_id) return; - range stmt_range = statement.access_resolved()->stmt_range_ref(); + range stmt_range = resolved_stmt->stmt_range_ref(); bool breakpoint_hit = false; diff --git a/parser_library/src/diagnostic.cpp b/parser_library/src/diagnostic.cpp index 85880ccb1..0aa5c9f76 100644 --- a/parser_library/src/diagnostic.cpp +++ b/parser_library/src/diagnostic.cpp @@ -1676,6 +1676,26 @@ diagnostic_op diagnostic_op::error_E043(const range& range) return diagnostic_op(diagnostic_severity::error, "E043", "Invalid name of variable in macro prototype", range); } +diagnostic_op diagnostic_op::error_E044(const range& range) +{ + return diagnostic_op(diagnostic_severity::error, "E044", "Illegal name field in macro prototype, discarded", range); +} + +diagnostic_op diagnostic_op::error_E045(const std::string& message, const range& range) +{ + return diagnostic_op(diagnostic_severity::error, "E045", "Sequence symbol already defined - " + message, range); +} + +diagnostic_op diagnostic_op::error_E046(const std::string& message, const range& range) +{ + return diagnostic_op(diagnostic_severity::error, "E046", "Missing MEND in " + message, range); +} + +diagnostic_op diagnostic_op::error_E047(const std::string& message, const range& range) +{ + return diagnostic_op(diagnostic_severity::error, "E047", "Lookahead failed, symbol not found - " + message, range); +} + diagnostic_op diagnostic_op::error_E048(const std::string& message, const range& range) { return diagnostic_op( @@ -1798,24 +1818,9 @@ diagnostic_op diagnostic_op::error_E070(const range& range) diagnostic_severity::error, "E070", "Invalid AREAD operand. Use AREAD [NOSTMT|NOPRINT|CLOCKB|CLOCKD].", range); } -diagnostic_op diagnostic_op::error_E044(const range& range) -{ - return diagnostic_op(diagnostic_severity::error, "E044", "Illegal name field in macro prototype, discarded", range); -} - -diagnostic_op diagnostic_op::error_E045(const std::string& message, const range& range) -{ - return diagnostic_op(diagnostic_severity::error, "E045", "Sequence symbol already defined - " + message, range); -} - -diagnostic_op diagnostic_op::error_E046(const std::string& message, const range& range) +diagnostic_op diagnostic_op::error_E071(const range& range) { - return diagnostic_op(diagnostic_severity::error, "E046", "Missing MEND in " + message, range); -} - -diagnostic_op diagnostic_op::error_E047(const std::string& message, const range& range) -{ - return diagnostic_op(diagnostic_severity::error, "E047", "Lookahead failed, symbol not found - " + message, range); + return diagnostic_op(diagnostic_severity::error, "E071", "Macro prototype expected.", range); } diagnostic_op diagnostic_op::warning_W010(const std::string& message, const range& range) diff --git a/parser_library/src/diagnostic.h b/parser_library/src/diagnostic.h index 44b1d9c66..615c1deec 100644 --- a/parser_library/src/diagnostic.h +++ b/parser_library/src/diagnostic.h @@ -570,6 +570,8 @@ struct diagnostic_op static diagnostic_op error_E070(const range& range); + static diagnostic_op error_E071(const range& range); + static diagnostic_op warning_W010(const std::string& message, const range& range); static diagnostic_op warning_W011(const range& range); diff --git a/parser_library/src/processing/instruction_sets/asm_processor.cpp b/parser_library/src/processing/instruction_sets/asm_processor.cpp index ba883f2e7..0e957fbf2 100644 --- a/parser_library/src/processing/instruction_sets/asm_processor.cpp +++ b/parser_library/src/processing/instruction_sets/asm_processor.cpp @@ -501,7 +501,7 @@ asm_processor::asm_processor(analyzing_context ctx, , open_code_(&open_code) {} -void asm_processor::process(context::shared_stmt_ptr stmt) +void asm_processor::process(std::shared_ptr stmt) { auto rebuilt_stmt = preprocess(stmt); diff --git a/parser_library/src/processing/instruction_sets/asm_processor.h b/parser_library/src/processing/instruction_sets/asm_processor.h index 5478a6117..df630a6da 100644 --- a/parser_library/src/processing/instruction_sets/asm_processor.h +++ b/parser_library/src/processing/instruction_sets/asm_processor.h @@ -37,7 +37,7 @@ class asm_processor : public low_language_processor statement_fields_parser& parser, opencode_provider& open_code); - void process(context::shared_stmt_ptr stmt) override; + void process(std::shared_ptr stmt) override; static bool process_copy(const semantics::complete_statement& stmt, analyzing_context ctx, diff --git a/parser_library/src/processing/instruction_sets/ca_processor.cpp b/parser_library/src/processing/instruction_sets/ca_processor.cpp index b1d629608..f615bce95 100644 --- a/parser_library/src/processing/instruction_sets/ca_processor.cpp +++ b/parser_library/src/processing/instruction_sets/ca_processor.cpp @@ -33,11 +33,10 @@ ca_processor::ca_processor(analyzing_context ctx, , open_code_(&open_code) {} -void ca_processor::process(context::shared_stmt_ptr stmt) +void ca_processor::process(std::shared_ptr stmt) { - auto res = stmt->access_resolved(); - auto& func = table_.at(res->opcode_ref().value); - func(*res); + auto& func = table_.at(stmt->opcode_ref().value); + func(*stmt); } ca_processor::process_table_t ca_processor::create_table(context::hlasm_context& h_ctx) diff --git a/parser_library/src/processing/instruction_sets/ca_processor.h b/parser_library/src/processing/instruction_sets/ca_processor.h index bfad590b0..7379c7ae3 100644 --- a/parser_library/src/processing/instruction_sets/ca_processor.h +++ b/parser_library/src/processing/instruction_sets/ca_processor.h @@ -40,7 +40,7 @@ class ca_processor : public instruction_processor processing_state_listener& listener, opencode_provider& open_code); - void process(context::shared_stmt_ptr stmt) override; + void process(std::shared_ptr stmt) override; private: opencode_provider* open_code_; diff --git a/parser_library/src/processing/instruction_sets/instruction_processor.h b/parser_library/src/processing/instruction_sets/instruction_processor.h index 93ed4399f..223275b5e 100644 --- a/parser_library/src/processing/instruction_sets/instruction_processor.h +++ b/parser_library/src/processing/instruction_sets/instruction_processor.h @@ -30,7 +30,7 @@ namespace hlasm_plugin::parser_library::processing { // processing is divided into classes for assembler, conditional assembly, machine, macro instruction processing class instruction_processor : public diagnosable_ctx { - virtual void process(context::shared_stmt_ptr stmt) = 0; + virtual void process(std::shared_ptr stmt) = 0; void collect_diags() const override { collect_diags_from_child(eval_ctx); } diff --git a/parser_library/src/processing/instruction_sets/low_language_processor.cpp b/parser_library/src/processing/instruction_sets/low_language_processor.cpp index 2da375d63..ba2060b33 100644 --- a/parser_library/src/processing/instruction_sets/low_language_processor.cpp +++ b/parser_library/src/processing/instruction_sets/low_language_processor.cpp @@ -31,7 +31,7 @@ low_language_processor::low_language_processor(analyzing_context ctx, , parser(parser) {} -rebuilt_statement low_language_processor::preprocess(context::shared_stmt_ptr statement) +rebuilt_statement low_language_processor::preprocess(std::shared_ptr statement) { auto stmt = std::static_pointer_cast(statement); auto [label, ops] = preprocess_inner(*stmt); diff --git a/parser_library/src/processing/instruction_sets/low_language_processor.h b/parser_library/src/processing/instruction_sets/low_language_processor.h index 31cd7099d..2f9509c06 100644 --- a/parser_library/src/processing/instruction_sets/low_language_processor.h +++ b/parser_library/src/processing/instruction_sets/low_language_processor.h @@ -42,7 +42,7 @@ class low_language_processor : public instruction_processor, public context::loc workspaces::parse_lib_provider& lib_provider, statement_fields_parser& parser); - rebuilt_statement preprocess(context::shared_stmt_ptr stmt); + rebuilt_statement preprocess(std::shared_ptr stmt); // adds dependency and also check for cyclic dependency and adds diagnostics if so template diff --git a/parser_library/src/processing/instruction_sets/mach_processor.cpp b/parser_library/src/processing/instruction_sets/mach_processor.cpp index f7bf7e4ad..5cf4c49ee 100644 --- a/parser_library/src/processing/instruction_sets/mach_processor.cpp +++ b/parser_library/src/processing/instruction_sets/mach_processor.cpp @@ -29,7 +29,7 @@ mach_processor::mach_processor(analyzing_context ctx, : low_language_processor(std::move(ctx), branch_provider, lib_provider, parser) {} -void mach_processor::process(context::shared_stmt_ptr stmt) +void mach_processor::process(std::shared_ptr stmt) { auto rebuilt_stmt = preprocess(stmt); @@ -39,7 +39,7 @@ void mach_processor::process(context::shared_stmt_ptr stmt) return *mnemonic->second.instruction; else return context::instruction::machine_instructions.at(name); - }(*stmt->access_resolved()->opcode_ref().value); + }(*stmt->opcode_ref().value); auto label_name = find_label_symbol(rebuilt_stmt); diff --git a/parser_library/src/processing/instruction_sets/mach_processor.h b/parser_library/src/processing/instruction_sets/mach_processor.h index d41484a08..6fb0f6823 100644 --- a/parser_library/src/processing/instruction_sets/mach_processor.h +++ b/parser_library/src/processing/instruction_sets/mach_processor.h @@ -30,7 +30,7 @@ class mach_processor : public low_language_processor workspaces::parse_lib_provider& lib_provider, statement_fields_parser& parser); - void process(context::shared_stmt_ptr stmt) override; + void process(std::shared_ptr stmt) override; }; } // namespace hlasm_plugin::parser_library::processing diff --git a/parser_library/src/processing/instruction_sets/macro_processor.cpp b/parser_library/src/processing/instruction_sets/macro_processor.cpp index d2fa59747..f26256a7e 100644 --- a/parser_library/src/processing/instruction_sets/macro_processor.cpp +++ b/parser_library/src/processing/instruction_sets/macro_processor.cpp @@ -27,12 +27,11 @@ macro_processor::macro_processor( : instruction_processor(std::move(ctx), branch_provider, lib_provider) {} -void macro_processor::process(context::shared_stmt_ptr stmt) +void macro_processor::process(std::shared_ptr stmt) { - auto args = get_args(*stmt->access_resolved()); + auto args = get_args(*stmt); - hlasm_ctx.enter_macro( - stmt->access_resolved()->opcode_ref().value, std::move(args.name_param), std::move(args.symbolic_params)); + hlasm_ctx.enter_macro(stmt->opcode_ref().value, std::move(args.name_param), std::move(args.symbolic_params)); } bool is_data_def(unsigned char c) diff --git a/parser_library/src/processing/instruction_sets/macro_processor.h b/parser_library/src/processing/instruction_sets/macro_processor.h index 5b32c9c15..e2f401a52 100644 --- a/parser_library/src/processing/instruction_sets/macro_processor.h +++ b/parser_library/src/processing/instruction_sets/macro_processor.h @@ -34,7 +34,7 @@ class macro_processor : public instruction_processor macro_processor( analyzing_context ctx, branching_provider& branch_provider, workspaces::parse_lib_provider& lib_provider); - void process(context::shared_stmt_ptr stmt) override; + void process(std::shared_ptr stmt) override; static context::macro_data_ptr string_to_macrodata(std::string data); diff --git a/parser_library/src/processing/statement_analyzers/lsp_analyzer.cpp b/parser_library/src/processing/statement_analyzers/lsp_analyzer.cpp index 0e3078713..300278add 100644 --- a/parser_library/src/processing/statement_analyzers/lsp_analyzer.cpp +++ b/parser_library/src/processing/statement_analyzers/lsp_analyzer.cpp @@ -39,10 +39,7 @@ lsp_analyzer::lsp_analyzer(context::hlasm_context& hlasm_ctx, lsp::lsp_context& void lsp_analyzer::analyze( const context::hlasm_statement& statement, statement_provider_kind prov_kind, processing_kind proc_kind) { - std::string instr; - if (statement.access_resolved()) - instr = *statement.access_resolved()->opcode_ref().value; - + const auto* resolved_stmt = statement.access_resolved(); switch (proc_kind) { case processing_kind::ORDINARY: @@ -52,17 +49,22 @@ void lsp_analyzer::analyze( { collect_occurences(lsp::occurence_kind::VAR, statement); collect_occurences(lsp::occurence_kind::SEQ, statement); - collect_var_definition(statement); - collect_copy_operands(statement); + if (resolved_stmt) + { + collect_var_definition(*resolved_stmt); + collect_copy_operands(*resolved_stmt); + } } break; case processing_kind::MACRO: - update_macro_nest(statement); + if (resolved_stmt) + update_macro_nest(*resolved_stmt); if (macro_nest_ > 1) break; // Do not collect occurences in nested macros to avoid collecting occurences multiple times collect_occurences(lsp::occurence_kind::VAR, statement); collect_occurences(lsp::occurence_kind::SEQ, statement); - collect_copy_operands(statement); + if (resolved_stmt) + collect_copy_operands(*resolved_stmt); break; default: break; @@ -139,17 +141,14 @@ void lsp_analyzer::collect_occurences(lsp::occurence_kind kind, const context::h { occurence_collector collector(kind, hlasm_ctx_, stmt_occurences_); - if (auto def_stmt = statement.access_deferred(); def_stmt) + if (auto def_stmt = statement.access_deferred()) { collect_occurence(def_stmt->label_ref(), collector); collect_occurence(def_stmt->instruction_ref(), collector); collect_occurence(def_stmt->deferred_ref(), collector); } - else + else if (auto res_stmt = statement.access_resolved()) { - auto res_stmt = statement.access_resolved(); - assert(res_stmt); - collect_occurence(res_stmt->label_ref(), collector); collect_occurence(res_stmt->instruction_ref(), collector); collect_occurence(res_stmt->operands_ref(), collector); @@ -239,31 +238,24 @@ bool lsp_analyzer::is_SET(const processing::resolved_statement& statement, conte return false; } -void lsp_analyzer::collect_var_definition(const context::hlasm_statement& statement) +void lsp_analyzer::collect_var_definition(const processing::resolved_statement& statement) { - auto res_stmt = statement.access_resolved(); - if (!res_stmt) - return; - bool global; context::SET_t_enum type; - if (is_SET(*res_stmt, type)) - collect_SET_defs(*res_stmt, type); - else if (is_LCL_GBL(*res_stmt, type, global)) - collect_LCL_GBL_defs(*res_stmt, type, global); + if (is_SET(statement, type)) + collect_SET_defs(statement, type); + else if (is_LCL_GBL(statement, type, global)) + collect_LCL_GBL_defs(statement, type, global); } -void lsp_analyzer::collect_copy_operands(const context::hlasm_statement& statement) +void lsp_analyzer::collect_copy_operands(const processing::resolved_statement& statement) { - auto res_stmt = statement.access_resolved(); - if (!res_stmt) - return; - - if (res_stmt->opcode_ref().value == hlasm_ctx_.ids().well_known.COPY && res_stmt->operands_ref().value.size() == 1 - && res_stmt->operands_ref().value.front()->access_asm()) + const auto& opcode = statement.opcode_ref().value; + const auto& operands = statement.operands_ref().value; + if (opcode == hlasm_ctx_.ids().well_known.COPY && operands.size() == 1 && operands.front()->access_asm()) { auto sym_expr = dynamic_cast( - res_stmt->operands_ref().value.front()->access_asm()->access_expr()->expression.get()); + operands.front()->access_asm()->access_expr()->expression.get()); if (sym_expr) add_copy_operand(sym_expr->value, sym_expr->get_range()); @@ -327,15 +319,12 @@ void lsp_analyzer::add_copy_operand(context::id_index name, const range& operand stmt_occurences_.emplace_back(lsp::occurence_kind::COPY_OP, name, operand_range); } -void lsp_analyzer::update_macro_nest(const context::hlasm_statement& statement) +void lsp_analyzer::update_macro_nest(const processing::resolved_statement& statement) { - auto res_stmt = statement.access_resolved(); - if (!res_stmt) - return; - - if (res_stmt->opcode_ref().value == hlasm_ctx_.ids().well_known.MACRO) + const auto& opcode = statement.opcode_ref().value; + if (opcode == hlasm_ctx_.ids().well_known.MACRO) macro_nest_++; - else if (res_stmt->opcode_ref().value == hlasm_ctx_.ids().well_known.MEND) + else if (opcode == hlasm_ctx_.ids().well_known.MEND) macro_nest_--; } diff --git a/parser_library/src/processing/statement_analyzers/lsp_analyzer.h b/parser_library/src/processing/statement_analyzers/lsp_analyzer.h index 569edbf52..24398f96b 100644 --- a/parser_library/src/processing/statement_analyzers/lsp_analyzer.h +++ b/parser_library/src/processing/statement_analyzers/lsp_analyzer.h @@ -63,8 +63,8 @@ class lsp_analyzer : public statement_analyzer void collect_occurence(const semantics::operands_si& operands, occurence_collector& collector); void collect_occurence(const semantics::deferred_operands_si& operands, occurence_collector& collector); - void collect_var_definition(const context::hlasm_statement& statement); - void collect_copy_operands(const context::hlasm_statement& statement); + void collect_var_definition(const processing::resolved_statement& statement); + void collect_copy_operands(const processing::resolved_statement& statement); void collect_SET_defs(const processing::resolved_statement& statement, context::SET_t_enum type); void collect_LCL_GBL_defs(const processing::resolved_statement& statement, context::SET_t_enum type, bool global); @@ -72,7 +72,7 @@ class lsp_analyzer : public statement_analyzer void add_copy_operand(context::id_index name, const range& operand_range); - void update_macro_nest(const context::hlasm_statement& statement); + void update_macro_nest(const processing::resolved_statement& statement); struct LCL_GBL_instr { diff --git a/parser_library/src/processing/statement_processors/lookahead_processor.cpp b/parser_library/src/processing/statement_processors/lookahead_processor.cpp index f1950dce3..48f84f850 100644 --- a/parser_library/src/processing/statement_processors/lookahead_processor.cpp +++ b/parser_library/src/processing/statement_processors/lookahead_processor.cpp @@ -51,19 +51,20 @@ void lookahead_processor::process_statement(context::shared_stmt_ptr statement) find_ord(static_cast(*statement)); } - auto resolved = statement->access_resolved(); - - if (resolved->opcode_ref().value == macro_id) - { - process_MACRO(); - } - else if (resolved->opcode_ref().value == mend_id) + if (auto resolved = statement->access_resolved()) { - process_MEND(); - } - else if (macro_nest_ == 0 && resolved->opcode_ref().value == copy_id) - { - process_COPY(*resolved); + if (resolved->opcode_ref().value == macro_id) + { + process_MACRO(); + } + else if (resolved->opcode_ref().value == mend_id) + { + process_MEND(); + } + else if (macro_nest_ == 0 && resolved->opcode_ref().value == copy_id) + { + process_COPY(*resolved); + } } } diff --git a/parser_library/src/processing/statement_processors/macrodef_processor.cpp b/parser_library/src/processing/statement_processors/macrodef_processor.cpp index 3c24f7b4e..ff771276f 100644 --- a/parser_library/src/processing/statement_processors/macrodef_processor.cpp +++ b/parser_library/src/processing/statement_processors/macrodef_processor.cpp @@ -121,7 +121,7 @@ processing_status macrodef_processor::get_macro_processing_status( return std::make_pair(format, op_code(code.opcode, context::instruction_type::CA)); } - else if (code.opcode == hlasm_ctx.ids().add("COPY")) + else if (code.opcode == hlasm_ctx.ids().well_known.COPY) { processing_format format(processing_kind::MACRO, processing_form::ASM, operand_occurence::PRESENT); @@ -149,12 +149,12 @@ void macrodef_processor::process_statement(const context::hlasm_statement& state omit_next_ = false; + auto res_stmt = statement.access_resolved(); + if (expecting_MACRO_) { result_.definition_location = hlasm_ctx.processing_stack().back().proc_location; - auto res_stmt = statement.access_resolved(); - if (!res_stmt || res_stmt->opcode_ref().value != macro_id) { range r = res_stmt ? res_stmt->stmt_range_ref() : range(statement.statement_position()); @@ -168,9 +168,15 @@ void macrodef_processor::process_statement(const context::hlasm_statement& state } else if (expecting_prototype_) { + if (!res_stmt) + { + range r = res_stmt ? res_stmt->stmt_range_ref() : range(statement.statement_position()); + add_diagnostic(diagnostic_op::error_E071(r)); + result_.invalid = true; + return; + } result_.invalid = false; - assert(statement.access_resolved()); - process_prototype(*statement.access_resolved()); + process_prototype(*res_stmt); expecting_prototype_ = false; } else @@ -178,7 +184,7 @@ void macrodef_processor::process_statement(const context::hlasm_statement& state if (hlasm_ctx.current_copy_stack().size() - initial_copy_nest_ == 0) curr_outer_position_ = statement.statement_position(); - if (auto res_stmt = statement.access_resolved()) + if (res_stmt) { process_sequence_symbol(res_stmt->label_ref()); @@ -192,10 +198,8 @@ void macrodef_processor::process_statement(const context::hlasm_statement& state { process_sequence_symbol(def_stmt->label_ref()); } - else - assert(false); - ++curr_line_; + ++curr_line_; // TODO: What are we doing here??? } } diff --git a/parser_library/src/processing/statement_processors/ordinary_processor.cpp b/parser_library/src/processing/statement_processors/ordinary_processor.cpp index 75fcb0ef6..225537f73 100644 --- a/parser_library/src/processing/statement_processors/ordinary_processor.cpp +++ b/parser_library/src/processing/statement_processors/ordinary_processor.cpp @@ -71,19 +71,24 @@ processing_status ordinary_processor::get_processing_status(const semantics::ins return *status; } -void ordinary_processor::process_statement(context::shared_stmt_ptr statement) +void ordinary_processor::process_statement(context::shared_stmt_ptr s) { - assert(statement->kind == context::statement_kind::RESOLVED); + assert(s->kind == context::statement_kind::RESOLVED); // TODO: not sure why in principle unresolved cannot arrive + // here + if (s->kind != context::statement_kind::RESOLVED) + return; - bool fatal = check_fatals(range(statement->statement_position())); + bool fatal = check_fatals(range(s->statement_position())); if (fatal) return; - switch (statement->access_resolved()->opcode_ref().type) + auto statement = std::static_pointer_cast(std::move(s)); + + switch (statement->opcode_ref().type) { case context::instruction_type::UNDEF: - add_diagnostic(diagnostic_op::error_E049(*statement->access_resolved()->opcode_ref().value, - statement->access_resolved()->instruction_ref().field_range)); + add_diagnostic( + diagnostic_op::error_E049(*statement->opcode_ref().value, statement->instruction_ref().field_range)); return; case context::instruction_type::CA: ca_proc_.process(std::move(statement)); diff --git a/parser_library/src/processing/statement_providers/members_statement_provider.cpp b/parser_library/src/processing/statement_providers/members_statement_provider.cpp index 02dd7fb45..abb6226be 100644 --- a/parser_library/src/processing/statement_providers/members_statement_provider.cpp +++ b/parser_library/src/processing/statement_providers/members_statement_provider.cpp @@ -40,9 +40,14 @@ context::shared_stmt_ptr members_statement_provider::get_next(const statement_pr if (!cache) return nullptr; - if (processor.kind == processing_kind::ORDINARY - && try_trigger_attribute_lookahead(retrieve_instruction(*cache), { *ctx.hlasm_ctx, lib_provider }, listener)) - return nullptr; + if (processor.kind == processing_kind::ORDINARY) + { + if (const auto* instr = retrieve_instruction(*cache)) + { + if (try_trigger_attribute_lookahead(*instr, { *ctx.hlasm_ctx, lib_provider }, listener)) + return nullptr; + } + } context::shared_stmt_ptr stmt; @@ -55,7 +60,6 @@ context::shared_stmt_ptr members_statement_provider::get_next(const statement_pr stmt = preprocess_deferred(processor, *cache); break; default: - assert(false); break; } @@ -67,18 +71,17 @@ context::shared_stmt_ptr members_statement_provider::get_next(const statement_pr return stmt; } -const semantics::instruction_si& members_statement_provider::retrieve_instruction( +const semantics::instruction_si* members_statement_provider::retrieve_instruction( const context::statement_cache& cache) const { switch (cache.get_base()->kind) { case context::statement_kind::RESOLVED: - return cache.get_base()->access_resolved()->instruction_ref(); + return &cache.get_base()->access_resolved()->instruction_ref(); case context::statement_kind::DEFERRED: - return cache.get_base()->access_deferred()->instruction_ref(); + return &cache.get_base()->access_deferred()->instruction_ref(); default: - assert(false); - throw std::runtime_error("unexpected statement_kind enum value"); + return nullptr; } } @@ -88,6 +91,7 @@ void members_statement_provider::fill_cache( context::statement_cache::cached_statement_t reparsed_stmt; auto def_impl = std::dynamic_pointer_cast(cache.get_base()); + // TODO: what if it fails? auto diags = def_impl->diagnostics(); for (auto i = diags.first; i != diags.second; ++i) reparsed_stmt.diags.push_back(*i); diff --git a/parser_library/src/processing/statement_providers/members_statement_provider.h b/parser_library/src/processing/statement_providers/members_statement_provider.h index 84c1774c4..e243e6084 100644 --- a/parser_library/src/processing/statement_providers/members_statement_provider.h +++ b/parser_library/src/processing/statement_providers/members_statement_provider.h @@ -46,7 +46,7 @@ class members_statement_provider : public statement_provider virtual context::statement_cache* get_next() = 0; private: - const semantics::instruction_si& retrieve_instruction(const context::statement_cache& cache) const; + const semantics::instruction_si* retrieve_instruction(const context::statement_cache& cache) const; void fill_cache(context::statement_cache& cache, const semantics::deferred_statement& def_stmt, diff --git a/parser_library/src/processing/statement_providers/statement_provider.cpp b/parser_library/src/processing/statement_providers/statement_provider.cpp index 8f07f78b6..2a4107c65 100644 --- a/parser_library/src/processing/statement_providers/statement_provider.cpp +++ b/parser_library/src/processing/statement_providers/statement_provider.cpp @@ -44,18 +44,18 @@ bool statement_provider::try_trigger_attribute_lookahead(const context::hlasm_st const semantics::label_si* label; std::set references; - if (statement.kind == context::statement_kind::DEFERRED) + if (auto def_stmt = statement.access_deferred()) { - auto def_stmt = statement.access_deferred(); label = &def_stmt->label_ref(); } - else + else if (auto res_stmt = statement.access_resolved()) { - auto res_stmt = statement.access_resolved(); label = &res_stmt->label_ref(); references.merge(process_operands(res_stmt->operands_ref(), eval_ctx)); } + else + return false; references.merge(process_label(*label, eval_ctx)); diff --git a/parser_library/test/processing/occurence_collector_test.cpp b/parser_library/test/processing/occurence_collector_test.cpp index 9e91977af..234812810 100644 --- a/parser_library/test/processing/occurence_collector_test.cpp +++ b/parser_library/test/processing/occurence_collector_test.cpp @@ -39,8 +39,10 @@ struct operand_occurence_analyzer_mock : public processing::statement_analyzer processing::statement_provider_kind, processing::processing_kind) override { + const auto* res_stmt = statement.access_resolved(); + ASSERT_TRUE(res_stmt); processing::occurence_collector collector(occ_kind, *a.context().hlasm_ctx, st); - const auto& operands = statement.access_resolved()->operands_ref().value; + const auto& operands = res_stmt->operands_ref().value; std::for_each(operands.begin(), operands.end(), [&](const auto& op) { op->apply(collector); }); } From 47eb6292d8fc226d3dfb2d6c03bb2b18bad773dd Mon Sep 17 00:00:00 2001 From: Slavomir Kucera Date: Mon, 26 Jul 2021 11:20:21 +0200 Subject: [PATCH 6/9] fixes eclipse/che-che4z-lsp-for-hlasm#144 --- parser_library/src/context/hlasm_statement.h | 6 ++- parser_library/src/processing/CMakeLists.txt | 2 + .../src/processing/error_statement.cpp | 28 +++++++++++++ .../src/processing/error_statement.h | 42 +++++++++++++++++++ .../postponed_statement_impl.h | 2 + .../src/processing/opencode_provider.cpp | 9 ++-- parser_library/src/processing/statement.h | 6 +++ .../ordinary_processor.cpp | 9 ++-- .../members_statement_provider.cpp | 5 +++ 9 files changed, 98 insertions(+), 11 deletions(-) create mode 100644 parser_library/src/processing/error_statement.cpp create mode 100644 parser_library/src/processing/error_statement.h diff --git a/parser_library/src/context/hlasm_statement.h b/parser_library/src/context/hlasm_statement.h index 1c01325fe..86253e79d 100644 --- a/parser_library/src/context/hlasm_statement.h +++ b/parser_library/src/context/hlasm_statement.h @@ -28,6 +28,7 @@ namespace hlasm_plugin::parser_library::semantics { struct deferred_statement; } // namespace hlasm_plugin::parser_library::semantics namespace hlasm_plugin::parser_library::processing { +class error_statement; struct resolved_statement; } // namespace hlasm_plugin::parser_library::processing @@ -42,7 +43,8 @@ using statement_block = std::vector; enum class statement_kind { RESOLVED, - DEFERRED + DEFERRED, + ERROR, }; // abstract structure representing general HLASM statement @@ -58,7 +60,7 @@ struct hlasm_statement virtual position statement_position() const = 0; - virtual std::pair diagnostics() const { return {}; } + virtual std::pair diagnostics() const = 0; virtual ~hlasm_statement() = default; diff --git a/parser_library/src/processing/CMakeLists.txt b/parser_library/src/processing/CMakeLists.txt index 6e02b35c1..e73b15d0a 100644 --- a/parser_library/src/processing/CMakeLists.txt +++ b/parser_library/src/processing/CMakeLists.txt @@ -15,6 +15,8 @@ target_sources(parser_library PRIVATE context_manager.cpp context_manager.h db2_preprocessor.cpp + error_statement.cpp + error_statement.h op_code.h opencode_provider.cpp opencode_provider.h diff --git a/parser_library/src/processing/error_statement.cpp b/parser_library/src/processing/error_statement.cpp new file mode 100644 index 000000000..4ac0ade84 --- /dev/null +++ b/parser_library/src/processing/error_statement.cpp @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2021 Broadcom. + * The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. + * + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Broadcom, Inc. - initial API and implementation + */ + +#include "error_statement.h" + + +namespace hlasm_plugin::parser_library::processing { + + +position error_statement::statement_position() const { return m_range.start; } + +std::pair error_statement::diagnostics() const +{ + return { m_errors.data(), m_errors.data() + m_errors.size() }; +} + +} // namespace hlasm_plugin::parser_library::processing \ No newline at end of file diff --git a/parser_library/src/processing/error_statement.h b/parser_library/src/processing/error_statement.h new file mode 100644 index 000000000..557ea0bb8 --- /dev/null +++ b/parser_library/src/processing/error_statement.h @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2021 Broadcom. + * The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. + * + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Broadcom, Inc. - initial API and implementation + */ + +#ifndef HLASMPARSER_PARSERLIBRARY_PROCESSING_ERROR_STATEMENT_H +#define HLASMPARSER_PARSERLIBRARY_PROCESSING_ERROR_STATEMENT_H + +#include "context/hlasm_statement.h" +#include "diagnostic.h" + +namespace hlasm_plugin::parser_library::processing { + +class error_statement : public context::hlasm_statement +{ + std::vector m_errors; + range m_range; + +public: + error_statement(range r, std::vector&& diags) + : hlasm_statement(context::statement_kind::ERROR) + , m_errors(std::make_move_iterator(diags.begin()), std::make_move_iterator(diags.end())) + , m_range(r) + {} + + position statement_position() const override; + + std::pair diagnostics() const override; +}; + +} // namespace hlasm_plugin::parser_library::processing + +#endif // HLASMPARSER_PARSERLIBRARY_PROCESSING_ERROR_STATEMENT_H \ No newline at end of file diff --git a/parser_library/src/processing/instruction_sets/postponed_statement_impl.h b/parser_library/src/processing/instruction_sets/postponed_statement_impl.h index e53b2b508..4b23b8b8d 100644 --- a/parser_library/src/processing/instruction_sets/postponed_statement_impl.h +++ b/parser_library/src/processing/instruction_sets/postponed_statement_impl.h @@ -42,6 +42,8 @@ struct postponed_statement_impl : public context::postponed_statement, public re processing_format format_ref() const override { return stmt.format_ref(); } const context::processing_stack_t& location_stack() const override { return stmt_location_stack; } + + std::pair diagnostics() const override { return stmt.diagnostics(); } }; } // namespace hlasm_plugin::parser_library::processing diff --git a/parser_library/src/processing/opencode_provider.cpp b/parser_library/src/processing/opencode_provider.cpp index 969a99438..9f34b9e62 100644 --- a/parser_library/src/processing/opencode_provider.cpp +++ b/parser_library/src/processing/opencode_provider.cpp @@ -19,6 +19,7 @@ #include "lexing/token_stream.h" #include "parsing/error_strategy.h" #include "parsing/parser_impl.h" +#include "processing/error_statement.h" #include "semantics/collector.h" #include "semantics/range_provider.h" @@ -450,13 +451,11 @@ context::shared_stmt_ptr opencode_provider::get_next(const statement_processor& if (!collector.has_instruction()) { - if (lookahead || !nested) + if (lookahead) return nullptr; else - return std::make_shared(range {}, - semantics::label_si { {} }, - semantics::instruction_si { {} }, - semantics::deferred_operands_si { {}, {}, {} }, + return std::make_shared( + range(position(m_current_logical_line_source.begin_line, 0)), std::move(collector.diag_container().diags)); } diff --git a/parser_library/src/processing/statement.h b/parser_library/src/processing/statement.h index 46392a91c..d2b1b8025 100644 --- a/parser_library/src/processing/statement.h +++ b/parser_library/src/processing/statement.h @@ -97,6 +97,12 @@ struct rebuilt_statement : public resolved_statement const range& stmt_range_ref() const override { return base_stmt->stmt_range_ref(); } const op_code& opcode_ref() const override { return base_stmt->opcode_ref(); } processing_format format_ref() const override { return base_stmt->format_ref(); } + + + std::pair diagnostics() const override + { + return base_stmt->diagnostics(); + } }; } // namespace hlasm_plugin::parser_library::processing diff --git a/parser_library/src/processing/statement_processors/ordinary_processor.cpp b/parser_library/src/processing/statement_processors/ordinary_processor.cpp index 225537f73..f0fa7e5c5 100644 --- a/parser_library/src/processing/statement_processors/ordinary_processor.cpp +++ b/parser_library/src/processing/statement_processors/ordinary_processor.cpp @@ -73,14 +73,15 @@ processing_status ordinary_processor::get_processing_status(const semantics::ins void ordinary_processor::process_statement(context::shared_stmt_ptr s) { - assert(s->kind == context::statement_kind::RESOLVED); // TODO: not sure why in principle unresolved cannot arrive - // here - if (s->kind != context::statement_kind::RESOLVED) - return; + auto diags = s->diagnostics(); + for (auto it = diags.first; it != diags.second; ++it) + add_diagnostic(*it); bool fatal = check_fatals(range(s->statement_position())); if (fatal) return; + if (s->kind != context::statement_kind::RESOLVED) + return; auto statement = std::static_pointer_cast(std::move(s)); diff --git a/parser_library/src/processing/statement_providers/members_statement_provider.cpp b/parser_library/src/processing/statement_providers/members_statement_provider.cpp index abb6226be..6e7f4b949 100644 --- a/parser_library/src/processing/statement_providers/members_statement_provider.cpp +++ b/parser_library/src/processing/statement_providers/members_statement_provider.cpp @@ -59,6 +59,9 @@ context::shared_stmt_ptr members_statement_provider::get_next(const statement_pr case context::statement_kind::DEFERRED: stmt = preprocess_deferred(processor, *cache); break; + case context::statement_kind::ERROR: + stmt = cache->get_base(); + break; default: break; } @@ -80,6 +83,8 @@ const semantics::instruction_si* members_statement_provider::retrieve_instructio return &cache.get_base()->access_resolved()->instruction_ref(); case context::statement_kind::DEFERRED: return &cache.get_base()->access_deferred()->instruction_ref(); + case context::statement_kind::ERROR: + return nullptr; default: return nullptr; } From 066f642a44303a498f7a122ff61cb5da9a77b3ff Mon Sep 17 00:00:00 2001 From: Slavomir Kucera Date: Mon, 26 Jul 2021 12:20:03 +0200 Subject: [PATCH 7/9] fix error reporting of simple lexing errors in macros --- .../src/processing/opencode_provider.cpp | 19 +++++++++++++------ parser_library/test/context/macro_test.cpp | 17 +++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/parser_library/src/processing/opencode_provider.cpp b/parser_library/src/processing/opencode_provider.cpp index 9f34b9e62..b2a9a75b3 100644 --- a/parser_library/src/processing/opencode_provider.cpp +++ b/parser_library/src/processing/opencode_provider.cpp @@ -410,8 +410,6 @@ bool opencode_provider::try_running_preprocessor() context::shared_stmt_ptr opencode_provider::get_next(const statement_processor& proc) { const bool lookahead = proc.kind == processing::processing_kind::LOOKAHEAD; - const bool nested = - proc.kind == processing::processing_kind::MACRO || proc.kind == processing::processing_kind::COPY; auto& ph = lookahead ? *m_lookahead_parser : *m_parser; @@ -420,11 +418,20 @@ context::shared_stmt_ptr opencode_provider::get_next(const statement_processor& return nullptr; auto& collector = ph.parser->get_collector(); - auto* diag_target = nested ? collector.diag_collector() : static_cast(m_diagnoser); + auto* diag_target = + proc.kind == processing::processing_kind::MACRO || proc.kind == processing::processing_kind::COPY + ? collector.diag_collector() + : static_cast(m_diagnoser); - // lookahead may read something that will be removed from the input stream later on - if (m_current_logical_line.continuation_error && !lookahead) - generate_continuation_error_messages(diag_target); + if (m_current_logical_line.continuation_error) + { + // continuation errors must be reported immediatelly + if (proc.kind == processing::processing_kind::MACRO) + generate_continuation_error_messages(static_cast(m_diagnoser)); + // lookahead may read something that will be removed from the input stream later on + else if (!lookahead) + generate_continuation_error_messages(diag_target); + } if (feed_line_res != extract_next_logical_line_result::process && is_comment()) { diff --git a/parser_library/test/context/macro_test.cpp b/parser_library/test/context/macro_test.cpp index 1252dae59..be49a8aac 100644 --- a/parser_library/test/context/macro_test.cpp +++ b/parser_library/test/context/macro_test.cpp @@ -827,3 +827,20 @@ TEST(macro, invalid_prototype) ASSERT_FALSE(a.diags().empty()); EXPECT_EQ(a.diags()[0].code, "E071"); } + +TEST(macro, invalid_continuation) +{ + std::string input = R"( + MACRO + MAC + SAM31 X +A + MEND +)"; + analyzer a(input); + a.analyze(); + a.collect_diags(); + + ASSERT_FALSE(a.diags().empty()); + EXPECT_EQ(a.diags()[0].code, "E001"); +} From 463193a55f63d523d5143559e608e6e26223f268 Mon Sep 17 00:00:00 2001 From: Slavomir Kucera Date: Mon, 26 Jul 2021 18:20:31 +0200 Subject: [PATCH 8/9] fix: Language server crashes on missing MEND. fix: Language server crashes on an unexpected statement in the lookahead mode. --- .../lookahead_processor.cpp | 11 +++--- .../macrodef_processor.cpp | 9 +++++ parser_library/test/context/macro_test.cpp | 37 +++++++++++++------ 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/parser_library/src/processing/statement_processors/lookahead_processor.cpp b/parser_library/src/processing/statement_processors/lookahead_processor.cpp index 48f84f850..8492e5245 100644 --- a/parser_library/src/processing/statement_processors/lookahead_processor.cpp +++ b/parser_library/src/processing/statement_processors/lookahead_processor.cpp @@ -45,14 +45,13 @@ processing_status lookahead_processor::get_processing_status(const semantics::in void lookahead_processor::process_statement(context::shared_stmt_ptr statement) { - if (macro_nest_ == 0) - { - find_seq(static_cast(*statement)); - find_ord(static_cast(*statement)); - } - if (auto resolved = statement->access_resolved()) { + if (macro_nest_ == 0) + { + find_seq(*resolved); + find_ord(*resolved); + } if (resolved->opcode_ref().value == macro_id) { process_MACRO(); diff --git a/parser_library/src/processing/statement_processors/macrodef_processor.cpp b/parser_library/src/processing/statement_processors/macrodef_processor.cpp index ff771276f..2b00ecc9e 100644 --- a/parser_library/src/processing/statement_processors/macrodef_processor.cpp +++ b/parser_library/src/processing/statement_processors/macrodef_processor.cpp @@ -94,6 +94,15 @@ void macrodef_processor::end_processing() hlasm_ctx.pop_statement_processing(); + // cleanup empty file scopes + for (auto& scope : result_.file_scopes) + { + scope.second.erase(std::remove_if(scope.second.begin(), + scope.second.end(), + [](const auto& slice) { return slice.begin_statement == slice.end_statement; }), + scope.second.end()); + } + listener_.finish_macro_definition(std::move(result_)); finished_flag_ = true; diff --git a/parser_library/test/context/macro_test.cpp b/parser_library/test/context/macro_test.cpp index be49a8aac..ca0a2018f 100644 --- a/parser_library/test/context/macro_test.cpp +++ b/parser_library/test/context/macro_test.cpp @@ -803,13 +803,8 @@ TEST(macro, invalid_invoked) analyzer a(input); a.analyze(); a.collect_diags(); - const auto& d = a.diags(); - - const std::array expected = { std::string("S0002"), std::string("E049") }; - std::vector codes; - std::transform(d.begin(), d.end(), std::back_inserter(codes), [](const auto& diag) { return diag.code; }); - EXPECT_TRUE(std::is_permutation(codes.begin(), codes.end(), expected.begin(), expected.end())); + EXPECT_TRUE(matches_message_codes(a.diags(), { "S0002", "E049" })); } TEST(macro, invalid_prototype) @@ -823,24 +818,44 @@ TEST(macro, invalid_prototype) analyzer a(input); a.analyze(); a.collect_diags(); + const auto& d = a.diags(); - ASSERT_FALSE(a.diags().empty()); - EXPECT_EQ(a.diags()[0].code, "E071"); + EXPECT_NE(std::find_if(d.begin(), d.end(), [](const auto& diag) { return diag.code == "E071"; }), d.end()); } -TEST(macro, invalid_continuation) +TEST(macro, early_macro_errors) { std::string input = R"( MACRO MAC SAM31 X A +AAA MEND )"; analyzer a(input); a.analyze(); a.collect_diags(); - ASSERT_FALSE(a.diags().empty()); - EXPECT_EQ(a.diags()[0].code, "E001"); + EXPECT_TRUE(matches_message_codes(a.diags(), { "E001", "S0003" })); +} + +TEST(macro, missing_mend) +{ + std::string input = R"( + MACRO + MACO + SAM31 X + X + + MACRO + MAC + SAM31 + MEND +)"; + analyzer a(input); + a.analyze(); + a.collect_diags(); + + EXPECT_TRUE(matches_message_codes(a.diags(), { "E001", "E046" })); } From 8738d2babc8b4b4108ce24038721bafc85e39c7c Mon Sep 17 00:00:00 2001 From: Slavomir Kucera Date: Mon, 26 Jul 2021 18:21:08 +0200 Subject: [PATCH 9/9] tweak error reporting --- .../src/processing/opencode_provider.cpp | 25 +++++++++++++------ .../members_statement_provider.cpp | 12 +++++---- parser_library/test/common_testing.h | 11 ++++++++ parser_library/test/processing/copy_test.cpp | 21 ++++++++++++++++ .../test/workspace/workspace_test.cpp | 12 ++++----- 5 files changed, 62 insertions(+), 19 deletions(-) diff --git a/parser_library/src/processing/opencode_provider.cpp b/parser_library/src/processing/opencode_provider.cpp index b2a9a75b3..0ec3d224d 100644 --- a/parser_library/src/processing/opencode_provider.cpp +++ b/parser_library/src/processing/opencode_provider.cpp @@ -409,7 +409,10 @@ bool opencode_provider::try_running_preprocessor() context::shared_stmt_ptr opencode_provider::get_next(const statement_processor& proc) { - const bool lookahead = proc.kind == processing::processing_kind::LOOKAHEAD; + using processing_kind = processing::processing_kind; + + const bool lookahead = proc.kind == processing_kind::LOOKAHEAD; + const bool nested = proc.kind == processing_kind::MACRO || proc.kind == processing_kind::COPY; auto& ph = lookahead ? *m_lookahead_parser : *m_parser; @@ -418,15 +421,12 @@ context::shared_stmt_ptr opencode_provider::get_next(const statement_processor& return nullptr; auto& collector = ph.parser->get_collector(); - auto* diag_target = - proc.kind == processing::processing_kind::MACRO || proc.kind == processing::processing_kind::COPY - ? collector.diag_collector() - : static_cast(m_diagnoser); + auto* diag_target = nested ? collector.diag_collector() : static_cast(m_diagnoser); if (m_current_logical_line.continuation_error) { - // continuation errors must be reported immediatelly - if (proc.kind == processing::processing_kind::MACRO) + // report continuation errors immediately + if (proc.kind == processing_kind::MACRO) generate_continuation_error_messages(static_cast(m_diagnoser)); // lookahead may read something that will be removed from the input stream later on else if (!lookahead) @@ -458,7 +458,16 @@ context::shared_stmt_ptr opencode_provider::get_next(const statement_processor& if (!collector.has_instruction()) { - if (lookahead) + if (proc.kind == processing_kind::MACRO) + { + // these kinds of errors are reported right away, + for (auto& diag : collector.diag_container().diags) + m_diagnoser->add_diagnostic(std::move(diag)); + // indicate errors were produced, but do not report them again + return std::make_shared( + range(position(m_current_logical_line_source.begin_line, 0)), std::vector()); + } + else if (lookahead) return nullptr; else return std::make_shared( diff --git a/parser_library/src/processing/statement_providers/members_statement_provider.cpp b/parser_library/src/processing/statement_providers/members_statement_provider.cpp index 6e7f4b949..6ea131f47 100644 --- a/parser_library/src/processing/statement_providers/members_statement_provider.cpp +++ b/parser_library/src/processing/statement_providers/members_statement_provider.cpp @@ -66,7 +66,6 @@ context::shared_stmt_ptr members_statement_provider::get_next(const statement_pr break; } - if (processor.kind == processing_kind::ORDINARY && try_trigger_attribute_lookahead(*stmt, { *ctx.hlasm_ctx, lib_provider }, listener)) return nullptr; @@ -94,9 +93,9 @@ void members_statement_provider::fill_cache( context::statement_cache& cache, const semantics::deferred_statement& def_stmt, const processing_status& status) { context::statement_cache::cached_statement_t reparsed_stmt; + // TODO: what if it fails? auto def_impl = std::dynamic_pointer_cast(cache.get_base()); - // TODO: what if it fails? auto diags = def_impl->diagnostics(); for (auto i = diags.first; i != diags.second; ++i) reparsed_stmt.diags.push_back(*i); @@ -123,7 +122,7 @@ void members_statement_provider::fill_cache( reparsed_stmt.stmt = std::make_shared(def_impl, std::move(op), std::move(rem)); } - cache.insert(status.first.form, reparsed_stmt); + cache.insert(status.first.form, std::move(reparsed_stmt)); } context::shared_stmt_ptr members_statement_provider::preprocess_deferred( @@ -141,8 +140,11 @@ context::shared_stmt_ptr members_statement_provider::preprocess_deferred( const auto& cache_item = cache.get(status.first.form); - for (const diagnostic_op& diag : cache_item->diags) - diagnoser.add_diagnostic(diag); + if (processor.kind != processing_kind::LOOKAHEAD) + { + for (const diagnostic_op& diag : cache_item->diags) + diagnoser.add_diagnostic(diag); + } return std::make_shared(cache_item->stmt, status); } diff --git a/parser_library/test/common_testing.h b/parser_library/test/common_testing.h index a6a408f35..02137aa10 100644 --- a/parser_library/test/common_testing.h +++ b/parser_library/test/common_testing.h @@ -15,7 +15,10 @@ #ifndef HLASMPLUGIN_HLASMPARSERLIBRARY_COMMON_TESTING_H #define HLASMPLUGIN_HLASMPARSERLIBRARY_COMMON_TESTING_H +#include +#include #include +#include #include "antlr4-runtime.h" #include "gmock/gmock.h" @@ -124,4 +127,12 @@ std::optional> get_var_vector(hlasm_context& ctx, std::string nam return result; } +inline bool matches_message_codes(const std::vector& d, std::initializer_list m) +{ + std::vector codes; + std::transform(d.begin(), d.end(), std::back_inserter(codes), [](const auto& d) { return d.code; }); + + return std::is_permutation(codes.begin(), codes.end(), m.begin(), m.end()); +} + #endif diff --git a/parser_library/test/processing/copy_test.cpp b/parser_library/test/processing/copy_test.cpp index b427a6eea..fce8a3818 100644 --- a/parser_library/test/processing/copy_test.cpp +++ b/parser_library/test/processing/copy_test.cpp @@ -605,3 +605,24 @@ TEST(copy, copy_empty_file) ASSERT_EQ(a.diags().size(), 0U); } + +TEST(copy, skip_invalid) +{ + std::string copybook_content = R"( + AGO .SKIP + 2 a +a a X +aaaaaaaaa +.SKIP ANOP +)"; + std::string input = R"( + COPY COPYBOOK +)"; + mock_parse_lib_provider lib_provider { { "COPYBOOK", copybook_content } }; + analyzer a(input, analyzer_options { &lib_provider }); + + a.analyze(); + a.collect_diags(); + + EXPECT_TRUE(a.diags().empty()); +} diff --git a/parser_library/test/workspace/workspace_test.cpp b/parser_library/test/workspace/workspace_test.cpp index 4d9acc137..b4ac09386 100644 --- a/parser_library/test/workspace/workspace_test.cpp +++ b/parser_library/test/workspace/workspace_test.cpp @@ -323,21 +323,21 @@ TEST_F(workspace_test, did_close_file) // 3 files are open // - open codes source1 and source2 with syntax errors using macro ERROR // - macro file lib/ERROR with syntax error - // on first reparse, there should be 2x2=4 diagnotics from sources and lib/ERROR file + // on first reparse, there should be 3 diagnotics from sources and lib/ERROR file ws.did_open_file("source1"); ws.did_open_file("source2"); - ASSERT_EQ(collect_and_get_diags_size(ws, file_manager), (size_t)4); - EXPECT_TRUE(match_strings({ faulty_macro_path, faulty_macro_path, "source2", "source1" })); + EXPECT_EQ(collect_and_get_diags_size(ws, file_manager), (size_t)3); + EXPECT_TRUE(match_strings({ faulty_macro_path, "source2", "source1" })); // when we close source1, only its diagnostics should disapear // macro's and source2's diagnostics should stay as it is still open ws.did_close_file("source1"); - ASSERT_EQ(collect_and_get_diags_size(ws, file_manager), (size_t)2); + EXPECT_EQ(collect_and_get_diags_size(ws, file_manager), (size_t)2); EXPECT_TRUE(match_strings({ faulty_macro_path, "source2" })); // even though we close the ERROR macro, its diagnostics will still be there as it is a dependency of source2 ws.did_close_file(faulty_macro_path); - ASSERT_EQ(collect_and_get_diags_size(ws, file_manager), (size_t)2); + EXPECT_EQ(collect_and_get_diags_size(ws, file_manager), (size_t)2); EXPECT_TRUE(match_strings({ faulty_macro_path, "source2" })); // if we remove the line using ERROR macro in the source2. its diagnostics will be removed as it is no longer a @@ -347,7 +347,7 @@ TEST_F(workspace_test, did_close_file) changes.push_back(document_change({ { 0, 0 }, { 0, 6 } }, new_text.c_str(), new_text.size())); file_manager.did_change_file("source2", 1, changes.data(), changes.size()); ws.did_change_file("source2", changes.data(), changes.size()); - ASSERT_EQ(collect_and_get_diags_size(ws, file_manager), (size_t)1); + EXPECT_EQ(collect_and_get_diags_size(ws, file_manager), (size_t)1); EXPECT_TRUE(match_strings({ "source2" })); // finally if we close the last source2 file, its diagnostics will disappear as well