From 2d1cd9fd8701d055ebe38f007cb01bbea0869cea Mon Sep 17 00:00:00 2001 From: Bartek Kryza Date: Sat, 1 Mar 2025 11:43:14 +0100 Subject: [PATCH] Fixed handling of diagram filters for forward declared elements (#392) --- .../plantuml/class_diagram_generator.cc | 6 ++--- src/class_diagram/model/diagram.cc | 22 ++++-------------- src/class_diagram/model/diagram.h | 23 +++++++++++++++---- src/common/model/filters/diagram_filter.cc | 15 ++---------- src/common/model/filters/diagram_filter.h | 17 ++++++-------- src/common/model/nested_trait.h | 1 + src/common/visitor/translation_unit_visitor.h | 2 -- .../visitor/translation_unit_visitor.cc | 3 ++- tests/t00082/.clang-uml | 2 +- tests/t00082/t00082.cc | 2 +- tests/t00082/test_case.h | 2 +- uml/sequence/template_builder_sequence.yml | 2 +- 12 files changed, 42 insertions(+), 55 deletions(-) diff --git a/src/class_diagram/generators/plantuml/class_diagram_generator.cc b/src/class_diagram/generators/plantuml/class_diagram_generator.cc index 2972a827..2a02577e 100644 --- a/src/class_diagram/generators/plantuml/class_diagram_generator.cc +++ b/src/class_diagram/generators/plantuml/class_diagram_generator.cc @@ -586,7 +586,7 @@ void generator::generate_relationships( std::set unique_relations; for (const auto &r : c.relationships()) { - LOG_DBG("== Processing relationship {}", + LOG_TRACE("== Processing relationship {}", plantuml_common::to_plantuml(r, config())); std::stringstream relstr; @@ -668,7 +668,7 @@ void generator::generate_relationships( if (!model().should_include(r.type())) continue; - LOG_DBG("== Processing relationship {}", to_string(r.type())); + LOG_TRACE("== Processing relationship {}", to_string(r.type())); std::stringstream relstr; eid_t destination{}; @@ -804,7 +804,7 @@ void generator::generate_relationships( std::set unique_relations; for (const auto &r : c.relationships()) { - LOG_DBG("== Processing relationship {}", + LOG_TRACE("== Processing relationship {}", plantuml_common::to_plantuml(r, config())); std::stringstream relstr; diff --git a/src/class_diagram/model/diagram.cc b/src/class_diagram/model/diagram.cc index c2d611ea..a7a32e3e 100644 --- a/src/class_diagram/model/diagram.cc +++ b/src/class_diagram/model/diagram.cc @@ -210,7 +210,7 @@ bool diagram::has_element(eid_t id) const std::string diagram::to_alias(eid_t id) const { - LOG_DBG("Looking for alias for {}", id); + LOG_TRACE("Looking for alias for {}", id); for (const auto &c : classes()) { if (c.get().id() == id) { @@ -270,23 +270,18 @@ void diagram::remove_redundant_dependencies() void diagram::apply_filter() { -// print_tree(1); - // First find all element ids which should be removed std::set to_remove; -// int lc{0}; -// while (true) { -// LOG_DBG("CLEANING DIAGRAM: {}", lc++); - + while (true) { for_all_elements([&](auto &&elements_view) mutable { for (const auto &el : elements_view) if (!filter().should_include(el.get())) to_remove.emplace(el.get().id()); }); -// if (to_remove.empty()) -// break; + if (to_remove.empty()) + break; element_view::remove(to_remove); element_view::remove(to_remove); @@ -295,22 +290,15 @@ void diagram::apply_filter() nested_trait_ns::remove(to_remove); -// LOG_DBG("REMOVED: {}", to_remove.size()); - to_remove.clear(); filter().reset(); -// } + } for_all_elements([&](auto &&elements_view) mutable { for (const auto &el : elements_view) el.get().apply_filter(filter(), to_remove); }); -// -// LOG_INFO("\n\n\n===========================================================" -// "\n\n\n"); -// -// print_tree(1); } bool diagram::is_empty() const diff --git a/src/class_diagram/model/diagram.h b/src/class_diagram/model/diagram.h index a838a15a..514c2230 100644 --- a/src/class_diagram/model/diagram.h +++ b/src/class_diagram/model/diagram.h @@ -222,10 +222,10 @@ public: parent_path.to_string()); auto e = nested_trait_ns::get_and_remove(id); - if (!e) - return; + assert(e); element_view::remove({id}); + added_elements_.erase(id); this->add(parent_path, std::move(e)); } @@ -279,6 +279,8 @@ public: void apply_filter() override; private: + std::set added_elements_; + template bool add_with_namespace_path(std::unique_ptr &&e); @@ -302,6 +304,11 @@ template bool diagram::contains(const ElementT &element) template bool diagram::add_with_namespace_path(std::unique_ptr &&e) { + if (added_elements_.count(e->id()) > 0) + return true; + + added_elements_.emplace(e->id()); + const auto base_name = e->name(); const auto full_name = e->full_name(false); const auto element_type = e->type_name(); @@ -355,6 +362,11 @@ template bool diagram::add_with_module_path( const common::model::path &parent_path, std::unique_ptr &&e) { + if (added_elements_.count(e->id()) > 0) + return true; + + added_elements_.emplace(e->id()); + const auto element_type = e->type_name(); // Make sure all parent modules are already packages in the @@ -388,12 +400,12 @@ template bool diagram::add_with_filesystem_path( const common::model::path &parent_path, std::unique_ptr &&e) { + if (added_elements_.count(e->id()) > 0) + return false; + LOG_DBG("Adding element {} at path {}", e->full_name(false), parent_path.to_string()); - if(e->full_name(false) == "clanguml::t00014::A") - LOG_DBG("AAAAA"); - const auto element_type = e->type_name(); // Make sure all parent modules are already packages in the @@ -418,6 +430,7 @@ bool diagram::add_with_filesystem_path( auto &e_ref = *e; if (add_element(parent_path, std::move(e))) { + added_elements_.emplace(e_ref.id()); element_view::add(std::ref(e_ref)); return true; } diff --git a/src/common/model/filters/diagram_filter.cc b/src/common/model/filters/diagram_filter.cc index c635d300..70de1602 100644 --- a/src/common/model/filters/diagram_filter.cc +++ b/src/common/model/filters/diagram_filter.cc @@ -991,8 +991,9 @@ void context_filter::initialize_effective_context_class_diagram( .find(context_cfg.pattern); for (const auto &maybe_match : context_matches) { - if (maybe_match) + if (maybe_match) { effective_context.emplace(maybe_match.value().id()); + } } const auto &context_enum_matches = @@ -1028,18 +1029,6 @@ void context_filter::initialize_effective_context_class_diagram( // For each class in the model find_elements_in_direct_relationship( d, context_cfg, effective_context, current_iteration_context); - // - // // For each concept in the model - // find_elements_in_direct_relationship( - // d, context_cfg, effective_context, - // current_iteration_context); - // - // // For each enum in the model - // find_elements_in_direct_relationship( - // d, context_cfg, effective_context, - // current_iteration_context); for (auto id : current_iteration_context) { if (effective_context.count(id) == 0) { diff --git a/src/common/model/filters/diagram_filter.h b/src/common/model/filters/diagram_filter.h index c181b851..4a75bfe2 100644 --- a/src/common/model/filters/diagram_filter.h +++ b/src/common/model/filters/diagram_filter.h @@ -401,7 +401,11 @@ struct edge_traversal_filter : public filter_visitor { ~edge_traversal_filter() override = default; - void reset() override { initialized_ = false, matching_elements_.clear(); }; + void reset() override + { + initialized_ = false; + matching_elements_.clear(); + }; tvl::value_t match(const diagram &d, const MatchOverrideT &e) const override { @@ -626,7 +630,8 @@ private: { if (d.type() == diagram_t::kClass) { for (const auto &p : nt) { - if (auto *pkg = dynamic_cast(p.get()); pkg) { + if (auto *pkg = dynamic_cast(p.get()); + pkg != nullptr) { process_elements(d, *pkg, context_cfg, effective_context, current_iteration_context); } @@ -779,14 +784,6 @@ private: process_elements( cd, cd, context_cfg, effective_context, current_iteration_context); - - // for (const auto &el : cd.template elements()) { - // // First search all elements of type ElementT in the - // diagram - // // which have a relationship to any of the - // effective_context - // // elements - // } } bool should_include( diff --git a/src/common/model/nested_trait.h b/src/common/model/nested_trait.h index baec15cb..8b3d06c8 100644 --- a/src/common/model/nested_trait.h +++ b/src/common/model/nested_trait.h @@ -17,6 +17,7 @@ */ #pragma once +#include "common/types.h" #include "util/util.h" #include diff --git a/src/common/visitor/translation_unit_visitor.h b/src/common/visitor/translation_unit_visitor.h index 6819c54c..a90c36a7 100644 --- a/src/common/visitor/translation_unit_visitor.h +++ b/src/common/visitor/translation_unit_visitor.h @@ -121,8 +121,6 @@ public: void set_source_location(const clang::Decl &decl, clanguml::common::model::source_location &element) { - LOG_DBG("SETTING SOURCE LOCATION TO {}", - decl.getLocation().printToString(source_manager())); set_source_location(decl.getLocation(), element); } diff --git a/src/sequence_diagram/visitor/translation_unit_visitor.cc b/src/sequence_diagram/visitor/translation_unit_visitor.cc index 49e826b9..81b91759 100644 --- a/src/sequence_diagram/visitor/translation_unit_visitor.cc +++ b/src/sequence_diagram/visitor/translation_unit_visitor.cc @@ -2642,7 +2642,8 @@ translation_unit_visitor::build_function_template_instantiation( set_qualified_name(decl, template_instantiation); - tbuilder().build(decl, template_instantiation, &decl, decl.getPrimaryTemplate(), + tbuilder().build(decl, template_instantiation, &decl, + decl.getPrimaryTemplate(), decl.getTemplateSpecializationArgs()->asArray(), common::to_string(&decl)); diff --git a/tests/t00082/.clang-uml b/tests/t00082/.clang-uml index 54f1fc9f..89c4d71f 100644 --- a/tests/t00082/.clang-uml +++ b/tests/t00082/.clang-uml @@ -16,5 +16,5 @@ diagrams: exclude: allof: elements: - - clanguml::t00082::ns1::nsA::A1 + - clanguml::t00082::ns1::nsA::D1 using_namespace: clanguml::t00082 \ No newline at end of file diff --git a/tests/t00082/t00082.cc b/tests/t00082/t00082.cc index d2f1c094..4bcb5e4c 100644 --- a/tests/t00082/t00082.cc +++ b/tests/t00082/t00082.cc @@ -4,7 +4,7 @@ namespace nsA { struct A1 { }; struct B1 : public A1 { }; struct C1 : public B1 { }; -struct D1 { }; +struct D1 : public C1 { }; } } namespace ns2 { diff --git a/tests/t00082/test_case.h b/tests/t00082/test_case.h index cc585e78..1eb496a0 100644 --- a/tests/t00082/test_case.h +++ b/tests/t00082/test_case.h @@ -25,7 +25,7 @@ TEST_CASE("t00082") CHECK_CLASS_MODEL("t00082", "t00082_class"); CHECK_CLASS_DIAGRAM(*config, diagram, *model, [](const auto &src) { - REQUIRE(!IsClass(src, {"ns1::nsA", "A1"})); + REQUIRE(IsClass(src, {"ns1::nsA", "A1"})); REQUIRE(IsClass(src, {"ns1::nsA", "B1"})); REQUIRE(IsClass(src, {"ns1::nsA", "C1"})); REQUIRE(!IsClass(src, {"ns1::nsA", "D1"})); diff --git a/uml/sequence/template_builder_sequence.yml b/uml/sequence/template_builder_sequence.yml index 57f3d499..9fd1812c 100644 --- a/uml/sequence/template_builder_sequence.yml +++ b/uml/sequence/template_builder_sequence.yml @@ -18,4 +18,4 @@ exclude: using_namespace: - clanguml from: - - function: "clanguml::common::visitor::template_builder::build(common::model::template_element &,const clang::NamedDecl *,const clang::TemplateDecl *,const clang::ArrayRef,std::string,std::optional)" + - function: "clanguml::common::visitor::template_builder::build(const clang::NamedDecl &,common::model::template_element &,const clang::NamedDecl *,const clang::TemplateDecl *,const clang::ArrayRef,std::string,std::optional)"