From 684282540229f9d362b8d59dc7bc0e3080fe22df Mon Sep 17 00:00:00 2001 From: Bartek Kryza Date: Sat, 10 Sep 2022 01:38:35 +0200 Subject: [PATCH] Fixed handling of nested classes in templates and anonymous nested structs --- CHANGELOG.md | 2 + src/class_diagram/model/diagram.cc | 3 - .../visitor/translation_unit_visitor.cc | 216 ++++++++++++++---- .../visitor/translation_unit_visitor.h | 10 +- src/common/clang_utils.cc | 28 +++ src/common/clang_utils.h | 2 + tests/t00004/t00004.cc | 6 +- tests/t00037/t00037.cc | 19 +- tests/t00037/test_case.h | 7 +- 9 files changed, 234 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87429663..d9e10d05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # CHANGELOG +### + * Fixed handling of classes nested in templates and anonymous nested structs * Fixed handling of configurable type aliases ### 0.2.0 diff --git a/src/class_diagram/model/diagram.cc b/src/class_diagram/model/diagram.cc index dd1128d0..bff5b3da 100644 --- a/src/class_diagram/model/diagram.cc +++ b/src/class_diagram/model/diagram.cc @@ -154,9 +154,6 @@ bool diagram::add_class(std::unique_ptr &&c) if (util::contains(base_name, "::")) throw std::runtime_error("Name cannot contain namespace: " + base_name); - if (util::contains(base_name, "<")) - throw std::runtime_error("Name cannot contain <: " + base_name); - if (util::contains(base_name, "*")) throw std::runtime_error("Name cannot contain *: " + base_name); diff --git a/src/class_diagram/visitor/translation_unit_visitor.cc b/src/class_diagram/visitor/translation_unit_visitor.cc index 0b50a477..199e3700 100644 --- a/src/class_diagram/visitor/translation_unit_visitor.cc +++ b/src/class_diagram/visitor/translation_unit_visitor.cc @@ -144,12 +144,50 @@ bool translation_unit_visitor::VisitEnumDecl(clang::EnumDecl *enm) auto &e = *e_ptr; std::string qualified_name = common::get_qualified_name(*enm); - namespace_ ns{qualified_name}; - ns.pop_back(); - e.set_name(common::get_tag_name(*enm)); - e.set_namespace(ns); - e.set_id(common::to_id(*enm)); + auto ns{common::get_tag_namespace(*enm)}; + + const auto *parent = enm->getParent(); + + if (parent && parent->isRecord()) { + // Here we have 2 options, either: + // - the parent is a regular C++ class/struct + // - the parent is a class template declaration/specialization + std::optional id_opt; + int64_t local_id = + static_cast(parent)->getID(); + + id_opt = get_ast_local_id(local_id); + + // If not, check if the parent template declaration is in the model + if (!id_opt) { + local_id = static_cast(parent) + ->getDescribedTemplate() + ->getID(); + if (static_cast(parent) + ->getDescribedTemplate()) + id_opt = get_ast_local_id(local_id); + } + + assert(id_opt); + + auto parent_class = diagram_.get_class(*id_opt); + + assert(parent_class); + + e.set_namespace(ns); + e.set_name(parent_class.value().full_name(true) + "##" + + enm->getNameAsString()); + e.set_id(common::to_id(e.full_name(false))); + e.add_relationship({relationship_t::kContainment, *id_opt}); + e.nested(true); + } + else { + e.set_name(common::get_tag_name(*enm)); + e.set_namespace(ns); + e.set_id(common::to_id(e.full_name(false))); + } + set_ast_local_id(enm->getID(), e.id()); process_comment(*enm, e); @@ -164,10 +202,6 @@ bool translation_unit_visitor::VisitEnumDecl(clang::EnumDecl *enm) e.constants().push_back(ev->getNameAsString()); } - if (enm->getParent()->isRecord()) { - process_record_containment(*enm, e); - } - auto namespace_declaration = common::get_enclosing_namespace(enm); if (namespace_declaration.has_value()) { e.set_namespace(namespace_declaration.value()); @@ -324,15 +358,13 @@ bool translation_unit_visitor::VisitCXXRecordDecl(clang::CXXRecordDecl *cls) cls->getQualifiedNameAsString(), cls->getLocation().printToString(source_manager_)); - const auto cls_id = common::to_id(*cls); - - set_ast_local_id(cls->getID(), cls_id); - - // Templated records are handled by VisitClassTemplateDecl() - if (cls->isTemplated() || cls->isTemplateDecl() || - (clang::dyn_cast_or_null(cls) != - nullptr)) - return true; + if (!cls->getParent()->isRecord()) + // Templated records are handled by VisitClassTemplateDecl() + // unless they are nested in template classes + if (cls->isTemplated() || cls->isTemplateDecl() || + (clang::dyn_cast_or_null( + cls) != nullptr)) + return true; // TODO: Add support for classes defined in function/method bodies if (cls->isLocalClass()) @@ -343,6 +375,10 @@ bool translation_unit_visitor::VisitCXXRecordDecl(clang::CXXRecordDecl *cls) if (!c_ptr) return true; + const auto cls_id = c_ptr->id(); + + set_ast_local_id(cls->getID(), cls_id); + auto &class_model = diagram().get_class(cls_id).has_value() ? *diagram().get_class(cls_id).get() : *c_ptr; @@ -381,17 +417,82 @@ std::unique_ptr translation_unit_visitor::create_class_declaration( auto &c = *c_ptr; // TODO: refactor to method get_qualified_name() - auto qualified_name = common::get_qualified_name(*cls); + auto qualified_name = + cls->getQualifiedNameAsString(); // common::get_qualified_name(*cls); if (!diagram().should_include(qualified_name)) return {}; - namespace_ ns{qualified_name}; - ns.pop_back(); + auto ns = common::get_tag_namespace(*cls); - c.set_name(common::get_tag_name(*cls)); - c.set_namespace(ns); - c.set_id(common::to_id(*cls)); + const auto *parent = cls->getParent(); + + if (parent && parent->isRecord()) { + // Here we have 2 options, either: + // - the parent is a regular C++ class/struct + // - the parent is a class template declaration/specialization + std::optional id_opt; + int64_t local_id = + static_cast(parent)->getID(); + + // First check if the parent has been added to the diagram as regular + // class + id_opt = get_ast_local_id(local_id); + + // If not, check if the parent template declaration is in the model + if (!id_opt) { + local_id = static_cast(parent) + ->getDescribedTemplate() + ->getID(); + if (static_cast(parent) + ->getDescribedTemplate()) + id_opt = get_ast_local_id(local_id); + } + + assert(id_opt); + + auto parent_class = diagram_.get_class(*id_opt); + + assert(parent_class); + + c.set_namespace(ns); + if (cls->getNameAsString().empty()) { + // Nested structs can be anonymous + if (anonymous_struct_relationships_.count(cls->getID()) > 0) { + const auto &[label, hint, access] = + anonymous_struct_relationships_[cls->getID()]; + + c.set_name(parent_class.value().full_name(true) + "##" + + fmt::format("({})", label)); + + parent_class.value().add_relationship( + {hint, common::to_id(c.full_name(false)), access, label}); + } + else + c.set_name(parent_class.value().full_name(true) + "##" + + fmt::format( + "(anonymous_{})", std::to_string(cls->getID()))); + } + else { + c.set_name(parent_class.value().full_name(true) + "##" + + cls->getNameAsString()); + } + + c.set_id(common::to_id(c.full_name(false))); + + if (!cls->getNameAsString().empty()) { + // Don't add anonymous structs as contained in the class + // as they are already added as aggregations + c.add_relationship({relationship_t::kContainment, *id_opt}); + } + + c.nested(true); + } + else { + c.set_name(common::get_tag_name(*cls)); + c.set_namespace(ns); + c.set_id(common::to_id(c.full_name(false))); + } c.is_struct(cls->isStruct()); @@ -415,11 +516,6 @@ void translation_unit_visitor::process_class_declaration( // Process class bases process_class_bases(&cls, c); - if (cls.getParent()->isRecord()) { - process_record_containment(cls, c); - c.nested(true); - } - c.complete(true); } @@ -482,6 +578,28 @@ bool translation_unit_visitor::process_template_parameters( return false; } +void translation_unit_visitor::process_template_record_containment( + const clang::TagDecl &record, + clanguml::common::model::element &element) const +{ + assert(record.getParent()->isRecord()); + + const auto *parent = record.getParent(); //->getOuterLexicalRecordContext(); + + if (parent && + static_cast(parent) + ->getDescribedTemplate()) { + auto id_opt = + get_ast_local_id(static_cast(parent) + ->getDescribedTemplate() + ->getID()); + + if (id_opt) { + element.add_relationship({relationship_t::kContainment, *id_opt}); + } + } +} + void translation_unit_visitor::process_record_containment( const clang::TagDecl &record, clanguml::common::model::element &element) const @@ -489,9 +607,8 @@ void translation_unit_visitor::process_record_containment( assert(record.getParent()->isRecord()); const auto *parent = record.getParent()->getOuterLexicalRecordContext(); - auto parent_name = - static_cast(record.getParent()) - ->getQualifiedNameAsString(); + auto parent_name = static_cast(parent) + ->getQualifiedNameAsString(); auto namespace_declaration = common::get_enclosing_namespace(parent); if (namespace_declaration.has_value()) { @@ -889,8 +1006,8 @@ void translation_unit_visitor::process_function_parameter( (relationship_type != relationship_t::kNone)) { relationship r{relationship_t::kDependency, type_element_id}; - LOG_DBG( - "Adding function parameter relationship from {} to {}: {}", + LOG_DBG("Adding function parameter relationship from {} to " + "{}: {}", c.full_name(), clanguml::common::model::to_string(r.type()), r.label()); @@ -898,8 +1015,9 @@ void translation_unit_visitor::process_function_parameter( } } - // Also consider the container itself if it is a template instantiation - // it's arguments could count as reference to relevant types + // Also consider the container itself if it is a template + // instantiation it's arguments could count as reference to relevant + // types auto underlying_type = p.getType(); if (underlying_type->isReferenceType()) underlying_type = underlying_type.getNonReferenceType(); @@ -1103,9 +1221,9 @@ void translation_unit_visitor::process_template_specialization_argument( auto type_name = common::to_string(arg.getAsType(), cls->getASTContext()); - // clang does not provide declared template parameter/argument names - // in template specializations - so we have to extract them from - // raw source code... + // clang does not provide declared template parameter/argument + // names in template specializations - so we have to extract + // them from raw source code... if (type_name.find("type-parameter-") == 0) { auto declaration_text = common::get_source_text_raw( cls->getSourceRange(), source_manager_); @@ -1948,8 +2066,20 @@ void translation_unit_visitor::process_field( if (!field.skip_relationship()) { // Find relationship for the type if the type has not been added // as aggregation - if (!template_instantiation_added_as_aggregation) - find_relationships(field_type, relationships, relationship_hint); + if (!template_instantiation_added_as_aggregation) { + if (field_type->getAsCXXRecordDecl() && + field_type->getAsCXXRecordDecl()->getNameAsString().empty()) { + // Relationships to fields whose type is an anonymous nested + // struct have to be handled separately here + anonymous_struct_relationships_[field_type->getAsCXXRecordDecl() + ->getID()] = + std::make_tuple( + field.name(), relationship_hint, field.access()); + } + else + find_relationships( + field_type, relationships, relationship_hint); + } add_relationships(c, field, relationships); } @@ -1997,11 +2127,13 @@ bool translation_unit_visitor::simplify_system_template( void translation_unit_visitor::set_ast_local_id( int64_t local_id, common::model::diagram_element::id_t global_id) { + LOG_DBG("== Setting local element mapping {} --> {}", local_id, global_id); + local_ast_id_map_[local_id] = global_id; } std::optional -translation_unit_visitor::get_ast_local_id(int64_t local_id) +translation_unit_visitor::get_ast_local_id(int64_t local_id) const { if (local_ast_id_map_.find(local_id) == local_ast_id_map_.end()) return {}; diff --git a/src/class_diagram/visitor/translation_unit_visitor.h b/src/class_diagram/visitor/translation_unit_visitor.h index 809a82ca..ca1338a3 100644 --- a/src/class_diagram/visitor/translation_unit_visitor.h +++ b/src/class_diagram/visitor/translation_unit_visitor.h @@ -95,6 +95,9 @@ private: const clang::TemplateArgument &arg, size_t argument_index, bool in_parameter_pack = false); + void process_template_record_containment(const clang::TagDecl &record, + clanguml::common::model::element &c) const; + void process_record_containment(const clang::TagDecl &record, clanguml::common::model::element &c) const; @@ -222,7 +225,7 @@ private: /// Retrieve the global clang-uml entity id based on the clang local id std::optional get_ast_local_id( - int64_t local_id); + int64_t local_id) const; clang::SourceManager &source_manager_; @@ -237,5 +240,10 @@ private: forward_declarations_; std::map local_ast_id_map_; + + std::map> + anonymous_struct_relationships_; }; } diff --git a/src/common/clang_utils.cc b/src/common/clang_utils.cc index aaaeea0a..c5fec542 100644 --- a/src/common/clang_utils.cc +++ b/src/common/clang_utils.cc @@ -39,6 +39,34 @@ std::optional get_enclosing_namespace( common::get_qualified_name(*namespace_declaration)}; } +model::namespace_ get_tag_namespace(const clang::TagDecl &declaration) +{ + model::namespace_ ns; + + auto *parent{declaration.getParent()}; + + // First walk up to the nearest namespace, e.g. from nested class or enum + while (parent && !parent->isNamespace()) { + parent = parent->getParent(); + } + + // Now build up the namespace + std::deque namespace_tokens; + while (parent && parent->isNamespace()) { + const auto *ns_decl = static_cast(parent); + if (!ns_decl->isInline() && !ns_decl->isAnonymousNamespace()) + namespace_tokens.push_front(ns_decl->getNameAsString()); + + parent = parent->getParent(); + } + + for (const auto &ns_token : namespace_tokens) { + ns |= ns_token; + } + + return ns; +} + std::string get_tag_name(const clang::TagDecl &declaration) { auto base_name = declaration.getNameAsString(); diff --git a/src/common/clang_utils.h b/src/common/clang_utils.h index 1e1da03e..b3860a59 100644 --- a/src/common/clang_utils.h +++ b/src/common/clang_utils.h @@ -51,6 +51,8 @@ template std::string get_qualified_name(const T &declaration) return qualified_name; } +model::namespace_ get_tag_namespace(const clang::TagDecl &declaration); + std::optional get_enclosing_namespace( const clang::DeclContext *decl); diff --git a/tests/t00004/t00004.cc b/tests/t00004/t00004.cc index 6c702235..3ccd4592 100644 --- a/tests/t00004/t00004.cc +++ b/tests/t00004/t00004.cc @@ -14,7 +14,8 @@ public: public: enum class Lights { Green, Yellow, Red }; - class AAA { }; + class AAA { + }; }; void foo2() const { } @@ -25,7 +26,8 @@ public: T t; class AA { - class AAA { }; + class AAA { + }; enum class CCC { CCC_1, CCC_2 }; }; diff --git a/tests/t00037/t00037.cc b/tests/t00037/t00037.cc index 2d17af61..55b46f36 100644 --- a/tests/t00037/t00037.cc +++ b/tests/t00037/t00037.cc @@ -1,7 +1,8 @@ namespace clanguml { namespace t00037 { -struct ST { +class ST { +public: struct { double t; double x; @@ -9,22 +10,20 @@ struct ST { double z; } dimensions; +private: struct { - double c; - double h; + double c{1.0}; + double h{1.0}; } units; }; struct A { A() { - st.dimensions.t = -1; - st.dimensions.x = 1; - st.dimensions.y = 1; - st.dimensions.z = 1; - - st.units.c = 1; - st.units.h = 1; + st.dimensions.t = -1.0; + st.dimensions.x = 1.0; + st.dimensions.y = 1.0; + st.dimensions.z = 1.0; } ST st; diff --git a/tests/t00037/test_case.h b/tests/t00037/test_case.h index 06bf6435..b6e0baa9 100644 --- a/tests/t00037/test_case.h +++ b/tests/t00037/test_case.h @@ -37,7 +37,12 @@ TEST_CASE("t00037", "[test-case][class]") REQUIRE_THAT(puml, IsClass(_A("ST"))); REQUIRE_THAT(puml, IsClass(_A("A"))); - REQUIRE_THAT(puml, IsClass(_A("ST::\\(anonymous_\\d+\\)"))); + REQUIRE_THAT(puml, IsClass(_A("ST::\\(units\\)"))); + REQUIRE_THAT(puml, IsClass(_A("ST::\\(dimensions\\)"))); + REQUIRE_THAT(puml, + IsAggregation(_A("ST"), _A("ST::\\(dimensions\\)"), "+dimensions")); + REQUIRE_THAT( + puml, IsAggregation(_A("ST"), _A("ST::\\(units\\)"), "-units")); save_puml( "./" + config.output_directory() + "/" + diagram->name + ".puml", puml);