From bc9a6da02c2d9b940e240645453654ae3eb0aedf Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sun, 28 Sep 2025 16:36:11 -0400 Subject: [PATCH 1/3] Implement... --- src/spider/CMakeLists.txt | 4 + src/spider/tdl/parser/SourceLocation.hpp | 11 +++ .../parser/ast/node_impl/TranslationUnit.hpp | 23 ++++++ src/spider/tdl/pass/Pass.hpp | 63 ++++++++++++++++ .../pass/analysis/DetectUndefinedStruct.cpp | 74 +++++++++++++++++++ .../pass/analysis/DetectUndefinedStruct.hpp | 57 ++++++++++++++ .../analysis/StructSpecDependencyGraph.cpp | 47 ++++-------- src/spider/tdl/pass/utils.hpp | 66 +++++++++++++++++ tests/CMakeLists.txt | 1 + ...st-pass-analysis-DetectUndefinedStruct.cpp | 62 ++++++++++++++++ ...ass-analysis-StructSpecDependencyGraph.cpp | 6 +- 11 files changed, 379 insertions(+), 35 deletions(-) create mode 100644 src/spider/tdl/pass/Pass.hpp create mode 100644 src/spider/tdl/pass/analysis/DetectUndefinedStruct.cpp create mode 100644 src/spider/tdl/pass/analysis/DetectUndefinedStruct.hpp create mode 100644 src/spider/tdl/pass/utils.hpp create mode 100644 tests/tdl/test-pass-analysis-DetectUndefinedStruct.cpp diff --git a/src/spider/CMakeLists.txt b/src/spider/CMakeLists.txt index b81e2dff..b896a91a 100644 --- a/src/spider/CMakeLists.txt +++ b/src/spider/CMakeLists.txt @@ -227,6 +227,7 @@ set(SPIDER_TDL_SHARED_SOURCES tdl/parser/ast/node_impl/type_impl/Struct.cpp tdl/parser/ast/utils.cpp tdl/parser/parse.cpp + tdl/pass/analysis/DetectUndefinedStruct.cpp tdl/pass/analysis/StructSpecDependencyGraph.cpp CACHE INTERNAL "spider task definition language shared source files" @@ -259,7 +260,10 @@ set(SPIDER_TDL_SHARED_HEADERS tdl/parser/Exception.hpp tdl/parser/parse.hpp tdl/parser/SourceLocation.hpp + tdl/pass/analysis/DetectUndefinedStruct.hpp tdl/pass/analysis/StructSpecDependencyGraph.hpp + tdl/pass/Pass.hpp + tdl/pass/utils.hpp CACHE INTERNAL "spider task definition language shared header files" ) diff --git a/src/spider/tdl/parser/SourceLocation.hpp b/src/spider/tdl/parser/SourceLocation.hpp index 7307998c..f5f50b70 100644 --- a/src/spider/tdl/parser/SourceLocation.hpp +++ b/src/spider/tdl/parser/SourceLocation.hpp @@ -1,6 +1,7 @@ #ifndef SPIDER_TDL_PARSER_SOURCELOCATION_HPP #define SPIDER_TDL_PARSER_SOURCELOCATION_HPP +#include #include #include @@ -25,6 +26,16 @@ class SourceLocation { return m_line == other.m_line && m_column == other.m_column; } + [[nodiscard]] auto operator<=>(SourceLocation const& other) const noexcept + -> std::strong_ordering { + if (auto const lint_comparison{m_line <=> other.m_line}; + std::strong_ordering::equal != lint_comparison) + { + return lint_comparison; + } + return m_column <=> other.m_column; + } + private: // Variables size_t m_line; diff --git a/src/spider/tdl/parser/ast/node_impl/TranslationUnit.hpp b/src/spider/tdl/parser/ast/node_impl/TranslationUnit.hpp index a556c647..fbfdbd0d 100644 --- a/src/spider/tdl/parser/ast/node_impl/TranslationUnit.hpp +++ b/src/spider/tdl/parser/ast/node_impl/TranslationUnit.hpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -47,6 +48,28 @@ class TranslationUnit : public Node { -> ystdlib::error_handling::Result override; // Methods + /** + * Visits all struct specs in the struct spec table in an unspecified order, invoking the given + * `visitor` for each struct spec. + * @tparam StructSpecVisitor + * @param visitor + * @return A void result on success, or an error code indicating the failure: + * - Forwards `visitor`'s return values. + */ + template + requires(std::is_invocable_r_v< + ystdlib::error_handling::Result, + StructSpecVisitor, + StructSpec const* + >) + [[nodiscard]] auto visit_struct_specs(StructSpecVisitor visitor) const + -> ystdlib::error_handling::Result { + for (auto const& [_, struct_spec] : m_struct_spec_table) { + YSTDLIB_ERROR_HANDLING_TRYV(visitor(struct_spec.get())); + } + return ystdlib::error_handling::success(); + } + /** * @param name * @return A shared pointer to the `StructSpec` with the given name if it exists in the struct diff --git a/src/spider/tdl/pass/Pass.hpp b/src/spider/tdl/pass/Pass.hpp new file mode 100644 index 00000000..70b55f8f --- /dev/null +++ b/src/spider/tdl/pass/Pass.hpp @@ -0,0 +1,63 @@ +#ifndef SPIDER_TDL_PASS_PASS_HPP +#define SPIDER_TDL_PASS_PASS_HPP + +#include +#include + +#include + +namespace spider::tdl::pass { +/** + * Represents an abstract pass over a TDL AST. + */ +class Pass { +public: + // Types + /** + * Represents an abstract error that can occur during the execution of a pass. + */ + class Error { + public: + // Constructor + Error() = default; + + // Delete copy constructor and assignment operator + Error(Error const&) = delete; + auto operator=(Error const&) -> Error& = delete; + + // Default move constructor and assignment operator + Error(Error&&) = default; + auto operator=(Error&&) -> Error& = default; + + // Destructor + virtual ~Error() = default; + + // Methods + [[nodiscard]] virtual auto to_string() const -> std::string = 0; + }; + + // Constructors + Pass() = default; + + // Delete copy constructor and assignment operator + Pass(Pass const&) = delete; + auto operator=(Pass const&) -> Pass& = delete; + + // Default move constructor and assignment operator + Pass(Pass&&) = default; + auto operator=(Pass&&) -> Pass& = default; + + // Destructor + virtual ~Pass() = default; + + // Methods + /** + * Executes the pass. + * @return A void result on success, or a pointer to the error on failure. + */ + [[nodiscard]] virtual auto run() -> boost::outcome_v2::std_checked> + = 0; +}; +} // namespace spider::tdl::pass + +#endif // SPIDER_TDL_PASS_PASS_HPP diff --git a/src/spider/tdl/pass/analysis/DetectUndefinedStruct.cpp b/src/spider/tdl/pass/analysis/DetectUndefinedStruct.cpp new file mode 100644 index 00000000..9d0da850 --- /dev/null +++ b/src/spider/tdl/pass/analysis/DetectUndefinedStruct.cpp @@ -0,0 +1,74 @@ +#include "DetectUndefinedStruct.hpp" + +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#include +#include +#include + +namespace spider::tdl::pass::analysis { +auto DetectUndefinedStruct::Error::to_string() const -> std::string { + std::vector undefined_struct_error_messages; + undefined_struct_error_messages.reserve(m_undefined_struct.size()); + for (auto const* undefined_struct : m_undefined_struct) { + undefined_struct_error_messages.emplace_back( + fmt::format( + "Referencing to an undefined struct `{}` at {}", + undefined_struct->get_name(), + undefined_struct->get_source_location().serialize_to_str() + ) + ); + } + return fmt::format( + "Found {} undefined struct reference(s):\n{}", + m_undefined_struct.size(), + fmt::join(undefined_struct_error_messages, "\n") + ); +} + +auto DetectUndefinedStruct::run() + -> boost::outcome_v2::std_checked> { + std::vector undefined_structs; + + auto struct_visitor + = [&](parser::ast::Struct const* struct_node) -> ystdlib::error_handling::Result { + if (nullptr == m_translation_unit->get_struct_spec(struct_node->get_name())) { + undefined_structs.emplace_back(struct_node); + } + return ystdlib::error_handling::success(); + }; + + std::ignore = visit_struct_node_using_dfs(m_translation_unit, struct_visitor); + std::ignore = m_translation_unit->visit_struct_specs( + [&]( + parser::ast::StructSpec const* struct_spec + ) -> ystdlib::error_handling::Result { + return visit_struct_node_using_dfs(struct_spec, struct_visitor); + } + ); + + if (undefined_structs.empty()) { + return boost::outcome_v2::success(); + } + std::ranges::sort( + undefined_structs, + [](parser::ast::Struct const* lhs, parser::ast::Struct const* rhs) -> bool { + return lhs->get_source_location() < rhs->get_source_location(); + } + ); + return boost::outcome_v2::failure( + std::make_unique(std::move(undefined_structs)) + ); +} +} // namespace spider::tdl::pass::analysis diff --git a/src/spider/tdl/pass/analysis/DetectUndefinedStruct.hpp b/src/spider/tdl/pass/analysis/DetectUndefinedStruct.hpp new file mode 100644 index 00000000..f1d6e7e3 --- /dev/null +++ b/src/spider/tdl/pass/analysis/DetectUndefinedStruct.hpp @@ -0,0 +1,57 @@ +#ifndef SPIDER_TDL_PASS_ANALYSIS_DETECTUNDEFINEDSTRUCTREF_HPP +#define SPIDER_TDL_PASS_ANALYSIS_DETECTUNDEFINEDSTRUCTREF_HPP + +#include +#include +#include +#include + +#include + +#include +#include + +namespace spider::tdl::pass::analysis { +/** + * A pass that detects undefined struct references in a TDL translation unit. + * NOTE: The translation unit must outlive the pass instance. + */ +class DetectUndefinedStruct : public Pass { +public: + // Types + /** + * Represents an error including all undefined structs. + * NOTE: The translation unit must outlive the error object. + */ + class Error : public Pass::Error { + public: + // Constructor + explicit Error(std::vector undefined_struct) + : m_undefined_struct{std::move(undefined_struct)} {} + + // Methods implementing `Pass::Error` + [[nodiscard]] auto to_string() const -> std::string override; + + private: + // Variables + std::vector m_undefined_struct; + }; + + // Constructor + explicit DetectUndefinedStruct(parser::ast::TranslationUnit const* translation_unit) + : m_translation_unit{translation_unit} {} + + // Methods implementing `Pass` + /** + * @return A void result on success, or a pointer to `DetectUndefinedStruct::Error` on failure. + */ + [[nodiscard]] auto run() + -> boost::outcome_v2::std_checked> override; + +private: + // Variables + parser::ast::TranslationUnit const* m_translation_unit; +}; +} // namespace spider::tdl::pass::analysis + +#endif // SPIDER_TDL_PASS_ANALYSIS_DETECTUNDEFINEDSTRUCTREF_HPP diff --git a/src/spider/tdl/pass/analysis/StructSpecDependencyGraph.cpp b/src/spider/tdl/pass/analysis/StructSpecDependencyGraph.cpp index efad5908..dd3eccd1 100644 --- a/src/spider/tdl/pass/analysis/StructSpecDependencyGraph.cpp +++ b/src/spider/tdl/pass/analysis/StructSpecDependencyGraph.cpp @@ -16,6 +16,7 @@ #include #include +#include namespace spider::tdl::pass::analysis { namespace { @@ -277,40 +278,22 @@ auto collect_use_ids( absl::flat_hash_map const& struct_spec_ids ) -> std::vector { absl::flat_hash_set use_ids; - std::vector ast_dfs_stack; - ast_dfs_stack.emplace_back(def); - while (false == ast_dfs_stack.empty()) { - auto const* node{ast_dfs_stack.back()}; - ast_dfs_stack.pop_back(); - - if (node == nullptr) { - // NOTE: This check is required by clang-tidy. In practice, this should never happen. - continue; - } - - auto const* node_as_struct{dynamic_cast(node)}; - if (nullptr == node_as_struct) { - // Not a struct node, continue DFS by pushing all the child nodes to the stack. - std::ignore = node->visit_children( - [&](parser::ast::Node const& child) -> ystdlib::error_handling::Result { - ast_dfs_stack.emplace_back(&child); - return ystdlib::error_handling::success(); - } - ); - continue; - } - auto const struct_name{node_as_struct->get_name()}; - auto const it{struct_specs.find(struct_name)}; - if (struct_specs.cend() == it) { - // This is a dangling reference, which will be caught in other analysis pass. In this - // dependency graph, we just ignore it. - continue; - } - - use_ids.emplace(struct_spec_ids.at(it->second.get())); - } + std::ignore = visit_struct_node_using_dfs( + def, + [&](parser::ast::Struct const* struct_node) -> ystdlib::error_handling::Result { + auto const struct_name{struct_node->get_name()}; + auto const it{struct_specs.find(struct_name)}; + if (struct_specs.cend() == it) { + // This is a dangling reference, which will be caught in other analysis pass. In + // this dependency graph, we just ignore it. + return ystdlib::error_handling::success(); + } + use_ids.emplace(struct_spec_ids.at(it->second.get())); + return ystdlib::error_handling::success(); + } + ); return std::vector{use_ids.cbegin(), use_ids.cend()}; } } // namespace diff --git a/src/spider/tdl/pass/utils.hpp b/src/spider/tdl/pass/utils.hpp new file mode 100644 index 00000000..a0f3f4df --- /dev/null +++ b/src/spider/tdl/pass/utils.hpp @@ -0,0 +1,66 @@ +#ifndef SPIDER_TDL_PASS_UTILS_HPP +#define SPIDER_TDL_PASS_UTILS_HPP + +#include +#include +#include + +#include + +#include + +namespace spider::tdl::pass { +/** + * Visits all `Struct` nodes in the AST rooted at `root` in a depth-first manner, invoking the given + * `visitor` for each `Struct` node encountered. + * @tparam StructVisitor + * @param root The root to start traversal from. + * @param visitor + * @return A void result on success, or an error code indicating the failure: + * - Forwards `visitor`'s return values. + */ +template +requires std::is_invocable_r_v< + ystdlib::error_handling::Result, + StructVisitor, + parser::ast::Struct const* +> +[[nodiscard]] auto visit_struct_node_using_dfs(parser::ast::Node const* root, StructVisitor visitor) + -> ystdlib::error_handling::Result; + +template +requires std::is_invocable_r_v< + ystdlib::error_handling::Result, + StructVisitor, + parser::ast::Struct const* +> +auto visit_struct_node_using_dfs(parser::ast::Node const* root, StructVisitor visitor) + -> ystdlib::error_handling::Result { + std::vector ast_dfs_stack{root}; + while (false == ast_dfs_stack.empty()) { + auto const* node{ast_dfs_stack.back()}; + ast_dfs_stack.pop_back(); + + if (node == nullptr) { + // NOTE: This check is required by clang-tidy. In practice, this should never happen. + continue; + } + + auto const* node_as_struct{dynamic_cast(node)}; + if (nullptr != node_as_struct) { + YSTDLIB_ERROR_HANDLING_TRYV(visitor(node_as_struct)); + continue; + } + + std::ignore = node->visit_children( + [&](parser::ast::Node const& child) -> ystdlib::error_handling::Result { + ast_dfs_stack.emplace_back(&child); + return ystdlib::error_handling::success(); + } + ); + } + return ystdlib::error_handling::success(); +} +} // namespace spider::tdl::pass + +#endif // SPIDER_TDL_PASS_UTILS_HPP diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 44a9e266..303e96fc 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -4,6 +4,7 @@ set(SPIDER_TEST_SOURCES storage/StorageTestHelper.hpp tdl/test-parser.cpp tdl/test-parser-ast.cpp + tdl/test-pass-analysis-DetectUndefinedStruct.cpp tdl/test-pass-analysis-StructSpecDependencyGraph.cpp utils/CoreDataUtils.hpp utils/CoreTaskUtils.hpp diff --git a/tests/tdl/test-pass-analysis-DetectUndefinedStruct.cpp b/tests/tdl/test-pass-analysis-DetectUndefinedStruct.cpp new file mode 100644 index 00000000..b0feb14f --- /dev/null +++ b/tests/tdl/test-pass-analysis-DetectUndefinedStruct.cpp @@ -0,0 +1,62 @@ +// NOLINTBEGIN(cert-err58-cpp,cppcoreguidelines-avoid-do-while,readability-function-cognitive-complexity,cppcoreguidelines-avoid-non-const-global-variables,cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays) + +#include +#include + +#include + +#include +#include + +namespace { +using spider::tdl::parser::parse_translation_unit_from_istream; +using spider::tdl::pass::analysis::DetectUndefinedStruct; + +constexpr std::string_view cTestCase1{R"(// Start of a TDL file. This is line#1. +struct class0 { + use_0: classN, // Undefined struct reference `classN` + use_1: Map>>, +}; + +namespace ns1 { + fn func0(input: class0) -> class1; + + fn func1( + input: class2 // Undefined struct reference `class2` + ) -> class3; // Undefined struct reference `class3` + + fn func2(a: ClassA, b: ClassB) -> int32; // Undefined struct references `ClassA` and `ClassB` +} + +struct class1 { + use_0: class0, + use_1: Map>>, // Undefined struct reference `class2` +}; +)"}; + +TEST_CASE("DetectUndefinedStruct Case 1", "[tdl][pass][analytics][DetectUndefinedStruct]") { + std::istringstream input_stream{std::string{cTestCase1}}; + auto const parse_result{parse_translation_unit_from_istream(input_stream)}; + REQUIRE_FALSE(parse_result.has_error()); + auto const& translation_unit{parse_result.value()}; + + auto detect_undefined_struct_pass{DetectUndefinedStruct{translation_unit.get()}}; + auto const run_result{detect_undefined_struct_pass.run()}; + REQUIRE(run_result.has_error()); + auto const* error{dynamic_cast(run_result.error().get())}; + REQUIRE(nullptr != error); + constexpr std::string_view cExpectedErrorMessage{ + "Found 7 undefined struct reference(s):\n" + "Referencing to an undefined struct `classN` at (3:11)\n" + "Referencing to an undefined struct `class2` at (4:37)\n" + "Referencing to an undefined struct `class2` at (11:15)\n" + "Referencing to an undefined struct `class3` at (12:9)\n" + "Referencing to an undefined struct `ClassA` at (14:16)\n" + "Referencing to an undefined struct `ClassB` at (14:27)\n" + "Referencing to an undefined struct `class2` at (19:37)" + }; + REQUIRE(cExpectedErrorMessage == error->to_string()); +} +} // namespace + +// NOLINTEND(cert-err58-cpp,cppcoreguidelines-avoid-do-while,readability-function-cognitive-complexity,cppcoreguidelines-avoid-non-const-global-variables,cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays) diff --git a/tests/tdl/test-pass-analysis-StructSpecDependencyGraph.cpp b/tests/tdl/test-pass-analysis-StructSpecDependencyGraph.cpp index 3971893e..36cbabb6 100644 --- a/tests/tdl/test-pass-analysis-StructSpecDependencyGraph.cpp +++ b/tests/tdl/test-pass-analysis-StructSpecDependencyGraph.cpp @@ -185,7 +185,7 @@ auto serialize_strongly_connected_components(StructSpecDependencyGraph& graph) return serialized_strongly_connected_components; } -TEST_CASE("Case 1", "[tdl][pass][analytics][StructSpecDependencyGraph]") { +TEST_CASE("StructSpecDependencyGraph Case 1", "[tdl][pass][analytics][StructSpecDependencyGraph]") { std::istringstream input_stream{std::string{cTestCase1}}; auto const parse_result{parse_translation_unit_from_istream(input_stream)}; REQUIRE_FALSE(parse_result.has_error()); @@ -212,7 +212,7 @@ TEST_CASE("Case 1", "[tdl][pass][analytics][StructSpecDependencyGraph]") { ); } -TEST_CASE("Case 2", "[tdl][pass][analytics][StructSpecDependencyGraph]") { +TEST_CASE("StructSpecDependencyGraphCase 2", "[tdl][pass][analytics][StructSpecDependencyGraph]") { std::istringstream input_stream{std::string{cTestCase2}}; auto const parse_result{parse_translation_unit_from_istream(input_stream)}; REQUIRE_FALSE(parse_result.has_error()); @@ -236,7 +236,7 @@ TEST_CASE("Case 2", "[tdl][pass][analytics][StructSpecDependencyGraph]") { ); } -TEST_CASE("Case 3", "[tdl][pass][analytics][StructSpecDependencyGraph]") { +TEST_CASE("StructSpecDependencyGraph Case 3", "[tdl][pass][analytics][StructSpecDependencyGraph]") { std::istringstream input_stream{std::string{cTestCase3}}; auto const parse_result{parse_translation_unit_from_istream(input_stream)}; REQUIRE_FALSE(parse_result.has_error()); From eb3029c7d076b0f8aca89247e84c94d7fd264022 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sun, 28 Sep 2025 16:50:57 -0400 Subject: [PATCH 2/3] Add a case without error. --- ...st-pass-analysis-DetectUndefinedStruct.cpp | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/tdl/test-pass-analysis-DetectUndefinedStruct.cpp b/tests/tdl/test-pass-analysis-DetectUndefinedStruct.cpp index b0feb14f..819dcaa0 100644 --- a/tests/tdl/test-pass-analysis-DetectUndefinedStruct.cpp +++ b/tests/tdl/test-pass-analysis-DetectUndefinedStruct.cpp @@ -34,6 +34,20 @@ struct class1 { }; )"}; +constexpr std::string_view cTestCase2{R"(// Start of a TDL file. This is line#1. +struct class0 { + use_1: Map>>, +}; + +namespace ns1 { + fn func0(input: class0) -> class1; +} + +struct class1 { + use_0: class0, +}; +)"}; + TEST_CASE("DetectUndefinedStruct Case 1", "[tdl][pass][analytics][DetectUndefinedStruct]") { std::istringstream input_stream{std::string{cTestCase1}}; auto const parse_result{parse_translation_unit_from_istream(input_stream)}; @@ -57,6 +71,17 @@ TEST_CASE("DetectUndefinedStruct Case 1", "[tdl][pass][analytics][DetectUndefine }; REQUIRE(cExpectedErrorMessage == error->to_string()); } + +TEST_CASE("DetectUndefinedStruct Case 2", "[tdl][pass][analytics][DetectUndefinedStruct]") { + std::istringstream input_stream{std::string{cTestCase2}}; + auto const parse_result{parse_translation_unit_from_istream(input_stream)}; + REQUIRE_FALSE(parse_result.has_error()); + auto const& translation_unit{parse_result.value()}; + + auto detect_undefined_struct_pass{DetectUndefinedStruct{translation_unit.get()}}; + auto const run_result{detect_undefined_struct_pass.run()}; + REQUIRE_FALSE(run_result.has_error()); +} } // namespace // NOLINTEND(cert-err58-cpp,cppcoreguidelines-avoid-do-while,readability-function-cognitive-complexity,cppcoreguidelines-avoid-non-const-global-variables,cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays) From bb31f912b6eaed032cd2310cfc3564cc017d33ae Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sun, 28 Sep 2025 18:07:10 -0400 Subject: [PATCH 3/3] Add circular dependency checker pass. --- src/spider/CMakeLists.txt | 2 + .../parser/ast/node_impl/TranslationUnit.hpp | 8 +- .../DetectStructCircularDependency.cpp | 85 +++++++++++++++++++ .../DetectStructCircularDependency.hpp | 63 ++++++++++++++ tests/tdl/test-parser.cpp | 4 +- ...ass-analysis-StructSpecDependencyGraph.cpp | 67 ++++++++++++--- 6 files changed, 213 insertions(+), 16 deletions(-) create mode 100644 src/spider/tdl/pass/analysis/DetectStructCircularDependency.cpp create mode 100644 src/spider/tdl/pass/analysis/DetectStructCircularDependency.hpp diff --git a/src/spider/CMakeLists.txt b/src/spider/CMakeLists.txt index b896a91a..7b54f497 100644 --- a/src/spider/CMakeLists.txt +++ b/src/spider/CMakeLists.txt @@ -227,6 +227,7 @@ set(SPIDER_TDL_SHARED_SOURCES tdl/parser/ast/node_impl/type_impl/Struct.cpp tdl/parser/ast/utils.cpp tdl/parser/parse.cpp + tdl/pass/analysis/DetectStructCircularDependency.cpp tdl/pass/analysis/DetectUndefinedStruct.cpp tdl/pass/analysis/StructSpecDependencyGraph.cpp CACHE INTERNAL @@ -260,6 +261,7 @@ set(SPIDER_TDL_SHARED_HEADERS tdl/parser/Exception.hpp tdl/parser/parse.hpp tdl/parser/SourceLocation.hpp + tdl/pass/analysis/DetectStructCircularDependency.hpp tdl/pass/analysis/DetectUndefinedStruct.hpp tdl/pass/analysis/StructSpecDependencyGraph.hpp tdl/pass/Pass.hpp diff --git a/src/spider/tdl/parser/ast/node_impl/TranslationUnit.hpp b/src/spider/tdl/parser/ast/node_impl/TranslationUnit.hpp index fbfdbd0d..9611649a 100644 --- a/src/spider/tdl/parser/ast/node_impl/TranslationUnit.hpp +++ b/src/spider/tdl/parser/ast/node_impl/TranslationUnit.hpp @@ -105,12 +105,12 @@ class TranslationUnit : public Node { -> ystdlib::error_handling::Result; /** - * @return A newly constructed dependency graph of struct specs defined in this translation - * unit. + * @return A shared pointer pointing to a newly constructed dependency graph of struct specs + * defined in this translation unit. */ [[nodiscard]] auto create_struct_spec_dependency_graph() const - -> pass::analysis::StructSpecDependencyGraph { - return pass::analysis::StructSpecDependencyGraph{m_struct_spec_table}; + -> std::shared_ptr { + return std::make_shared(m_struct_spec_table); } private: diff --git a/src/spider/tdl/pass/analysis/DetectStructCircularDependency.cpp b/src/spider/tdl/pass/analysis/DetectStructCircularDependency.cpp new file mode 100644 index 00000000..9afe0479 --- /dev/null +++ b/src/spider/tdl/pass/analysis/DetectStructCircularDependency.cpp @@ -0,0 +1,85 @@ +#include "DetectStructCircularDependency.hpp" + +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#include +#include + +namespace spider::tdl::pass::analysis { +auto DetectStructCircularDependency::Error::to_string() const -> std::string { + std::vector circular_dependency_group_error_messages; + circular_dependency_group_error_messages.reserve(m_strongly_connected_components.size()); + for (auto const& group : m_strongly_connected_components) { + std::vector struct_descriptions; + struct_descriptions.reserve(group.size()); + for (auto const& struct_spec : group) { + struct_descriptions.emplace_back( + fmt::format( + " `{}` at {}", + struct_spec->get_name(), + struct_spec->get_source_location().serialize_to_str() + ) + ); + } + circular_dependency_group_error_messages.emplace_back( + fmt::format( + "Found a circular dependency group of {} struct spec(s):\n{}", + group.size(), + fmt::join(struct_descriptions, "\n") + ) + ); + } + return fmt::format( + "Found {} circular dependency group(s):\n{}", + m_strongly_connected_components.size(), + fmt::join(circular_dependency_group_error_messages, "\n") + ); +} + +auto DetectStructCircularDependency::run() + -> boost::outcome_v2::std_checked> { + auto const& strongly_connected_components{ + m_struct_spec_dependency_graph->get_strongly_connected_components() + }; + if (strongly_connected_components.empty()) { + return boost::outcome_v2::success(); + } + + std::vector>> + circular_dependency_groups; + circular_dependency_groups.reserve(strongly_connected_components.size()); + for (auto const& scc : strongly_connected_components) { + std::vector> group; + group.reserve(scc.size()); + for (auto const id : scc) { + group.emplace_back(m_struct_spec_dependency_graph->get_struct_spec_from_id(id)); + } + std::ranges::sort(group, [](auto const& lhs, auto const& rhs) -> bool { + return lhs->get_source_location() < rhs->get_source_location(); + }); + circular_dependency_groups.emplace_back(std::move(group)); + } + + std::ranges::sort(circular_dependency_groups, [](auto const& lhs, auto const& rhs) -> bool { + // Compare by the source location of the first struct spec in each group. This is safe + // because: + // - Each group is guaranteed to be non-empty. + // - Each struct spec should only appear in one SCC, which guarantees the source locations + // are unique. + return lhs.front()->get_source_location() < rhs.front()->get_source_location(); + }); + + return boost::outcome_v2::failure( + std::make_unique(std::move(circular_dependency_groups)) + ); +} +} // namespace spider::tdl::pass::analysis diff --git a/src/spider/tdl/pass/analysis/DetectStructCircularDependency.hpp b/src/spider/tdl/pass/analysis/DetectStructCircularDependency.hpp new file mode 100644 index 00000000..7ffeb775 --- /dev/null +++ b/src/spider/tdl/pass/analysis/DetectStructCircularDependency.hpp @@ -0,0 +1,63 @@ +#ifndef SPIDER_TDL_PASS_ANALYSIS_DETECTSTRUCTCIRCULARDEPENDENCY_HPP +#define SPIDER_TDL_PASS_ANALYSIS_DETECTSTRUCTCIRCULARDEPENDENCY_HPP + +#include +#include +#include +#include + +#include + +#include +#include +#include + +namespace spider::tdl::pass::analysis { +/** + * Wrapper of `StructSpecDependencyGraph` to detect circular dependencies among struct specs. + */ +class DetectStructCircularDependency : public Pass { +public: + // Types + /** + * Represents an error including all circular dependency groups (reported as strongly connected + * components). + */ + class Error : public Pass::Error { + public: + // Constructor + explicit Error( + std::vector>> + strongly_connected_components + ) + : m_strongly_connected_components{std::move(strongly_connected_components)} {} + + // Methods implementing `Pass::Error` + [[nodiscard]] auto to_string() const -> std::string override; + + private: + // Variables + std::vector>> + m_strongly_connected_components; + }; + + // Constructor + explicit DetectStructCircularDependency( + std::shared_ptr struct_spec_dependency_graph + ) + : m_struct_spec_dependency_graph{std::move(struct_spec_dependency_graph)} {} + + // Methods implementing `Pass` + /** + * @return A void result on success, or a pointer to `DetectStructCircularDependency::Error` + * on failure. + */ + [[nodiscard]] auto run() + -> boost::outcome_v2::std_checked> override; + +private: + std::shared_ptr m_struct_spec_dependency_graph; +}; +} // namespace spider::tdl::pass::analysis + +#endif // SPIDER_TDL_PASS_ANALYSIS_DETECTSTRUCTCIRCULARDEPENDENCY_HPP diff --git a/tests/tdl/test-parser.cpp b/tests/tdl/test-parser.cpp index ce9c5b00..b3f810ba 100644 --- a/tests/tdl/test-parser.cpp +++ b/tests/tdl/test-parser.cpp @@ -273,8 +273,8 @@ TEST_CASE("Parsing `cTestInput1`", "[tdl][parser]") { REQUIRE(serialize_result.value() == cExpectedSerializedAst); auto struct_spec_dependency_graph{translation_unit->create_struct_spec_dependency_graph()}; - REQUIRE(struct_spec_dependency_graph.get_num_struct_specs() == 2); - REQUIRE(struct_spec_dependency_graph.get_strongly_connected_components().empty()); + REQUIRE(struct_spec_dependency_graph->get_num_struct_specs() == 2); + REQUIRE(struct_spec_dependency_graph->get_strongly_connected_components().empty()); } TEST_CASE("Parser errors", "[tdl][parser]") { diff --git a/tests/tdl/test-pass-analysis-StructSpecDependencyGraph.cpp b/tests/tdl/test-pass-analysis-StructSpecDependencyGraph.cpp index 36cbabb6..040cf142 100644 --- a/tests/tdl/test-pass-analysis-StructSpecDependencyGraph.cpp +++ b/tests/tdl/test-pass-analysis-StructSpecDependencyGraph.cpp @@ -12,10 +12,12 @@ #include #include +#include #include namespace { using spider::tdl::parser::parse_translation_unit_from_istream; +using spider::tdl::pass::analysis::DetectStructCircularDependency; using spider::tdl::pass::analysis::StructSpecDependencyGraph; constexpr std::string_view cTestCase1{R"(// Start of a TDL file. This is line#1. @@ -192,10 +194,10 @@ TEST_CASE("StructSpecDependencyGraph Case 1", "[tdl][pass][analytics][StructSpec auto const& translation_unit{parse_result.value()}; auto struct_spec_dependency_graph{translation_unit->create_struct_spec_dependency_graph()}; - REQUIRE(struct_spec_dependency_graph.get_num_struct_specs() == 7); + REQUIRE(struct_spec_dependency_graph->get_num_struct_specs() == 7); auto const serialized_strongly_connected_components{ - serialize_strongly_connected_components(struct_spec_dependency_graph) + serialize_strongly_connected_components(*struct_spec_dependency_graph) }; REQUIRE(serialized_strongly_connected_components.size() == 4); std::set const expected_serialized_strongly_connected_components{ @@ -208,8 +210,31 @@ TEST_CASE("StructSpecDependencyGraph Case 1", "[tdl][pass][analytics][StructSpec == expected_serialized_strongly_connected_components); REQUIRE_FALSE( - struct_spec_dependency_graph.get_struct_specs_in_topological_ordering().has_value() + struct_spec_dependency_graph->get_struct_specs_in_topological_ordering().has_value() ); + + DetectStructCircularDependency detect_circular_dependency_pass{struct_spec_dependency_graph}; + auto const run_result{detect_circular_dependency_pass.run()}; + REQUIRE(run_result.has_error()); + auto const* error{ + dynamic_cast(run_result.error().get()) + }; + REQUIRE(nullptr != error); + constexpr std::string_view cExpectedErrorMessage{ + "Found 4 circular dependency group(s):\n" + "Found a circular dependency group of 3 struct spec(s):\n" + " `class0` at (20:0)\n" + " `class1` at (25:0)\n" + " `class3` at (34:0)\n" + "Found a circular dependency group of 2 struct spec(s):\n" + " `class2` at (29:0)\n" + " `class4` at (38:0)\n" + "Found a circular dependency group of 1 struct spec(s):\n" + " `class5` at (42:0)\n" + "Found a circular dependency group of 1 struct spec(s):\n" + " `class6` at (46:0)" + }; + REQUIRE(cExpectedErrorMessage == error->to_string()); } TEST_CASE("StructSpecDependencyGraphCase 2", "[tdl][pass][analytics][StructSpecDependencyGraph]") { @@ -219,10 +244,10 @@ TEST_CASE("StructSpecDependencyGraphCase 2", "[tdl][pass][analytics][StructSpecD auto const& translation_unit{parse_result.value()}; auto struct_spec_dependency_graph{translation_unit->create_struct_spec_dependency_graph()}; - REQUIRE(struct_spec_dependency_graph.get_num_struct_specs() == 6); + REQUIRE(struct_spec_dependency_graph->get_num_struct_specs() == 6); auto const serialized_strongly_connected_components{ - serialize_strongly_connected_components(struct_spec_dependency_graph) + serialize_strongly_connected_components(*struct_spec_dependency_graph) }; REQUIRE(serialized_strongly_connected_components.size() == 1); std::set const expected_serialized_strongly_connected_components{ @@ -232,8 +257,26 @@ TEST_CASE("StructSpecDependencyGraphCase 2", "[tdl][pass][analytics][StructSpecD == expected_serialized_strongly_connected_components); REQUIRE_FALSE( - struct_spec_dependency_graph.get_struct_specs_in_topological_ordering().has_value() + struct_spec_dependency_graph->get_struct_specs_in_topological_ordering().has_value() ); + + DetectStructCircularDependency detect_circular_dependency_pass{struct_spec_dependency_graph}; + auto const run_result{detect_circular_dependency_pass.run()}; + REQUIRE(run_result.has_error()); + auto const* error{ + dynamic_cast(run_result.error().get()) + }; + REQUIRE(nullptr != error); + constexpr std::string_view cExpectedErrorMessage{ + "Found 1 circular dependency group(s):\n" + "Found a circular dependency group of 5 struct spec(s):\n" + " `class0` at (12:0)\n" + " `class1` at (16:0)\n" + " `class2` at (20:0)\n" + " `class3` at (25:0)\n" + " `class4` at (29:0)" + }; + REQUIRE(cExpectedErrorMessage == error->to_string()); } TEST_CASE("StructSpecDependencyGraph Case 3", "[tdl][pass][analytics][StructSpecDependencyGraph]") { @@ -243,15 +286,19 @@ TEST_CASE("StructSpecDependencyGraph Case 3", "[tdl][pass][analytics][StructSpec auto const& translation_unit{parse_result.value()}; auto struct_spec_dependency_graph{translation_unit->create_struct_spec_dependency_graph()}; - REQUIRE(struct_spec_dependency_graph.get_num_struct_specs() == 7); + REQUIRE(struct_spec_dependency_graph->get_num_struct_specs() == 7); auto const serialized_strongly_connected_components{ - serialize_strongly_connected_components(struct_spec_dependency_graph) + serialize_strongly_connected_components(*struct_spec_dependency_graph) }; REQUIRE(serialized_strongly_connected_components.empty()); + DetectStructCircularDependency detect_circular_dependency_pass{struct_spec_dependency_graph}; + auto const run_result{detect_circular_dependency_pass.run()}; + REQUIRE_FALSE(run_result.has_error()); + auto const optional_topological_ordering{ - struct_spec_dependency_graph.get_struct_specs_in_topological_ordering() + struct_spec_dependency_graph->get_struct_specs_in_topological_ordering() }; REQUIRE(optional_topological_ordering.has_value()); std::vector topological_ordering_struct_spec_names; @@ -259,7 +306,7 @@ TEST_CASE("StructSpecDependencyGraph Case 3", "[tdl][pass][analytics][StructSpec topological_ordering_struct_spec_names.reserve(optional_topological_ordering->size()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) for (auto const id : *optional_topological_ordering) { - auto const struct_spec{struct_spec_dependency_graph.get_struct_spec_from_id(id)}; + auto const struct_spec{struct_spec_dependency_graph->get_struct_spec_from_id(id)}; REQUIRE(nullptr != struct_spec); topological_ordering_struct_spec_names.emplace_back(struct_spec->get_name()); }