Skip to content

Commit

Permalink
refactor: Parser reports all its diagnostics using single channel (#141)
Browse files Browse the repository at this point in the history
  • Loading branch information
michalbali256 authored Jul 9, 2021
1 parent 77275dd commit 656caf3
Show file tree
Hide file tree
Showing 45 changed files with 387 additions and 450 deletions.
2 changes: 1 addition & 1 deletion parser_library/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions parser_library/src/analyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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;
Expand All @@ -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_,
Expand Down
2 changes: 1 addition & 1 deletion parser_library/src/analyzer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 2 additions & 2 deletions parser_library/src/diagnosable.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <vector>

#include "diagnostic.h"
#include "diagnostic_consumer.h"
#include "protocol.h"

// Interface that allows to collect objects (diagnostics)
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions parser_library/src/diagnosable_ctx.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;

Expand All @@ -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());
}
Expand Down
3 changes: 1 addition & 2 deletions parser_library/src/diagnosable_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
12 changes: 2 additions & 10 deletions parser_library/src/diagnostic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 3 additions & 10 deletions parser_library/src/diagnostic.h
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
50 changes: 0 additions & 50 deletions parser_library/src/diagnostic_adder.cpp

This file was deleted.

22 changes: 14 additions & 8 deletions parser_library/src/diagnostic_adder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
76 changes: 76 additions & 0 deletions parser_library/src/diagnostic_consumer.h
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions parser_library/src/expressions/evaluation_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
{
Expand Down
2 changes: 1 addition & 1 deletion parser_library/src/expressions/mach_expr_term.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
2 changes: 0 additions & 2 deletions parser_library/src/parsing/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 656caf3

Please sign in to comment.