refactor: Parser reports all its diagnostics using single channel (#141)

parser_library/src/CMakeLists.txt

@@ -22,7 +22,7 @@ target_sources(parser_library PRIVATE
 	diagnosable_impl.h
 	diagnostic.cpp
 	diagnostic.h
-	diagnostic_adder.cpp
+	diagnostic_consumer.h
 	diagnostic_adder.h
 	ebcdic_encoding.cpp
 	ebcdic_encoding.h

parser_library/src/analyzer.cpp

@@ -48,14 +48,14 @@ workspaces::parse_lib_provider& analyzer_options::get_lib_provider() const
 }
 
 std::unique_ptr<processing::preprocessor> analyzer_options::get_preprocessor(
-    processing::library_fetcher lf, processing::diag_reporter dr) const
+    processing::library_fetcher lf, diagnostic_op_consumer& diag_consumer) const
 {
     return std::visit(
-        [&lf, &dr](const auto& p) -> std::unique_ptr<processing::preprocessor> {
+        [&lf, &diag_consumer](const auto& p) -> std::unique_ptr<processing::preprocessor> {
             if constexpr (std::is_same_v<std::decay_t<decltype(p)>, std::monostate>)
                 return {};
             else
-                return processing::preprocessor::create(p, std::move(lf), std::move(dr));
+                return processing::preprocessor::create(p, std::move(lf), &diag_consumer);
         },
         preprocessor_args);
 }
@@ -70,7 +70,7 @@ analyzer::analyzer(const std::string& text, analyzer_options opts)
                 opts.get_lib_provider(),
                 mngr_,
                 src_proc_,
-                opts.file_name,
+                *this,
                 opts.get_preprocessor(
                     [libs = &opts.get_lib_provider(), program = opts.file_name, &ctx = ctx_](std::string_view library) {
                         std::string uri;
@@ -82,7 +82,7 @@ analyzer::analyzer(const std::string& text, analyzer_options opts)
 
                         return result;
                     },
-                    [this](diagnostic_op d) { this->add_diagnostic(std::move(d)); }),
+                    *this),
                 opts.parsing_opencode == file_is_opencode::yes ? opencode_provider_options { true, 10 }
                                                                : opencode_provider_options {}),
           ctx_,

parser_library/src/analyzer.h

@@ -68,7 +68,7 @@ class analyzer_options
     analyzing_context& get_context();
     workspaces::parse_lib_provider& get_lib_provider() const;
     std::unique_ptr<processing::preprocessor> get_preprocessor(
-        processing::library_fetcher, processing::diag_reporter) const;
+        processing::library_fetcher, diagnostic_op_consumer&) const;
 
     friend class analyzer;
 

parser_library/src/diagnosable.h

@@ -19,6 +19,7 @@
 #include <vector>
 
 #include "diagnostic.h"
+#include "diagnostic_consumer.h"
 #include "protocol.h"
 
 // Interface that allows to collect objects (diagnostics)
@@ -34,14 +35,13 @@ namespace hlasm_plugin::parser_library {
 // Interface that allows to collect objects (diagnostics)
 // from a tree structure of objects.
 template<typename T>
-class collectable
+class collectable : public diagnostic_consumer<T>
 {
 public:
     using diagnostic_container = std::vector<T>;
 
     virtual void collect_diags() const = 0;
     virtual diagnostic_container& diags() const = 0;
-    virtual void add_diagnostic(T diagnostic) const = 0;
     // Specifies whether objects(diagnostics) should be moved
     // when collecting from this object.
     virtual bool is_once_only() const = 0;

parser_library/src/diagnosable_ctx.h

@@ -23,7 +23,7 @@ namespace hlasm_plugin::parser_library {
 
 // abstract diagnosable class that enhances collected diagnostics
 // adds a stack of nested file positions that indicate where the diagnostic occured
-class diagnosable_ctx : public diagnosable_impl
+class diagnosable_ctx : public diagnosable_impl, public diagnostic_op_consumer
 {
     context::hlasm_context& ctx_;
 
@@ -36,7 +36,7 @@ class diagnosable_ctx : public diagnosable_impl
             ctx_.processing_stack());
     }
 
-    void add_diagnostic(diagnostic_op diagnostic) const
+    void add_diagnostic(diagnostic_op diagnostic) const override
     {
         add_diagnostic_inner(std::move(diagnostic), ctx_.processing_stack());
     }

parser_library/src/diagnosable_impl.h

@@ -27,7 +27,6 @@ class collectable_impl : public virtual collectable<T>
 public:
     typename collectable<T>::diagnostic_container& diags() const override { return container; }
 
-protected:
     // Collects diagnostics from one collectable: calls its collect_diags
     // and then moves or copies the diagnostics depending on is_once_only
     virtual void collect_diags_from_child(const collectable<T>& child) const
@@ -48,9 +47,9 @@ class collectable_impl : public virtual collectable<T>
 
     void add_diagnostic(T diagnostic) const override { container.push_back(std::move(diagnostic)); }
 
+protected:
     bool is_once_only() const override { return true; }
 
-protected:
     ~collectable_impl() = default;
 
 private:

parser_library/src/diagnostic.cpp

@@ -2058,17 +2058,9 @@ diagnostic_s diagnostic_s::warning_L0004(const std::string& path, const std::str
         {});
 }
 
-diagnostic_s diagnostic_s::error_S100(const std::string& filename, const std::string& message, const range& range)
+diagnostic_op diagnostic_op::error_S100(const std::string& message, const range& range)
 {
-    return diagnostic_s(
-        filename, range, diagnostic_severity::error, "S100", "Long ordinary symbol name - " + message, {});
-}
-
-diagnostic_s diagnostic_s::error_S101(const std::string& filename, const std::string& message, const range& range)
-{
-    return diagnostic_s(
-        filename, range, diagnostic_severity::error, "S101", "Illegal attribute reference - " + message, {});
+    return diagnostic_op(diagnostic_severity::error, "S100", "Long ordinary symbol name - " + message, range);
 }
 
-
 } // namespace hlasm_plugin::parser_library

