Fixed id resolution of callexpr callees by function declaration

src/sequence_diagram/model/diagram.cc

@@ -80,24 +80,35 @@ void diagram::print() const
 
     LOG_DBG(" --- Activities ---");
     for (const auto &[from_id, act] : sequences) {
+
         LOG_DBG("Sequence id={}:", from_id);
+
         const auto &from_activity = *(participants.at(from_id));
+
         LOG_DBG("   Activity id={}, from={}:", act.from,
             from_activity.full_name(false));
+
         for (const auto &message : act.messages) {
             if (participants.find(message.from) == participants.end())
                 continue;
-            if (participants.find(message.to) == participants.end())
-                continue;
 
             const auto &from_participant = *participants.at(message.from);
-            const auto &to_participant = *participants.at(message.to);
 
-            LOG_DBG("       Message from={}, from_id={}, "
+            if (participants.find(message.to) == participants.end()) {
-                    "to={}, to_id={}, name={}",
+                LOG_DBG("       Message from={}, from_id={}, "
-                from_participant.full_name(false), from_participant.id(),
+                        "to={}, to_id={}, name={}",
-                to_participant.full_name(false), to_participant.id(),
+                    from_participant.full_name(false), from_participant.id(),
-                message.message_name);
+                    "__UNRESOLVABLE_ID__", message.to, message.message_name);
+            }
+            else {
+                const auto &to_participant = *participants.at(message.to);
+
+                LOG_DBG("       Message from={}, from_id={}, "
+                        "to={}, to_id={}, name={}",
+                    from_participant.full_name(false), from_participant.id(),
+                    to_participant.full_name(false), to_participant.id(),
+                    message.message_name);
+            }
         }
     }
 }
@@ -192,6 +203,57 @@ void diagram::end_loop_stmt(
     }
 }
 
+void diagram::add_if_stmt(
+    const common::model::diagram_element::id_t current_caller_id,
+    common::model::message_t type)
+{
+    using clanguml::common::model::message_t;
+
+    if (sequences.find(current_caller_id) == sequences.end()) {
+        activity a;
+        a.from = current_caller_id;
+        sequences.insert({current_caller_id, std::move(a)});
+    }
+    message m;
+    m.from = current_caller_id;
+    m.type = type;
+
+    sequences[current_caller_id].messages.emplace_back(std::move(m));
+}
+
+void diagram::end_if_stmt(
+    const common::model::diagram_element::id_t current_caller_id,
+    common::model::message_t type)
+{
+    using clanguml::common::model::message_t;
+
+    message m;
+    m.from = current_caller_id;
+    m.type = message_t::kIfEnd;
+
+    if (sequences.find(current_caller_id) != sequences.end()) {
+
+        auto &current_messages = sequences[current_caller_id].messages;
+        // Remove the if/else messages if there were no calls
+        // added to the diagram between them
+        auto last_if_it =
+            std::find_if(current_messages.rbegin(), current_messages.rend(),
+                [](const message &m) { return m.type == message_t::kIf; });
+
+        bool last_if_block_is_empty =
+            std::none_of(current_messages.rbegin(), last_if_it,
+                [](const message &m) { return m.type == message_t::kCall; });
+
+        if (!last_if_block_is_empty) {
+            current_messages.emplace_back(std::move(m));
+        }
+        else {
+            current_messages.erase(
+                (last_if_it + 1).base(), current_messages.end());
+        }
+    }
+}
+
 }
 
 namespace clanguml::common::model {

src/sequence_diagram/model/diagram.h

@@ -103,16 +103,26 @@ public:
 
     std::set<common::model::diagram_element::id_t> active_participants_;
 
+    void add_if_stmt(
+        const common::model::diagram_element::id_t current_caller_id,
+        common::model::message_t type);
+    void end_if_stmt(
+        const common::model::diagram_element::id_t current_caller_id,
+        common::model::message_t type);
+
     void add_loop_stmt(
         const common::model::diagram_element::id_t current_caller_id,
         common::model::message_t type);
     void end_loop_stmt(
         const common::model::diagram_element::id_t current_caller_id,
         common::model::message_t type);
+
     void add_while_stmt(const common::model::diagram_element::id_t i);
     void end_while_stmt(const common::model::diagram_element::id_t i);
+
     void add_do_stmt(const common::model::diagram_element::id_t i);
     void end_do_stmt(const common::model::diagram_element::id_t i);
+
     void add_for_stmt(const common::model::diagram_element::id_t i);
     void end_for_stmt(const common::model::diagram_element::id_t i);
 };

src/sequence_diagram/visitor/translation_unit_visitor.cc

