Skip to content

Commit

Permalink
Fixed handling of multiple lambda expressions in a single function call
Browse files Browse the repository at this point in the history
  • Loading branch information
bkryza committed Nov 9, 2024
1 parent 51d20bb commit b08a002
Show file tree
Hide file tree
Showing 9 changed files with 208 additions and 36 deletions.
80 changes: 57 additions & 23 deletions src/sequence_diagram/visitor/translation_unit_visitor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,22 @@ bool translation_unit_visitor::VisitFunctionDecl(
return true;
}

bool translation_unit_visitor::TraverseFunctionTemplateDecl(
clang::FunctionTemplateDecl *declaration)
{
// We need to backup the context, since other methods or functions can
// be traversed during this traversal (e.g. template function/method
// specializations)
auto context_backup = context();

RecursiveASTVisitor<translation_unit_visitor>::TraverseFunctionTemplateDecl(
declaration);

call_expression_context_ = context_backup;

return true;
}

bool translation_unit_visitor::VisitFunctionTemplateDecl(
clang::FunctionTemplateDecl *declaration)
{
Expand Down Expand Up @@ -637,6 +653,8 @@ bool translation_unit_visitor::VisitLambdaExpr(clang::LambdaExpr *expr)
m.set_from(context().caller_id());
m.set_to(lambda_method_model_ptr->id());

ensure_activity_exists(m);

diagram().add_active_participant(m.from());
diagram().add_active_participant(m.to());

Expand Down Expand Up @@ -913,6 +931,8 @@ bool translation_unit_visitor::TraverseIfStmt(clang::IfStmt *stmt)
}
else {
context().enter_ifstmt(stmt);
LOG_TRACE("Entered if statement at {}",
stmt->getBeginLoc().printToString(source_manager()));

message m{message_t::kIf, current_caller_id};
set_source_location(*stmt, m);
Expand Down Expand Up @@ -1294,11 +1314,7 @@ bool translation_unit_visitor::VisitObjCMessageExpr(
m.set_comment(raw_expr_comment->getBeginLoc().getHashValue(),
stripped_comment);

if (diagram().sequences().find(m.from()) ==
diagram().sequences().end()) {
activity a{m.from()};
diagram().sequences().insert({m.from(), std::move(a)});
}
ensure_activity_exists(m);

diagram().add_active_participant(m.from());
diagram().add_active_participant(m.to());
Expand Down Expand Up @@ -1473,11 +1489,7 @@ bool translation_unit_visitor::VisitCallExpr(clang::CallExpr *expr)
m.set_comment(raw_expr_comment->getBeginLoc().getHashValue(),
stripped_comment);

if (diagram().sequences().find(m.from()) ==
diagram().sequences().end()) {
activity a{m.from()};
diagram().sequences().insert({m.from(), std::move(a)});
}
ensure_activity_exists(m);

diagram().add_active_participant(m.from());
diagram().add_active_participant(m.to());
Expand All @@ -1490,6 +1502,13 @@ bool translation_unit_visitor::VisitCallExpr(clang::CallExpr *expr)

return true;
}
void translation_unit_visitor::ensure_activity_exists(const model::message &m)
{
if (diagram().sequences().find(m.from()) == diagram().sequences().end()) {
model::activity a{m.from()};
diagram().sequences().insert({m.from(), std::move(a)});
}
}

bool translation_unit_visitor::generate_message_from_comment(
model::message &m) const
Expand Down Expand Up @@ -1555,11 +1574,7 @@ bool translation_unit_visitor::VisitCXXConstructExpr(
return true;

if (m.from().value() > 0 && m.to().value() > 0) {
if (diagram().sequences().find(m.from()) ==
diagram().sequences().end()) {
activity a{m.from()};
diagram().sequences().insert({m.from(), std::move(a)});
}
ensure_activity_exists(m);

diagram().add_active_participant(m.from());
diagram().add_active_participant(m.to());
Expand Down Expand Up @@ -2308,7 +2323,7 @@ std::string translation_unit_visitor::make_lambda_name(
void translation_unit_visitor::push_message(
clang::CallExpr *expr, model::message &&m)
{
call_expr_message_map_.emplace(expr, std::move(m));
call_expr_message_map_[expr].push_back(std::move(m));
}

void translation_unit_visitor::push_message(
Expand All @@ -2332,15 +2347,24 @@ void translation_unit_visitor::pop_message_to_diagram(clang::CallExpr *expr)
return;
}

auto msg = std::move(call_expr_message_map_.at(expr));
if (call_expr_message_map_.at(expr).empty())
return;

auto caller_id = msg.from();
while (!call_expr_message_map_.at(expr).empty()) {
auto msg = call_expr_message_map_.at(expr).front();

if (caller_id == 0)
return;
auto caller_id = msg.from();

if (caller_id == 0)
return;

if (diagram().has_activity(caller_id))
diagram().get_activity(caller_id).add_message(std::move(msg));
if (diagram().has_activity(caller_id))
diagram().get_activity(caller_id).add_message(std::move(msg));
else
LOG_DBG("Skipping message due to missing activity: {}", caller_id);

call_expr_message_map_.at(expr).pop_front();
}

call_expr_message_map_.erase(expr);
}
Expand Down Expand Up @@ -2627,8 +2651,18 @@ bool translation_unit_visitor::should_include(
bool translation_unit_visitor::should_include(
const clang::LambdaExpr *expr) const
{
if (context().caller_id() == 0)
return false;

if (!context().valid())
return false;

const auto expr_file = expr->getBeginLoc().printToString(source_manager());
return diagram().should_include(common::model::source_file{expr_file});

if (!diagram().should_include(common::model::source_file{expr_file}))
return false;

return true;
}

bool translation_unit_visitor::should_include(
Expand Down
8 changes: 6 additions & 2 deletions src/sequence_diagram/visitor/translation_unit_visitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
#include <clang/AST/RecursiveASTVisitor.h>
#include <clang/Basic/SourceManager.h>

#include <stack>
#include <deque>

namespace clanguml::sequence_diagram::visitor {

Expand Down Expand Up @@ -122,6 +122,8 @@ class translation_unit_visitor

bool VisitFunctionDecl(clang::FunctionDecl *declaration);

bool TraverseFunctionTemplateDecl(clang::FunctionTemplateDecl *declaration);

bool VisitFunctionTemplateDecl(
clang::FunctionTemplateDecl *function_declaration);

Expand Down Expand Up @@ -545,7 +547,8 @@ class translation_unit_visitor
* expressions (e.g. a(b(c(), d())), as they need to be added to the diagram
* sequence after the visitor leaves the call expression AST node
*/
std::map<clang::CallExpr *, model::message> call_expr_message_map_;
std::map<clang::CallExpr *, std::deque<model::message>>
call_expr_message_map_;
std::map<clang::CXXConstructExpr *, model::message>
construct_expr_message_map_;
std::map<clang::ObjCMessageExpr *, model::message> objc_message_map_;
Expand All @@ -566,5 +569,6 @@ class translation_unit_visitor
processed_comments_by_caller_id_;

template_builder_t template_builder_;
void ensure_activity_exists(const model::message &m);
};
} // namespace clanguml::sequence_diagram::visitor
18 changes: 7 additions & 11 deletions src/util/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,15 @@

namespace clanguml::util {

// For release builds, use only file names in the log paths, for debug use
// full paths to make it easier to navigate to specific file:line in the code
// from logs
#if defined(NDEBUG)
#define FILENAME_ \
(strrchr(__FILE__, '/') ? strrchr(__FILE__, '/') + 1 : __FILE__)
#else
#define FILENAME_ __FILE__
#endif

constexpr unsigned kDefaultMessageCommentWidth{25U};

Expand Down Expand Up @@ -360,17 +367,6 @@ template <typename T, typename F> void for_each(const T &collection, F &&func)
std::forward<decltype(func)>(func));
}

template <typename T, typename C, typename F>
void for_each_if(const T &collection, C &&cond, F &&func)
{
std::for_each(std::begin(collection), std::end(collection),
[cond = std::forward<decltype(cond)>(cond),
func = std::forward<decltype(func)>(func)](const auto &e) {
if (cond(e))
func(e);
});
}

template <typename R, typename T, typename F>
std::vector<R> map(const std::vector<T> &in, F &&f)
{
Expand Down
28 changes: 28 additions & 0 deletions tests/t20060/.clang-uml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
diagrams:
t20060_sequence:
type: sequence
glob:
- t20060.cc
include:
namespaces:
- clanguml::t20060
exclude:
paths:
- include/t20060.h
using_namespace: clanguml::t20060
from:
- function: "clanguml::t20060::tmain()"
t20060_sequence_inlined:
type: sequence
inline_lambda_messages: true
glob:
- t20060.cc
include:
namespaces:
- clanguml::t20060
exclude:
paths:
- include/t20060.h
using_namespace: clanguml::t20060
from:
- function: "clanguml::t20060::tmain()"
22 changes: 22 additions & 0 deletions tests/t20060/include/t20060.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#include <algorithm>
#include <iterator>

namespace clanguml {
namespace t20060 {

namespace util {

template <typename T, typename C, typename F>
void for_each_if(const T &collection, C &&cond, F &&func)
{
std::for_each(std::begin(collection), std::end(collection),
[cond = std::forward<decltype(cond)>(cond),
func = std::forward<decltype(func)>(func)](auto &e) mutable {
if (cond(e))
func(e);
});
}
}

}
}
27 changes: 27 additions & 0 deletions tests/t20060/t20060.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#include "include/t20060.h"

#include <string>
#include <vector>

namespace clanguml {
namespace t20060 {

bool is_empty(const std::string &t) { return t.empty(); }

void log() { }

void tmain()
{
std::vector<std::string> vs;

std::string out;

util::for_each_if(
vs, [](const auto &s) { return !is_empty(s); },
[&](const auto &s) {
log();
out += s;
});
}
}
}
56 changes: 56 additions & 0 deletions tests/t20060/test_case.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/**
* tests/t20060/test_case.h
*
* Copyright (c) 2021-2024 Bartek Kryza <[email protected]>
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

TEST_CASE("t20060")
{
using namespace clanguml::test;
using namespace std::string_literals;

{
auto [config, db, diagram, model] =
CHECK_SEQUENCE_MODEL("t20060", "t20060_sequence");

CHECK_SEQUENCE_DIAGRAM(*config, diagram, *model, [](const auto &src) {
REQUIRE(MessageOrder(src,
{//
{"tmain()", "tmain()::(lambda t20060.cc:20:13)",
"operator()(const auto &) const"}, //
{"tmain()::(lambda t20060.cc:20:13)",
"is_empty(const std::string &)", ""}, //
{"tmain()", "tmain()::(lambda t20060.cc:21:9)",
"operator()(const auto &) const"},
{"tmain()::(lambda t20060.cc:21:9)", "log()", ""}

}));
});
}

{
auto [config, db, diagram, model] =
CHECK_SEQUENCE_MODEL("t20060", "t20060_sequence_inlined");

CHECK_SEQUENCE_DIAGRAM(*config, diagram, *model, [](const auto &src) {
REQUIRE(MessageOrder(src,
{ //
{"tmain()", "is_empty(const std::string &)", ""}, //
{"tmain()", "log()", ""}

}));
});
}
}
2 changes: 2 additions & 0 deletions tests/test_cases.cc
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,8 @@ void CHECK_INCLUDE_DIAGRAM(const clanguml::config::config &config,
#include "t20059/test_case.h"
#endif

#include "t20060/test_case.h"

///
/// Package diagram tests
///
Expand Down
3 changes: 3 additions & 0 deletions tests/test_cases.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,9 @@ test_cases:
- name: t20059
title: Objective-C sequence diagram with polymorphism test case
description:
- name: t20060
title: Test case for multiple lambda expressions in function calls arguments
description:
Package diagrams:
- name: t30001
title: Basic package diagram test case
Expand Down

0 comments on commit b08a002

Please sign in to comment.