parser_library/src/diagnostic.h

@@ -106,12 +106,7 @@ struct diagnostic_op
     range diag_range;
     diagnostic_op() = default;
 
-    diagnostic_op(diagnostic_severity severity, std::string code, std::string message)
-        : severity(severity)
-        , code(std::move(code))
-        , message(std::move(message)) {};
-
-    diagnostic_op(diagnostic_severity severity, std::string code, std::string message, range diag_range)
+    diagnostic_op(diagnostic_severity severity, std::string code, std::string message, range diag_range = {})
         : severity(severity)
         , code(std::move(code))
         , message(std::move(message))
@@ -629,6 +624,8 @@ struct diagnostic_op
 
     static diagnostic_op error_CW001(const range& range);
 
+    static diagnostic_op error_S100(const std::string& message, const range& range);
+
     static diagnostic_op error_P0001(const range& range);
 
     static diagnostic_op error_P0002(const range& range, std::string_view lib);
@@ -729,10 +726,6 @@ class diagnostic_s
 
     static diagnostic_s warning_L0004(const std::string& path, const std::string& macro_name);
 
-    static diagnostic_s error_S100(const std::string& filename, const std::string& message, const range& range);
-
-    static diagnostic_s error_S101(const std::string& filename, const std::string& message, const range& range);
-
     static diagnostic_s error_W002(const std::string& file_name, const std::string& ws_name);
 
     static diagnostic_s error_W003(const std::string& file_name, const std::string& ws_name);

parser_library/src/diagnostic_adder.h

@@ -26,22 +26,28 @@ namespace hlasm_plugin::parser_library {
 // class that simplyfies adding of diagnostics
 // holds both range and collectable object
 // hence, there is no need to specify range in a diagnostic creation, just pass diagnostic function
+
 class diagnostic_adder
 {
-    const collectable<diagnostic_s>* s_diagnoser_;
-    const collectable<diagnostic_op>* op_diagnoser_;
+    const diagnostic_op_consumer* op_diagnoser_ = nullptr;
     range diag_range_;
 
 public:
-    bool diagnostics_present;
-
-    diagnostic_adder(const collectable<diagnostic_s>* diagnoser, range diag_range);
+    bool diagnostics_present = false;
 
-    diagnostic_adder(const collectable<diagnostic_op>* diagnoser, range diag_range);
+    diagnostic_adder(const diagnostic_op_consumer& diagnoser, range diag_range)
+        : op_diagnoser_(&diagnoser)
+        , diag_range_(diag_range) {};
 
-    diagnostic_adder();
+    diagnostic_adder() = default;
 
-    void operator()(const std::function<diagnostic_op(range)>& f);
+    template<typename F, typename = std::enable_if<std::is_invocable_r_v<diagnostic_op, F, range>>>
+    void operator()(F&& f)
+    {
+        diagnostics_present = true;
+        if (op_diagnoser_)
+            op_diagnoser_->add_diagnostic(f(diag_range_));
+    }
 };
 
 } // namespace hlasm_plugin::parser_library

parser_library/src/diagnostic_consumer.h