@@ -102,7 +102,8 @@ bool translation_unit_visitor::VisitCXXRecordDecl(clang::CXXRecordDecl *cls)
     if (cls->isTemplated() && cls->getDescribedTemplate()) {
         // If the described templated of this class is already in the model
         // skip it:
-        if (get_unique_id(cls->getDescribedTemplate()->getID()))
+        auto local_id = cls->getDescribedTemplate()->getID();
+        if (get_unique_id(local_id))
             return true;
     }
 
@@ -110,12 +111,12 @@ bool translation_unit_visitor::VisitCXXRecordDecl(clang::CXXRecordDecl *cls)
     if (cls->isLocalClass())
         return true;
 
-    LOG_DBG("Visiting class declaration at {}",
+    LOG_TRACE("Visiting class declaration at {}",
         cls->getBeginLoc().printToString(source_manager()));
 
     // Build the class declaration and store it in the diagram, even
     // if we don't need it for any of the participants of this diagram
-    auto c_ptr = create_class_declaration(cls);
+    auto c_ptr = this->create_class_declaration(cls);
 
     if (!c_ptr)
         return true;
@@ -327,9 +328,16 @@ bool translation_unit_visitor::VisitCXXMethodDecl(clang::CXXMethodDecl *m)
 
     m_ptr->set_id(common::to_id(method_full_name));
 
-    set_unique_id(m->getID(), m_ptr->id());
+    // Callee methods in call expressions are referred to by first declaration
+    // id
+    if (m->isThisDeclarationADefinition()) {
+        set_unique_id(m->getFirstDecl()->getID(), m_ptr->id());
+    }
 
-    LOG_DBG("Set id {} for method name {}", m_ptr->id(), method_full_name);
+    set_unique_id(m->getID(), m_ptr->id()); // This is probably not necessary?
+
+    LOG_DBG("Set id {} --> {} for method name {} [{}]", m->getID(), m_ptr->id(),
+        method_full_name, m->isThisDeclarationADefinition());
 
     context().update(m);
 
@@ -379,6 +387,9 @@ bool translation_unit_visitor::VisitFunctionDecl(clang::FunctionDecl *f)
 
         context().set_caller_id(f_ptr->id());
 
+        if (f->isThisDeclarationADefinition()) {
+            set_unique_id(f->getFirstDecl()->getID(), f_ptr->id());
+        }
         set_unique_id(f->getID(), f_ptr->id());
 
         set_source_location(*f, *f_ptr);
@@ -408,6 +419,9 @@ bool translation_unit_visitor::VisitFunctionDecl(clang::FunctionDecl *f)
 
         context().set_caller_id(f_ptr->id());
 
+        if (f->isThisDeclarationADefinition()) {
+            set_unique_id(f->getFirstDecl()->getID(), f_ptr->id());
+        }
         set_unique_id(f->getID(), f_ptr->id());
 
         set_source_location(*f, *f_ptr);
@@ -469,11 +483,11 @@ bool translation_unit_visitor::VisitLambdaExpr(clang::LambdaExpr *expr)
     const auto lambda_full_name =
         expr->getLambdaClass()->getCanonicalDecl()->getNameAsString();
 
-    LOG_DBG("Visiting lambda expression {} at {}", lambda_full_name,
+    LOG_TRACE("Visiting lambda expression {} at {}", lambda_full_name,
         expr->getBeginLoc().printToString(source_manager()));
 
-    LOG_DBG("Lambda call operator ID {} - lambda class ID {}, class call "
+    LOG_TRACE("Lambda call operator ID {} - lambda class ID {}, class call "
-            "operator ID {}",
+              "operator ID {}",
         expr->getCallOperator()->getID(), expr->getLambdaClass()->getID(),
         expr->getLambdaClass()->getLambdaCallOperator()->getID());
 
@@ -593,75 +607,37 @@ bool translation_unit_visitor::TraverseIfStmt(clang::IfStmt *stmt)
     using clanguml::sequence_diagram::model::activity;
     using clanguml::sequence_diagram::model::message;
 
-    const auto current_caller_id = context().caller_id();
     bool elseif_block{false};
 
-    if (current_caller_id && !stmt->isConstexpr()) {
+    const auto current_caller_id = context().caller_id();
-        const auto *current_ifstmt = context().current_ifstmt();
+    const auto *current_ifstmt = context().current_ifstmt();
-        context().enter_ifstmt(stmt);
 
-        if (diagram().sequences.find(current_caller_id) ==
+    // Check if this is a beginning of a new if statement, or an
-            diagram().sequences.end()) {
+    // else if condition of the current if statement
-            activity a;
+    if (current_ifstmt != nullptr) {
-            a.from = current_caller_id;
+        for (const auto *child_stmt : current_ifstmt->children()) {
-            diagram().sequences.insert({current_caller_id, std::move(a)});
+            if (child_stmt == stmt) {
-        }
+                elseif_block = true;
-        message m;
+                break;
-        m.from = current_caller_id;
-
-        // Check if this is a beginning of a new if statement, or an
-        // else if condition of the current if statement
-        if (current_ifstmt != nullptr) {
-            for (const auto *child_stmt : current_ifstmt->children()) {
-                if (child_stmt == stmt) {
-                    elseif_block = true;
-                    break;
-                }
             }
         }
+    }
 
+    if (current_caller_id && !stmt->isConstexpr()) {
+        context().enter_ifstmt(stmt);
         if (elseif_block) {
-            m.type = message_t::kElseIf;
             context().enter_elseifstmt(stmt);
+            diagram().add_if_stmt(current_caller_id, message_t::kElseIf);
         }
         else {
-            m.type = message_t::kIf;
+            diagram().add_if_stmt(current_caller_id, message_t::kIf);
         }
-
-        diagram().sequences[current_caller_id].messages.emplace_back(
-            std::move(m));
     }
 
     RecursiveASTVisitor<translation_unit_visitor>::TraverseIfStmt(stmt);
 
     if (current_caller_id && !stmt->isConstexpr() && !elseif_block) {
-        message m;
+        diagram().end_if_stmt(current_caller_id, message_t::kIfEnd);
-        m.from = current_caller_id;
-        m.type = message_t::kIfEnd;
-
-        if (diagram().sequences.find(current_caller_id) !=
-            diagram().sequences.end()) {
-
-            auto &current_messages =
-                diagram().sequences[current_caller_id].messages;
-            // Remove the if/else messages if there were no calls
-            // added to the diagram between them
-            auto last_if_it =
-                std::find_if(current_messages.rbegin(), current_messages.rend(),
-                    [](const message &m) { return m.type == message_t::kIf; });
-
-            bool last_if_block_is_empty = std::none_of(
-                current_messages.rbegin(), last_if_it,
-                [](const message &m) { return m.type == message_t::kCall; });
-
-            if (!last_if_block_is_empty) {
-                current_messages.emplace_back(std::move(m));
-            }
-            else {
-                current_messages.erase(
-                    (last_if_it + 1).base(), current_messages.end());
-            }
-        }
     }
 
     return true;
@@ -673,17 +649,18 @@ bool translation_unit_visitor::TraverseWhileStmt(clang::WhileStmt *stmt)
     using clanguml::sequence_diagram::model::activity;
     using clanguml::sequence_diagram::model::message;
 
-    context().enter_loopstmt(stmt);
-
     const auto current_caller_id = context().caller_id();
 
-    diagram().add_while_stmt(current_caller_id);
+    if (current_caller_id) {
-
+        context().enter_loopstmt(stmt);
+        diagram().add_while_stmt(current_caller_id);
+    }
     RecursiveASTVisitor<translation_unit_visitor>::TraverseWhileStmt(stmt);
 
-    diagram().end_while_stmt(current_caller_id);
+    if (current_caller_id) {
-
+        diagram().end_while_stmt(current_caller_id);
-    context().leave_loopstmt();
+        context().leave_loopstmt();
+    }
 
     return true;
 }
@@ -694,17 +671,19 @@ bool translation_unit_visitor::TraverseDoStmt(clang::DoStmt *stmt)
     using clanguml::sequence_diagram::model::activity;
     using clanguml::sequence_diagram::model::message;
 
-    context().enter_loopstmt(stmt);
-
     const auto current_caller_id = context().caller_id();
 
-    diagram().add_do_stmt(current_caller_id);
+    if (current_caller_id) {
+        context().enter_loopstmt(stmt);
+        diagram().add_do_stmt(current_caller_id);
+    }
 
     RecursiveASTVisitor<translation_unit_visitor>::TraverseDoStmt(stmt);
 
-    context().leave_loopstmt();
+    if (current_caller_id) {
-
+        context().leave_loopstmt();
-    diagram().end_do_stmt(current_caller_id);
+        diagram().end_do_stmt(current_caller_id);
+    }
 
     return true;
 }
@@ -715,17 +694,19 @@ bool translation_unit_visitor::TraverseForStmt(clang::ForStmt *stmt)
     using clanguml::sequence_diagram::model::activity;
     using clanguml::sequence_diagram::model::message;
 
-    context().enter_loopstmt(stmt);
-
     const auto current_caller_id = context().caller_id();
 
-    diagram().add_for_stmt(current_caller_id);
+    if (current_caller_id) {
+        context().enter_loopstmt(stmt);
+        diagram().add_for_stmt(current_caller_id);
+    }
 
     RecursiveASTVisitor<translation_unit_visitor>::TraverseForStmt(stmt);
 
-    context().leave_loopstmt();
+    if (current_caller_id) {
-
+        context().leave_loopstmt();
-    diagram().end_for_stmt(current_caller_id);
+        diagram().end_for_stmt(current_caller_id);
+    }
 
     return true;
 }
@@ -737,18 +718,20 @@ bool translation_unit_visitor::TraverseCXXForRangeStmt(
     using clanguml::sequence_diagram::model::activity;
     using clanguml::sequence_diagram::model::message;
 
-    context().enter_loopstmt(stmt);
-
     const auto current_caller_id = context().caller_id();
 
-    diagram().add_for_stmt(current_caller_id);
+    if (current_caller_id) {
+        context().enter_loopstmt(stmt);
+        diagram().add_for_stmt(current_caller_id);
+    }
 
     RecursiveASTVisitor<translation_unit_visitor>::TraverseCXXForRangeStmt(
         stmt);
 
-    context().leave_loopstmt();
+    if (current_caller_id) {
-
+        context().leave_loopstmt();
-    diagram().end_for_stmt(current_caller_id);
+        diagram().end_for_stmt(current_caller_id);
+    }
 
     return true;
 }
@@ -930,6 +913,7 @@ bool translation_unit_visitor::process_class_method_call_expression(
         return false;
 
     m.to = method_decl->getID();
+
     m.message_name = method_decl->getNameAsString();
 
     m.return_type =
@@ -939,9 +923,7 @@ bool translation_unit_visitor::process_class_method_call_expression(
     LOG_DBG("Set callee method id {} for method name {}", m.to,
         method_decl->getQualifiedNameAsString());
 
-    if (get_unique_id(callee_decl->getID()))
+    diagram().add_active_participant(method_decl->getID());
-        diagram().add_active_participant(
-            get_unique_id(callee_decl->getID()).value());
 
     return true;
 }

src/sequence_diagram/visitor/translation_unit_visitor.h

@@ -245,7 +245,9 @@ private:
         std::unique_ptr<clanguml::sequence_diagram::model::class_>>
         forward_declarations_;
 
-    std::map<int64_t, common::model::diagram_element::id_t> local_ast_id_map_;
+    std::map</* local id from ->getID() */ int64_t,
+        /* global ID based on full name */ common::model::diagram_element::id_t>
+        local_ast_id_map_;
 
     std::map<int64_t /* local anonymous struct id */,
         std::tuple<std::string /* field name */, common::model::relationship_t,

src/util/util.h

@@ -53,6 +53,10 @@ std::string trim(const std::string &s);
     spdlog::get("console")->debug(std::string("[{}:{}] ") + fmt__,             \
         __FILENAME__, __LINE__, ##__VA_ARGS__)
 
+#define LOG_TRACE(fmt__, ...)                                                  \
+    spdlog::get("console")->trace(std::string("[{}:{}] ") + fmt__,             \
+        __FILENAME__, __LINE__, ##__VA_ARGS__)
+
 /**
  * @brief Setup spdlog logger.
  *

tests/t20020/t20020.cc

@@ -16,10 +16,29 @@ struct B {
     int b2() { return 4; }
 };
 
+struct C {
+    void log() const { }
+
+    void c1() const
+    {
+        if (c2())
+            log();
+    }
+
+    bool c2() const { return true; }
+};
+
+template <typename T> struct D {
+
+    T d1(T x, T y) { return x + y; }
+};
+
 int tmain()
 {
     A a;
     B b;
+    C c;
+    D<int> d;
 
     int result{0};
 
@@ -38,6 +57,12 @@ int tmain()
 
     b.log();
 
+    if (true)
+        c.c1();
+
+    if (true)
+        d.d1(1, 1);
+
     // This if/else should not be included in the diagram at all
     // as the calls to std will be excluded by the diagram filters
     if (result != 2) {

tests/t20020/test_case.h

@@ -43,6 +43,8 @@ TEST_CASE("t20020", "[test-case][sequence]")
     REQUIRE_THAT(puml, HasCall(_A("tmain()"), _A("B"), "b2()"));
     REQUIRE_THAT(puml, HasCall(_A("tmain()"), _A("B"), "log()"));
 
+    REQUIRE_THAT(puml, HasCall(_A("tmain()"), _A("C"), "c1()"));
+
     save_puml(
         "./" + config.output_directory() + "/" + diagram->name + ".puml", puml);
 }