From 4ab0d29252ee4b97139af90bd76b7a080df47962 Mon Sep 17 00:00:00 2001 From: Bartek Kryza Date: Sun, 8 Oct 2023 16:55:25 +0200 Subject: [PATCH] Make sure sequence diagram messages generated during static variable initialization are rendered only once --- .../json/sequence_diagram_generator.cc | 7 ++ .../json/sequence_diagram_generator.h | 2 + .../mermaid/sequence_diagram_generator.cc | 7 ++ .../mermaid/sequence_diagram_generator.h | 10 ++- .../plantuml/sequence_diagram_generator.cc | 7 ++ .../plantuml/sequence_diagram_generator.h | 10 ++- src/sequence_diagram/model/message.cc | 10 +++ src/sequence_diagram/model/message.h | 6 ++ .../visitor/translation_unit_visitor.cc | 28 +++++++ .../visitor/translation_unit_visitor.h | 6 ++ tests/t20037/.clang-uml | 14 ++++ tests/t20037/t20037.cc | 38 ++++++++++ tests/t20037/test_case.h | 75 +++++++++++++++++++ tests/test_cases.cc | 1 + tests/test_cases.yaml | 3 + 15 files changed, 222 insertions(+), 2 deletions(-) create mode 100644 tests/t20037/.clang-uml create mode 100644 tests/t20037/t20037.cc create mode 100644 tests/t20037/test_case.h diff --git a/src/sequence_diagram/generators/json/sequence_diagram_generator.cc b/src/sequence_diagram/generators/json/sequence_diagram_generator.cc index e0fdf0c1..45e13927 100644 --- a/src/sequence_diagram/generators/json/sequence_diagram_generator.cc +++ b/src/sequence_diagram/generators/json/sequence_diagram_generator.cc @@ -256,6 +256,13 @@ void generator::process_call_message(const model::message &m, { visited.push_back(m.from()); + if (m.in_static_declaration_context()) { + if (util::contains(already_generated_in_static_context_, m)) + return; + + already_generated_in_static_context_.push_back(m); + } + LOG_DBG("Generating message {} --> {}", m.from(), m.to()); generate_call(m, current_block_statement()); diff --git a/src/sequence_diagram/generators/json/sequence_diagram_generator.h b/src/sequence_diagram/generators/json/sequence_diagram_generator.h index 4e8ad13d..985f3dda 100644 --- a/src/sequence_diagram/generators/json/sequence_diagram_generator.h +++ b/src/sequence_diagram/generators/json/sequence_diagram_generator.h @@ -243,6 +243,8 @@ private: mutable std::vector> block_statements_stack_; + + mutable std::vector already_generated_in_static_context_; }; } // namespace clanguml::sequence_diagram::generators::json diff --git a/src/sequence_diagram/generators/mermaid/sequence_diagram_generator.cc b/src/sequence_diagram/generators/mermaid/sequence_diagram_generator.cc index 63f84a5f..6f364767 100644 --- a/src/sequence_diagram/generators/mermaid/sequence_diagram_generator.cc +++ b/src/sequence_diagram/generators/mermaid/sequence_diagram_generator.cc @@ -139,6 +139,13 @@ void generator::generate_activity(const activity &a, std::ostream &ostr, std::vector &visited) const { for (const auto &m : a.messages()) { + if (m.in_static_declaration_context()) { + if (util::contains(already_generated_in_static_context_, m)) + continue; + + already_generated_in_static_context_.push_back(m); + } + if (m.type() == message_t::kCall) { const auto &to = model().get_participant(m.to()); diff --git a/src/sequence_diagram/generators/mermaid/sequence_diagram_generator.h b/src/sequence_diagram/generators/mermaid/sequence_diagram_generator.h index c6bbd33d..3645daa1 100644 --- a/src/sequence_diagram/generators/mermaid/sequence_diagram_generator.h +++ b/src/sequence_diagram/generators/mermaid/sequence_diagram_generator.h @@ -137,9 +137,17 @@ private: */ std::string generate_alias(const model::participant &participant) const; - mutable std::set generated_participants_; + /** + * @brief Convert config to model message render mode. + * + * @return Method render mode. + */ model::function::message_render_mode select_method_arguments_render_mode() const; + + mutable std::set generated_participants_; + + mutable std::vector already_generated_in_static_context_; }; } // namespace mermaid diff --git a/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.cc b/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.cc index 8ba07087..e0fefc49 100644 --- a/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.cc +++ b/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.cc @@ -135,6 +135,13 @@ void generator::generate_activity(const activity &a, std::ostream &ostr, std::vector &visited) const { for (const auto &m : a.messages()) { + if (m.in_static_declaration_context()) { + if (util::contains(already_generated_in_static_context_, m)) + continue; + + already_generated_in_static_context_.push_back(m); + } + if (m.type() == message_t::kCall) { const auto &to = model().get_participant(m.to()); diff --git a/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.h b/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.h index 2519a081..a7612770 100644 --- a/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.h +++ b/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.h @@ -139,9 +139,17 @@ private: */ std::string render_name(std::string name) const; - mutable std::set generated_participants_; + /** + * @brief Convert config to model message render mode. + * + * @return Method render mode. + */ model::function::message_render_mode select_method_arguments_render_mode() const; + + mutable std::set generated_participants_; + + mutable std::vector already_generated_in_static_context_; }; } // namespace plantuml diff --git a/src/sequence_diagram/model/message.cc b/src/sequence_diagram/model/message.cc index b401e4f8..69c059bb 100644 --- a/src/sequence_diagram/model/message.cc +++ b/src/sequence_diagram/model/message.cc @@ -78,4 +78,14 @@ std::optional message::condition_text() const return condition_text_; } +bool message::in_static_declaration_context() const +{ + return in_static_declaration_context_; +} + +void message::in_static_declaration_context(bool v) +{ + in_static_declaration_context_ = v; +} + } // namespace clanguml::sequence_diagram::model diff --git a/src/sequence_diagram/model/message.h b/src/sequence_diagram/model/message.h index e0b795fc..0fb93720 100644 --- a/src/sequence_diagram/model/message.h +++ b/src/sequence_diagram/model/message.h @@ -150,6 +150,10 @@ public: */ std::optional condition_text() const; + bool in_static_declaration_context() const; + + void in_static_declaration_context(bool v); + private: common::model::message_t type_{common::model::message_t::kNone}; @@ -167,6 +171,8 @@ private: std::string return_type_{}; std::optional condition_text_; + + bool in_static_declaration_context_{false}; }; } // namespace clanguml::sequence_diagram::model diff --git a/src/sequence_diagram/visitor/translation_unit_visitor.cc b/src/sequence_diagram/visitor/translation_unit_visitor.cc index 59ea7c6f..0eaeb31e 100644 --- a/src/sequence_diagram/visitor/translation_unit_visitor.cc +++ b/src/sequence_diagram/visitor/translation_unit_visitor.cc @@ -936,6 +936,8 @@ bool translation_unit_visitor::VisitCallExpr(clang::CallExpr *expr) message m{message_t::kCall, context().caller_id()}; + m.in_static_declaration_context(within_static_variable_declaration_ > 0); + set_source_location(*expr, m); // If we're currently inside a lambda expression, set it's id as @@ -1031,6 +1033,27 @@ bool translation_unit_visitor::VisitCallExpr(clang::CallExpr *expr) return true; } +bool translation_unit_visitor::TraverseVarDecl(clang::VarDecl *decl) +{ + LOG_TRACE("Traversing cxx variable declaration at {} [caller_id = {}]", + decl->getBeginLoc().printToString(source_manager()), + context().caller_id()); + + decl->dump(); + + decl->getInit()->dump(); + + if (decl->isStaticLocal()) + within_static_variable_declaration_++; + + RecursiveASTVisitor::TraverseVarDecl(decl); + + if (decl->isStaticLocal()) + within_static_variable_declaration_--; + + return true; +} + bool translation_unit_visitor::VisitCXXConstructExpr( clang::CXXConstructExpr *expr) { @@ -1051,8 +1074,12 @@ bool translation_unit_visitor::VisitCXXConstructExpr( expr->getBeginLoc().printToString(source_manager()), context().caller_id()); + expr->dump(); + message m{message_t::kCall, context().caller_id()}; + m.in_static_declaration_context(within_static_variable_declaration_ > 0); + set_source_location(*expr, m); if (context().lambda_caller_id() != 0) { @@ -2427,6 +2454,7 @@ bool translation_unit_visitor::should_include(const clang::CallExpr *expr) const return false; const auto expr_file = expr->getBeginLoc().printToString(source_manager()); + return diagram().should_include(common::model::source_file{expr_file}); } diff --git a/src/sequence_diagram/visitor/translation_unit_visitor.h b/src/sequence_diagram/visitor/translation_unit_visitor.h index ed314b7b..56d24430 100644 --- a/src/sequence_diagram/visitor/translation_unit_visitor.h +++ b/src/sequence_diagram/visitor/translation_unit_visitor.h @@ -68,6 +68,8 @@ public: bool TraverseCallExpr(clang::CallExpr *expr); + bool TraverseVarDecl(clang::VarDecl *VD); + bool TraverseCXXMemberCallExpr(clang::CXXMemberCallExpr *expr); bool TraverseCXXOperatorCallExpr(clang::CXXOperatorCallExpr *expr); @@ -540,5 +542,9 @@ private: std::tuple> anonymous_struct_relationships_; + + mutable unsigned within_static_variable_declaration_{0}; + mutable std::set + already_visited_in_static_declaration_{}; }; } // namespace clanguml::sequence_diagram::visitor diff --git a/tests/t20037/.clang-uml b/tests/t20037/.clang-uml new file mode 100644 index 00000000..c910987e --- /dev/null +++ b/tests/t20037/.clang-uml @@ -0,0 +1,14 @@ +compilation_database_dir: .. +output_directory: diagrams +diagrams: + t20037_sequence: + type: sequence + glob: + - ../../tests/t20037/t20037.cc + include: + namespaces: + - clanguml::t20037 + using_namespace: + - clanguml::t20037 + from: + - function: "clanguml::t20037::tmain(int,char **)" \ No newline at end of file diff --git a/tests/t20037/t20037.cc b/tests/t20037/t20037.cc new file mode 100644 index 00000000..bf46610e --- /dev/null +++ b/tests/t20037/t20037.cc @@ -0,0 +1,38 @@ +namespace clanguml { +namespace t20037 { + +struct A { + A() + : a{100} + { + } + + int a; +}; + +struct B { + int get() { return b; } + + int b{100}; +}; + +B initb() { return B{}; } + +int c() { return 1; } + +int a() +{ + static A a; + static B b = initb(); + + return a.a + b.get() + c(); +} + +void tmain(int argc, char **argv) +{ + auto a1 = a(); + auto a2 = a(); + auto a3 = a(); +} +} +} \ No newline at end of file diff --git a/tests/t20037/test_case.h b/tests/t20037/test_case.h new file mode 100644 index 00000000..c4d60600 --- /dev/null +++ b/tests/t20037/test_case.h @@ -0,0 +1,75 @@ +/** + * tests/t20037/test_case.h + * + * Copyright (c) 2021-2023 Bartek Kryza + * + * 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("t20037", "[test-case][sequence]") +{ + auto [config, db] = load_config("t20037"); + + auto diagram = config.diagrams["t20037_sequence"]; + + REQUIRE(diagram->name == "t20037_sequence"); + + auto model = generate_sequence_diagram(*db, diagram); + + REQUIRE(model->name() == "t20037_sequence"); + + { + auto src = generate_sequence_puml(diagram, *model); + AliasMatcher _A(src); + + REQUIRE_THAT(src, StartsWith("@startuml")); + REQUIRE_THAT(src, EndsWith("@enduml\n")); + + REQUIRE_THAT(src, HasCall(_A("tmain(int,char **)"), _A("a()"), "")); + REQUIRE_THAT(src, HasCall(_A("a()"), _A("initb()"), "")); + REQUIRE_THAT(src, HasCall(_A("a()"), _A("B"), "get()")); + REQUIRE_THAT(src, HasCall(_A("a()"), _A("c()"), "")); + + save_puml(config.output_directory(), diagram->name + ".puml", src); + } + + { + auto j = generate_sequence_json(diagram, *model); + + using namespace json; + + std::vector messages = { + FindMessage(j, "tmain(int,char **)", "a()", ""), + FindMessage(j, "a()", "initb()", ""), + FindMessage(j, "a()", "B", "get()"), + FindMessage(j, "a()", "c()", "")}; + + REQUIRE(std::is_sorted(messages.begin(), messages.end())); + + save_json(config.output_directory(), diagram->name + ".json", j); + } + + { + auto src = generate_sequence_mermaid(diagram, *model); + + mermaid::SequenceDiagramAliasMatcher _A(src); + using mermaid::HasCall; + + REQUIRE_THAT(src, HasCall(_A("tmain(int,char **)"), _A("a()"), "")); + REQUIRE_THAT(src, HasCall(_A("a()"), _A("initb()"), "")); + REQUIRE_THAT(src, HasCall(_A("a()"), _A("B"), "get()")); + REQUIRE_THAT(src, HasCall(_A("a()"), _A("c()"), "")); + + save_mermaid(config.output_directory(), diagram->name + ".mmd", src); + } +} \ No newline at end of file diff --git a/tests/test_cases.cc b/tests/test_cases.cc index b2e6222a..17a7289e 100644 --- a/tests/test_cases.cc +++ b/tests/test_cases.cc @@ -423,6 +423,7 @@ using namespace clanguml::test::matchers; #include "t20034/test_case.h" #include "t20035/test_case.h" #include "t20036/test_case.h" +#include "t20037/test_case.h" /// /// Package diagram tests diff --git a/tests/test_cases.yaml b/tests/test_cases.yaml index bf07b136..d964cbe6 100644 --- a/tests/test_cases.yaml +++ b/tests/test_cases.yaml @@ -307,6 +307,9 @@ test_cases: - name: t20036 title: Test case for rendering all call chains leading to an activity (to) description: + - name: t20037 + title: Test case checking if activities in static variable declarations appear only once + description: Package diagrams: - name: t30001 title: Basic package diagram test case