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 6039c5dc..6caac46c 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()); }