@@ -0,0 +1,76 @@
+/*
+ * Copyright (c) 2019 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_DIAGNOSTIC_CONSUMER_H
+#define HLASMPARSER_PARSERLIBRARY_DIAGNOSTIC_CONSUMER_H
+
+#include <functional>
+#include <vector>
+
+#include "diagnostic.h"
+
+// Interface that allows to consume diagnostics regardless of how are they processed afterwards
+
+namespace hlasm_plugin::parser_library {
+
+// Interface that allows to collect objects (diagnostics)
+// from a tree structure of objects.
+template<typename T>
+class diagnostic_consumer
+{
+    // TODO The reason why this function is const is that all current implementations have mutable containters where
+    // the diagnostics are stored and large parts of the project depend on that constness of the function
+public:
+    virtual void add_diagnostic(T diagnostic) const = 0;
+
+protected:
+    ~diagnostic_consumer() = default;
+};
+
+using diagnostic_s_consumer = diagnostic_consumer<diagnostic_s>;
+using diagnostic_op_consumer = diagnostic_consumer<diagnostic_op>;
+
+namespace transform_traits {
+template<typename R, typename T>
+T arg0(std::function<R(T)>);
+
+template<typename T>
+using arg0_t = decltype(arg0(std::function(std::declval<T>())));
+} // namespace transform_traits
+
+template<typename F, typename T = typename transform_traits::arg0_t<F>>
+class diagnostic_consumer_transform : public diagnostic_consumer<T>
+{
+    F consumer;
+
+public:
+    explicit diagnostic_consumer_transform(F f)
+        : consumer(std::move(f))
+    {}
+    void add_diagnostic(T diagnostic) const override { consumer(std::move(diagnostic)); };
+};
+
+template<typename T>
+class diagnostic_consumer_container : public diagnostic_consumer<T>
+{
+public:
+    mutable std::vector<T> diags;
+    void add_diagnostic(T diagnostic) const override { diags.push_back(std::move(diagnostic)); };
+};
+
+using diagnostic_op_consumer_container = diagnostic_consumer_container<diagnostic_op>;
+
+} // namespace hlasm_plugin::parser_library
+
+#endif

parser_library/src/expressions/conditional_assembly/ca_operator_unary.cpp

@@ -79,7 +79,7 @@ context::SET_t ca_function_unary_operator::operation(context::SET_t operand, con
     }
     else if (expr_kind == context::SET_t_enum::C_TYPE)
     {
-        diagnostic_adder add_diagnostic(&eval_ctx, expr_range);
+        diagnostic_adder add_diagnostic(eval_ctx, expr_range);
         switch (function)
         {
             case ca_expr_ops::BYTE:

parser_library/src/expressions/conditional_assembly/terms/ca_function.cpp

@@ -87,7 +87,7 @@ void ca_function::apply(ca_expr_visitor& visitor) const { visitor.visit(*this);
 context::SET_t ca_function::evaluate(const evaluation_context& eval_ctx) const
 {
     context::SET_t str_ret;
-    diagnostic_adder add_diagnostic(&eval_ctx, expr_range);
+    diagnostic_adder add_diagnostic(eval_ctx, expr_range);
 
     switch (function)
     {

parser_library/src/expressions/conditional_assembly/terms/ca_var_sym.cpp

@@ -79,7 +79,7 @@ context::SET_t ca_var_sym::convert_return_types(
 {
     if (retval.type == context::SET_t_enum::C_TYPE)
     {
-        diagnostic_adder add_diags(&eval_ctx, expr_range);
+        diagnostic_adder add_diags(eval_ctx, expr_range);
         switch (type)
         {
             case context::SET_t_enum::A_TYPE:

parser_library/src/expressions/evaluation_context.h

@@ -21,7 +21,7 @@
 namespace hlasm_plugin::parser_library::expressions {
 
 // structure holding required objects to correcly perform evaluation of expressions
-struct evaluation_context : diagnosable_ctx
+struct evaluation_context : public diagnosable_ctx
 {
     analyzing_context ctx;
     context::hlasm_context& hlasm_ctx;
@@ -34,7 +34,7 @@ struct evaluation_context : diagnosable_ctx
         , lib_provider(lib_provider)
     {}
 
-    evaluation_context(const evaluation_context&) = delete;
+    evaluation_context(const evaluation_context& oth) = delete;
 
     void collect_diags() const override
     {

parser_library/src/expressions/mach_expr_term.cpp

@@ -90,7 +90,7 @@ void mach_expr_symbol::apply(mach_expr_visitor& visitor) const { visitor.visit(*
 mach_expr_self_def::mach_expr_self_def(std::string option, std::string value, range rng)
     : mach_expression(rng)
 {
-    diagnostic_adder add_diagnostic(this, rng);
+    diagnostic_adder add_diagnostic(*this, rng);
     value_ = ca_constant::self_defining_term(option, value, add_diagnostic);
 }
 

parser_library/src/parsing/CMakeLists.txt

@@ -14,8 +14,6 @@ target_sources(parser_library PRIVATE
 	error_strategy.h
 	parser_error_listener.cpp
 	parser_error_listener.h
-	parser_error_listener_ctx.h
-	parser_error_listener_substitution.h
 	parser_impl.cpp
 	parser_impl.h
 	parser_tools.cpp