-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Next x3 grammar v2 #59
base: main
Are you sure you want to change the base?
Conversation
- import of old branch of rework - rebased to current main branch (and dated forward) ToDo: - tagging doesn't work as expected, see syntax.cpp; there where backports from github IIRC??? * Node 'struct entity_declaration' not tagged - internal error, BUT * works with architecture_body !!! There is a discrepancy - the label match check API is insufficient to gather these errors easily - rewrite TESTs
- x3::rule<..>( name ) names the rule for error handling; getting it the right name lowers the lookup in make_error_description() inside error_handler.
- disabled since compile time increases dramatically
- added comment rule to grammar - replaced phrase_parse() by parse() and removed skipper.hpp
- replace design_file entry with grammar_entry
- cleanup parser's parse.hpp API - add comment - parse() catches also x3::expectation_failure
- using testsuite namespace for all test; it's single test suite which shouldn't collide with other test during compile time - added test for - bit_string_literal
- fix GPL3-or-later - apply Clang-format
- add testsuite for Spirit X3 charset: - Spirit X3 doesn't support full set, also compare function for x3::no_case doesn't work - todo.md extended - add soft hyphen to charset, also documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There were too many comments to post at once. Showing the first 25 out of 64. Check the log or trigger a new build to see more.
@@ -5,11 +5,21 @@ | |||
|
|||
#pragma once | |||
|
|||
#include <ibis/vhdl/ast/node/association_element.hpp> | |||
#include <ibis/vhdl/ast/util/variant.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'ibis/vhdl/ast/util/variant.hpp' file not found [clang-diagnostic-error]
#include <ibis/vhdl/ast/util/variant.hpp>
^
#include <vector> | ||
|
||
namespace ibis::vhdl::ast { | ||
|
||
struct association_element : position_tagged { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: constructor does not initialize these fields: formal_part, actual_part [cppcoreguidelines-pro-type-member-init]
source/vhdl/include/ibis/vhdl/ast/node/association_list.hpp:18:
- optional<ast::formal_part> formal_part;
- ast::actual_part actual_part;
+ optional<ast::formal_part> formal_part{};
+ ast::actual_part actual_part{};
@@ -6,6 +6,7 @@ | |||
#pragma once | |||
|
|||
#include <ibis/vhdl/ast/util/position_tagged.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'ibis/vhdl/ast/util/position_tagged.hpp' file not found [clang-diagnostic-error]
#include <ibis/vhdl/ast/util/position_tagged.hpp>
^
ast::primary primary_lhs; | ||
operator_token operator_; | ||
ast::primary primary_rhs; | ||
struct chunk { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: constructor does not initialize these fields: operator_, primary [cppcoreguidelines-pro-type-member-init]
source/vhdl/include/ibis/vhdl/ast/node/factor.hpp:19:
- operator_token operator_;
- ast::primary primary;
+ operator_token operator_{};
+ ast::primary primary{};
@@ -8,12 +8,13 @@ | |||
#include <ibis/vhdl/ast/util/position_tagged.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'ibis/vhdl/ast/util/position_tagged.hpp' file not found [clang-diagnostic-error]
#include <ibis/vhdl/ast/util/position_tagged.hpp>
^
constexpr std::size_t len = (Strs.size() + ... + 0); | ||
std::array<char, len + 1> arr{}; | ||
auto append = [i = 0, &arr](auto const& s) mutable { | ||
for (auto c : s) arr[i++] = c; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: statement should be inside braces [readability-braces-around-statements]
for (auto c : s) arr[i++] = c; | |
for (auto c : s) { arr[i++] = c; | |
} |
constexpr std::size_t len = (Strs.size() + ... + 0); | ||
std::array<char, len + 1> arr{}; | ||
auto append = [i = 0, &arr](auto const& s) mutable { | ||
for (auto c : s) arr[i++] = c; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable name 'c' is too short, expected at least 3 characters [readability-identifier-length]
for (auto c : s) arr[i++] = c;
^
}; | ||
// Helper to get the value out | ||
template <std::string_view const&... Strs> | ||
static constexpr auto join_v = join<Strs...>::value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'join_v' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]
static constexpr auto join_v = join<Strs...>::value; | |
constexpr auto join_v = join<Strs...>::value; |
// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) | ||
#define BOOST_TEST_MODULE Spirit X3 Test Suite | ||
|
||
#include <boost/test/unit_test.hpp> // IWYU pragma: keep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'boost/test/unit_test.hpp' file not found [clang-diagnostic-error]
#include <boost/test/unit_test.hpp> // IWYU pragma: keep
^
|
||
#pragma once | ||
|
||
#include <testsuite/util/dataset_loader.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'testsuite/util/dataset_loader.hpp' file not found [clang-diagnostic-error]
#include <testsuite/util/dataset_loader.hpp>
^
Cpp-Linter Report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There were too many comments to post at once. Showing the first 25 out of 38. Check the log or trigger a new build to see more.
#include <boost/test/data/monomorphic.hpp> | ||
|
||
#include <string_view> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: included header string_view is not used directly [misc-include-cleaner]
|
||
// C++ | ||
#include <filesystem> | ||
#include <fstream> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: included header filesystem is not used directly [misc-include-cleaner]
#include <fstream> | |
#include <fstream> |
// C++ | ||
#include <filesystem> | ||
#include <fstream> | ||
#include <iterator> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: included header fstream is not used directly [misc-include-cleaner]
#include <iterator> | |
#include <iterator> |
#include <filesystem> | ||
#include <fstream> | ||
#include <iterator> | ||
#include <optional> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: included header iterator is not used directly [misc-include-cleaner]
#include <optional> | |
#include <optional> |
#include <fstream> | ||
#include <iterator> | ||
#include <optional> | ||
#include <regex> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: included header optional is not used directly [misc-include-cleaner]
#include <regex> | |
#include <regex> |
using base_specifier = ast::bit_string_literal::base_specifier; | ||
using namespace std::literals::string_view_literals; | ||
|
||
struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: do not declare C-style arrays, use std::array<> instead [cppcoreguidelines-avoid-c-arrays]
struct {
^
{ | ||
} | ||
|
||
void operator()(ast::constant_declaration const& node, [[maybe_unused]] std::string_view) const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: all parameters should be named in a function [readability-named-parameter]
void operator()(ast::constant_declaration const& node, [[maybe_unused]] std::string_view) const | |
void operator()(ast::constant_declaration const& node, [[maybe_unused]] std::string_view /*unused*/) const |
} | ||
} | ||
|
||
void operator()(ast::bit_string_literal const& node, [[maybe_unused]] std::string_view) const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: all parameters should be named in a function [readability-named-parameter]
void operator()(ast::bit_string_literal const& node, [[maybe_unused]] std::string_view) const | |
void operator()(ast::bit_string_literal const& node, [[maybe_unused]] std::string_view /*unused*/) const |
|
||
void operator()(ast::bit_string_literal const& node, [[maybe_unused]] std::string_view) const | ||
{ | ||
assert(test_index < expected_size && "test_index reached count of expected array size!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: implicit conversion 'const char *' -> 'bool' [readability-implicit-bool-conversion]
assert(test_index < expected_size && "test_index reached count of expected array size!"); | |
assert(test_index < expected_size && true); |
} | ||
|
||
template <typename NodeT> | ||
void operator()([[maybe_unused]] NodeT const&, [[maybe_unused]] std::string_view) const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: all parameters should be named in a function [readability-named-parameter]
void operator()([[maybe_unused]] NodeT const&, [[maybe_unused]] std::string_view) const | |
void operator()([[maybe_unused]] NodeT const& /*unused*/, [[maybe_unused]] std::string_view /*unused*/) const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
{ | ||
} | ||
|
||
ExpectedT const& expected; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: member 'expected' of type 'const ExpectedT &' is a reference [cppcoreguidelines-avoid-const-or-ref-data-members]
ExpectedT const& expected;
^
{ | ||
} | ||
|
||
ExpectedT const& expected; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: member variable 'expected' has public visibility [misc-non-private-member-variables-in-classes]
ExpectedT const& expected;
^
} | ||
|
||
ExpectedT const& expected; | ||
std::size_t const expected_size; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: member 'expected_size' of type 'const std::size_t' (aka 'const unsigned long') is const qualified [cppcoreguidelines-avoid-const-or-ref-data-members]
std::size_t const expected_size;
^
} | ||
|
||
ExpectedT const& expected; | ||
std::size_t const expected_size; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: member variable 'expected_size' has public visibility [misc-non-private-member-variables-in-classes]
std::size_t const expected_size;
^
} | ||
|
||
ExpectedT const& expected; | ||
std::size_t const expected_size; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: no header providing "std::size_t" is directly included [misc-include-cleaner]
testsuite/vhdl/grammar/src/test/literal.cpp:5:
- #include <testsuite/vhdl/grammar/testsuite_parse.hpp>
+ #include <cstddef>
+ #include <testsuite/vhdl/grammar/testsuite_parse.hpp>
|
||
} // namespace data | ||
|
||
BOOST_AUTO_TEST_CASE(bit_string_literal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable 'bit_string_literal' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
BOOST_AUTO_TEST_CASE(bit_string_literal)
^
// SPDX-License-Identifier: GPL-3.0-or-later | ||
// | ||
|
||
#include <testsuite/vhdl/grammar/testsuite_parse.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'testsuite/vhdl/grammar/testsuite_parse.hpp' file not found [clang-diagnostic-error]
#include <testsuite/vhdl/grammar/testsuite_parse.hpp>
^
#include <boost/test/data/test_case.hpp> | ||
|
||
#include <iostream> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: included header iostream is not used directly [misc-include-cleaner]
// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) | ||
#define BOOST_TEST_MODULE VHDL Grammar Test Suite | ||
|
||
#include <boost/test/unit_test.hpp> // IWYU pragma: keep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'boost/test/unit_test.hpp' file not found [clang-diagnostic-error]
#include <boost/test/unit_test.hpp> // IWYU pragma: keep
^
// Copyright (c) 2017-2022 Olaf (<[email protected]>). | ||
// SPDX-License-Identifier: GPL-3.0-or-later | ||
|
||
#include <testsuite/vhdl/grammar/testsuite_parse.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'testsuite/vhdl/grammar/testsuite_parse.hpp' file not found [clang-diagnostic-error]
#include <testsuite/vhdl/grammar/testsuite_parse.hpp>
^
Merge into Main in 2024