From d4d749ae3484ef702dd6ed38b3d8a55fd61276e2 Mon Sep 17 00:00:00 2001 From: Bartek Kryza Date: Sat, 4 Mar 2023 11:51:52 +0100 Subject: [PATCH 1/3] Fixed nested anonymous namespace regression --- .../visitor/translation_unit_visitor.cc | 34 +++++++++++-------- src/common/clang_utils.cc | 17 ---------- src/common/clang_utils.h | 3 -- 3 files changed, 20 insertions(+), 34 deletions(-) diff --git a/src/class_diagram/visitor/translation_unit_visitor.cc b/src/class_diagram/visitor/translation_unit_visitor.cc index dfec7160..8f082185 100644 --- a/src/class_diagram/visitor/translation_unit_visitor.cc +++ b/src/class_diagram/visitor/translation_unit_visitor.cc @@ -167,11 +167,6 @@ bool translation_unit_visitor::VisitEnumDecl(clang::EnumDecl *enm) e.constants().push_back(ev->getNameAsString()); } - auto namespace_declaration = common::get_enclosing_namespace(enm); - if (namespace_declaration.has_value()) { - e.set_namespace(namespace_declaration.value()); - } - if (diagram().should_include(qualified_name)) diagram().add_enum(std::move(e_ptr)); @@ -202,10 +197,13 @@ bool translation_unit_visitor::VisitClassTemplateSpecializationDecl( auto &template_specialization = *template_specialization_ptr; - process_template_specialization_children(cls, template_specialization); + if (cls->hasBody()) { + process_template_specialization_children(cls, template_specialization); + } - // Process template specialization bases - process_class_bases(cls, template_specialization); + if (cls->hasDefinition()) + // Process template specialization bases + process_class_bases(cls, template_specialization); const auto maybe_id = get_ast_local_id(cls->getSpecializedTemplate()->getID()); @@ -1035,10 +1033,8 @@ void translation_unit_visitor::process_record_containment( auto parent_name = static_cast(parent) ->getQualifiedNameAsString(); - auto namespace_declaration = common::get_enclosing_namespace(parent); - if (namespace_declaration.has_value()) { - element.set_namespace(namespace_declaration.value()); - } + auto namespace_declaration = common::get_tag_namespace(record); + element.set_namespace(namespace_declaration); if (const auto *record_decl = clang::dyn_cast(record.getParent()); @@ -1150,8 +1146,10 @@ void translation_unit_visitor::process_template_specialization_children( } } - for (const auto *friend_declaration : cls->friends()) { - process_friend(*friend_declaration, c); + if (cls->hasFriends()) { + for (const auto *friend_declaration : cls->friends()) { + process_friend(*friend_declaration, c); + } } } @@ -1747,6 +1745,14 @@ translation_unit_visitor::process_template_specialization( template_instantiation.is_struct(cls->isStruct()); + process_record_parent(cls, template_instantiation, namespace_{}); + + if (!template_instantiation.is_nested()) { + template_instantiation.set_name(common::get_tag_name(*cls)); + template_instantiation.set_id( + common::to_id(template_instantiation.full_name(false))); + } + process_comment(*cls, template_instantiation); set_source_location(*cls, template_instantiation); diff --git a/src/common/clang_utils.cc b/src/common/clang_utils.cc index 33b088f5..543a40b3 100644 --- a/src/common/clang_utils.cc +++ b/src/common/clang_utils.cc @@ -43,23 +43,6 @@ model::access_t access_specifier_to_access_t( return access; } -std::optional get_enclosing_namespace( - const clang::DeclContext *decl) -{ - if (!decl->getEnclosingNamespaceContext()->isNamespace()) - return {}; - - const auto *namespace_declaration = - clang::cast(decl->getEnclosingNamespaceContext()); - - if (namespace_declaration == nullptr) { - return {}; - } - - return clanguml::common::model::namespace_{ - common::get_qualified_name(*namespace_declaration)}; -} - model::namespace_ get_tag_namespace(const clang::TagDecl &declaration) { model::namespace_ ns; diff --git a/src/common/clang_utils.h b/src/common/clang_utils.h index 4cc0d547..0ed46aa5 100644 --- a/src/common/clang_utils.h +++ b/src/common/clang_utils.h @@ -75,9 +75,6 @@ model::namespace_ get_tag_namespace(const clang::TagDecl &declaration); model::namespace_ get_template_namespace( const clang::TemplateDecl &declaration); -std::optional get_enclosing_namespace( - const clang::DeclContext *decl); - std::string to_string(const clang::QualType &type, const clang::ASTContext &ctx, bool try_canonical = true); From a58b633d01e5cce13c038e277a1e5a0fbb4794bc Mon Sep 17 00:00:00 2001 From: Bartek Kryza Date: Sat, 4 Mar 2023 15:02:35 +0100 Subject: [PATCH 2/3] Fixed handling of template class specializations nested in other classes --- .../visitor/translation_unit_visitor.cc | 46 +++++++++++++++---- .../visitor/translation_unit_visitor.h | 10 ++++ src/common/model/diagram_element.cc | 4 +- tests/t00008/t00008.cc | 15 ++++++ tests/t00008/test_case.h | 7 +++ 5 files changed, 71 insertions(+), 11 deletions(-) diff --git a/src/class_diagram/visitor/translation_unit_visitor.cc b/src/class_diagram/visitor/translation_unit_visitor.cc index 8f082185..d80d1e68 100644 --- a/src/class_diagram/visitor/translation_unit_visitor.cc +++ b/src/class_diagram/visitor/translation_unit_visitor.cc @@ -182,9 +182,13 @@ bool translation_unit_visitor::VisitClassTemplateSpecializationDecl( if (!diagram().should_include(cls->getQualifiedNameAsString())) return true; - LOG_DBG("= Visiting template specialization declaration {} at {}", + LOG_DBG("= Visiting template specialization declaration {} at {} " + "(described class id {})", cls->getQualifiedNameAsString(), - cls->getLocation().printToString(source_manager())); + cls->getLocation().printToString(source_manager()), + cls->getSpecializedTemplate() + ? cls->getSpecializedTemplate()->getTemplatedDecl()->getID() + : 0); // TODO: Add support for classes defined in function/method bodies if (cls->isLocalClass() != nullptr) @@ -212,10 +216,11 @@ bool translation_unit_visitor::VisitClassTemplateSpecializationDecl( {relationship_t::kInstantiation, maybe_id.value()}); if (diagram_.should_include(template_specialization)) { - const auto name = template_specialization.full_name(false); + const auto full_name = template_specialization.full_name(false); const auto id = template_specialization.id(); - LOG_DBG("Adding class template specialization {} with id {}", name, id); + LOG_DBG("Adding class template specialization {} with id {}", full_name, + id); diagram_.add_class(std::move(template_specialization_ptr)); } @@ -280,6 +285,8 @@ bool translation_unit_visitor::VisitClassTemplateDecl( if (!c_ptr) return true; + add_processed_template_class(cls->getQualifiedNameAsString()); + // Override the id with the template id, for now we don't care about the // underlying templated class id @@ -679,12 +686,18 @@ bool translation_unit_visitor::VisitCXXRecordDecl(clang::CXXRecordDecl *cls) LOG_DBG("== isTemplateDecl() = {}", cls->isTemplateDecl()); LOG_DBG("== isTemplated() = {}", cls->isTemplated()); LOG_DBG("== getParent()->isRecord()() = {}", cls->getParent()->isRecord()); - if (cls->getParent()->isRecord()) { + if (const auto *parent_record = + clang::dyn_cast(cls->getParent()); + parent_record != nullptr) { LOG_DBG("== getParent()->getQualifiedNameAsString() = {}", - clang::dyn_cast(cls->getParent()) - ->getQualifiedNameAsString()); + parent_record->getQualifiedNameAsString()); } + if (has_processed_template_class(cls->getQualifiedNameAsString())) + // If we have already processed the template of this class + // skip it + return true; + if (cls->isTemplated() && (cls->getDescribedTemplate() != nullptr)) { // If the described templated of this class is already in the model // skip it: @@ -837,11 +850,14 @@ void translation_unit_visitor::process_record_parent( std::optional id_opt; + auto parent_ns = ns; if (parent != nullptr) { const auto *parent_record_decl = clang::dyn_cast(parent); if (parent_record_decl != nullptr) { + parent_ns = common::get_tag_namespace(*parent_record_decl); + int64_t local_id = parent_record_decl->getID(); // First check if the parent has been added to the diagram as @@ -868,7 +884,7 @@ void translation_unit_visitor::process_record_parent( assert(parent_class); - c.set_namespace(ns); + c.set_namespace(parent_ns); const auto cls_name = cls->getNameAsString(); if (cls_name.empty()) { // Nested structs can be anonymous @@ -1745,7 +1761,7 @@ translation_unit_visitor::process_template_specialization( template_instantiation.is_struct(cls->isStruct()); - process_record_parent(cls, template_instantiation, namespace_{}); + process_record_parent(cls, template_instantiation, ns); if (!template_instantiation.is_nested()) { template_instantiation.set_name(common::get_tag_name(*cls)); @@ -2833,4 +2849,16 @@ bool translation_unit_visitor::should_include(const clang::NamedDecl *decl) diagram().should_include(decl->getQualifiedNameAsString()); } +void translation_unit_visitor::add_processed_template_class( + std::string qualified_name) +{ + processed_template_qualified_names_.emplace(std::move(qualified_name)); +} + +bool translation_unit_visitor::has_processed_template_class( + const std::string &qualified_name) const +{ + return util::contains(processed_template_qualified_names_, qualified_name); +} + } // namespace clanguml::class_diagram::visitor diff --git a/src/class_diagram/visitor/translation_unit_visitor.h b/src/class_diagram/visitor/translation_unit_visitor.h index 351ac8a1..22cbf048 100644 --- a/src/class_diagram/visitor/translation_unit_visitor.h +++ b/src/class_diagram/visitor/translation_unit_visitor.h @@ -287,6 +287,10 @@ private: void set_ast_local_id( int64_t local_id, common::model::diagram_element::id_t global_id); + void add_processed_template_class(std::string qualified_name); + + bool has_processed_template_class(const std::string &qualified_name) const; + /// Retrieve the global clang-uml entity id based on the clang local id std::optional get_ast_local_id( int64_t local_id) const; @@ -307,5 +311,11 @@ private: std::tuple> anonymous_struct_relationships_; + + // When visiting CXX records we need to know if they have already been + // process in VisitClassTemplateDecl or VisitClassTemplateSpecializationDecl + // If yes, then we need to skip it + // TODO: There must be a better way to do this... + std::set processed_template_qualified_names_; }; } // namespace clanguml::class_diagram::visitor diff --git a/src/common/model/diagram_element.cc b/src/common/model/diagram_element.cc index c4d01756..e8392e28 100644 --- a/src/common/model/diagram_element.cc +++ b/src/common/model/diagram_element.cc @@ -46,8 +46,8 @@ void diagram_element::add_relationship(relationship &&cr) return; } - LOG_DBG("Adding relationship: '{}' - {} - '{}'", cr.destination(), - to_string(cr.type()), full_name(true)); + LOG_DBG("Adding relationship from: '{}' ({}) - {} - '{}'", id(), + full_name(true), to_string(cr.type()), cr.destination()); if (!util::contains(relationships_, cr)) relationships_.emplace_back(std::move(cr)); diff --git a/tests/t00008/t00008.cc b/tests/t00008/t00008.cc index f711fda7..11c1ec02 100644 --- a/tests/t00008/t00008.cc +++ b/tests/t00008/t00008.cc @@ -32,5 +32,20 @@ struct D { void add(int i) { ints.template_template.values.push_back(i); } }; + +struct E { + template struct nested_template { + using DT = ET; + + static DT *get(ET *d) { return d; } + }; +}; + +template <> struct E::nested_template { + using DeclType = char; + + static DeclType *getDecl(char *c) { return c; } +}; + } // namespace t00008 } // namespace clanguml diff --git a/tests/t00008/test_case.h b/tests/t00008/test_case.h index 2f216a71..446541d0 100644 --- a/tests/t00008/test_case.h +++ b/tests/t00008/test_case.h @@ -48,5 +48,12 @@ TEST_CASE("t00008", "[test-case][class]") // REQUIRE_THAT(puml, IsField(Public("bool (*)(int, int) comparator"))); REQUIRE_THAT(puml, (IsField("comparator", "CMP"))); + REQUIRE_THAT(puml, !IsClass(_A("E::nested_template"))); + REQUIRE_THAT(puml, IsClassTemplate("E::nested_template", "ET")); + REQUIRE_THAT(puml, IsClassTemplate("E::nested_template", "char")); + REQUIRE_THAT(puml, + IsInstantiation( + _A("E::nested_template"), _A("E::nested_template"))); + save_puml(config.output_directory() + "/" + diagram->name + ".puml", puml); } From cfca79182c48f18065860caa7ca51b48570460d7 Mon Sep 17 00:00:00 2001 From: Bartek Kryza Date: Sat, 4 Mar 2023 21:11:14 +0100 Subject: [PATCH 3/3] Added relationship exclusion to context filter --- src/common/model/diagram_filter.cc | 8 ++++++-- tests/t00041/.clang-uml | 4 +++- tests/t00041/t00041.cc | 4 ++++ tests/t00041/test_case.h | 2 ++ 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/common/model/diagram_filter.cc b/src/common/model/diagram_filter.cc index a4bd4ec2..8e04e573 100644 --- a/src/common/model/diagram_filter.cc +++ b/src/common/model/diagram_filter.cc @@ -301,6 +301,8 @@ tvl::value_t context_filter::match(const diagram &d, const element &e) const if (d.type() != diagram_t::kClass) return {}; + // Running this filter makes sense only after the entire diagram is + // generated - i.e. during diagram generation if (!d.complete()) return {}; @@ -319,11 +321,13 @@ tvl::value_t context_filter::match(const diagram &d, const element &e) const // relationship with any of the context_root's for (const relationship &rel : context_root.value().relationships()) { - if (rel.destination() == e.id()) + if (d.should_include(rel.type()) && + rel.destination() == e.id()) return true; } for (const relationship &rel : e.relationships()) { - if (rel.destination() == context_root.value().id()) + if (d.should_include(rel.type()) && + rel.destination() == context_root.value().id()) return true; } diff --git a/tests/t00041/.clang-uml b/tests/t00041/.clang-uml index e40ae69b..502eba3f 100644 --- a/tests/t00041/.clang-uml +++ b/tests/t00041/.clang-uml @@ -17,4 +17,6 @@ diagrams: - clanguml::t00041::ns1::N exclude: namespaces: - - clanguml::t00041::detail \ No newline at end of file + - clanguml::t00041::detail + relationships: + - dependency \ No newline at end of file diff --git a/tests/t00041/t00041.cc b/tests/t00041/t00041.cc index 5204c843..b7373f6b 100644 --- a/tests/t00041/t00041.cc +++ b/tests/t00041/t00041.cc @@ -22,10 +22,14 @@ namespace detail { struct G { }; } // namespace detail +struct H { }; + struct RR : public R { E *e; F *f; detail::G *g; + + void foo(H *h) { } }; struct RRR : public RR { }; diff --git a/tests/t00041/test_case.h b/tests/t00041/test_case.h index 465c255b..d474e870 100644 --- a/tests/t00041/test_case.h +++ b/tests/t00041/test_case.h @@ -48,6 +48,7 @@ TEST_CASE("t00041", "[test-case][class]") REQUIRE_THAT(puml, IsClass(_A("RR"))); REQUIRE_THAT(puml, IsClass(_A("RRR"))); REQUIRE_THAT(puml, !IsClass(_A("detail::G"))); + REQUIRE_THAT(puml, !IsClass(_A("H"))); REQUIRE_THAT(puml, IsBaseClass(_A("R"), _A("RR"))); REQUIRE_THAT(puml, IsBaseClass(_A("RR"), _A("RRR"))); @@ -55,6 +56,7 @@ TEST_CASE("t00041", "[test-case][class]") REQUIRE_THAT(puml, IsAssociation(_A("D"), _A("RR"), "+rr")); REQUIRE_THAT(puml, IsAssociation(_A("RR"), _A("E"), "+e")); REQUIRE_THAT(puml, IsAssociation(_A("RR"), _A("F"), "+f")); + REQUIRE_THAT(puml, !IsDependency(_A("RR"), _A("H"))); REQUIRE_THAT(puml, IsClass(_A("ns1::N"))); REQUIRE_THAT(puml, IsClass(_A("ns1::NN")));