Merge pull request #106 from bkryza/fix-processing-clang-uml-examples

Fix processing clang uml examples

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));
 
@@ -187,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)
@@ -202,10 +201,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
+    if (cls->hasDefinition())
-    process_class_bases(cls, template_specialization);
+        // Process template specialization bases
+        process_class_bases(cls, template_specialization);
 
     const auto maybe_id =
         get_ast_local_id(cls->getSpecializedTemplate()->getID());
@@ -214,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));
     }
@@ -282,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
 
@@ -681,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<clang::RecordDecl>(cls->getParent());
+        parent_record != nullptr) {
         LOG_DBG("== getParent()->getQualifiedNameAsString() = {}",
-            clang::dyn_cast<clang::RecordDecl>(cls->getParent())
+            parent_record->getQualifiedNameAsString());
-                ->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:
@@ -839,11 +850,14 @@ void translation_unit_visitor::process_record_parent(
 
     std::optional<common::model::diagram_element::id_t> id_opt;
 
+    auto parent_ns = ns;
     if (parent != nullptr) {
         const auto *parent_record_decl =
             clang::dyn_cast<clang::RecordDecl>(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
@@ -870,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
@@ -1035,10 +1049,8 @@ void translation_unit_visitor::process_record_containment(
     auto parent_name = static_cast<const clang::RecordDecl *>(parent)
                            ->getQualifiedNameAsString();
 
-    auto namespace_declaration = common::get_enclosing_namespace(parent);
+    auto namespace_declaration = common::get_tag_namespace(record);
-    if (namespace_declaration.has_value()) {
+    element.set_namespace(namespace_declaration);
-        element.set_namespace(namespace_declaration.value());
-    }
 
     if (const auto *record_decl =
             clang::dyn_cast<clang::RecordDecl>(record.getParent());
@@ -1150,8 +1162,10 @@ void translation_unit_visitor::process_template_specialization_children(
         }
     }
 
-    for (const auto *friend_declaration : cls->friends()) {
+    if (cls->hasFriends()) {
-        process_friend(*friend_declaration, c);
+        for (const auto *friend_declaration : cls->friends()) {
+            process_friend(*friend_declaration, c);
+        }
     }
 }
 
@@ -1747,6 +1761,14 @@ translation_unit_visitor::process_template_specialization(
 
     template_instantiation.is_struct(cls->isStruct());
 
+    process_record_parent(cls, template_instantiation, ns);
+
+    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);
 
@@ -2827,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

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<common::model::diagram_element::id_t> get_ast_local_id(
         int64_t local_id) const;
@@ -307,5 +311,11 @@ private:
         std::tuple<std::string /* field name */, common::model::relationship_t,
             common::model::access_t>>
         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<std::string> processed_template_qualified_names_;
 };
 } // namespace clanguml::class_diagram::visitor

src/common/clang_utils.cc

@@ -43,23 +43,6 @@ model::access_t access_specifier_to_access_t(
     return access;
 }
 
-std::optional<clanguml::common::model::namespace_> get_enclosing_namespace(
-    const clang::DeclContext *decl)
-{
-    if (!decl->getEnclosingNamespaceContext()->isNamespace())
-        return {};
-
-    const auto *namespace_declaration =
-        clang::cast<clang::NamespaceDecl>(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;

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<clanguml::common::model::namespace_> get_enclosing_namespace(
-    const clang::DeclContext *decl);
-
 std::string to_string(const clang::QualType &type, const clang::ASTContext &ctx,
     bool try_canonical = true);
 

src/common/model/diagram_element.cc

@@ -46,8 +46,8 @@ void diagram_element::add_relationship(relationship &&cr)
         return;
     }
 
-    LOG_DBG("Adding relationship: '{}' - {} - '{}'", cr.destination(),
+    LOG_DBG("Adding relationship from: '{}' ({}) - {} - '{}'", id(),
-        to_string(cr.type()), full_name(true));
+        full_name(true), to_string(cr.type()), cr.destination());
 
     if (!util::contains(relationships_, cr))
         relationships_.emplace_back(std::move(cr));

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;
                 }
 

tests/t00008/t00008.cc

@@ -32,5 +32,20 @@ struct D {
 
     void add(int i) { ints.template_template.values.push_back(i); }
 };
+
+struct E {
+    template <typename ET> struct nested_template {
+        using DT = ET;
+
+        static DT *get(ET *d) { return d; }
+    };
+};
+
+template <> struct E::nested_template<char> {
+    using DeclType = char;
+
+    static DeclType *getDecl(char *c) { return c; }
+};
+
 } // namespace t00008
 } // namespace clanguml

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<Public>("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<ET>"), _A("E::nested_template<char>")));
+
     save_puml(config.output_directory() + "/" + diagram->name + ".puml", puml);
 }

tests/t00041/.clang-uml

@@ -17,4 +17,6 @@ diagrams:
         - clanguml::t00041::ns1::N
     exclude:
       namespaces:
-        - clanguml::t00041::detail
+        - clanguml::t00041::detail
+      relationships:
+        - dependency

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 { };

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")));