From 661622d417e037e1ba22e727af84883b7544f516 Mon Sep 17 00:00:00 2001 From: Bartek Kryza Date: Wed, 12 Feb 2025 00:27:31 +0100 Subject: [PATCH] Fixed handling of forward declarations in class diagrams (#388) --- CHANGELOG.md | 1 + .../visitor/translation_unit_visitor.cc | 81 +++++++++++++++---- .../visitor/translation_unit_visitor.h | 11 +++ src/common/model/diagram_element.h | 6 +- tests/t00091/.clang-uml | 10 +++ tests/t00091/t00091_a.cc | 17 ++++ tests/t00091/t00091_b.cc | 17 ++++ tests/t00091/test_case.h | 45 +++++++++++ tests/test_cases.cc | 3 + tests/test_cases.yaml | 3 + 10 files changed, 176 insertions(+), 18 deletions(-) create mode 100644 tests/t00091/.clang-uml create mode 100644 tests/t00091/t00091_a.cc create mode 100644 tests/t00091/t00091_b.cc create mode 100644 tests/t00091/test_case.h diff --git a/CHANGELOG.md b/CHANGELOG.md index d707e982..b72e3bc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # CHANGELOG + * Fixed handling of forward declarations in class diagrams (#388) * Added support for coroutines in sequence diagrams (#376) * Fixed supported for compile_flags.txt (#381) * Added separate return messages for each return branch in sequence diagrams diff --git a/src/class_diagram/visitor/translation_unit_visitor.cc b/src/class_diagram/visitor/translation_unit_visitor.cc index 5f08162b..53a68839 100644 --- a/src/class_diagram/visitor/translation_unit_visitor.cc +++ b/src/class_diagram/visitor/translation_unit_visitor.cc @@ -142,6 +142,25 @@ bool translation_unit_visitor::VisitEnumDecl(clang::EnumDecl *enm) enm->getLocation().printToString(source_manager())); auto e_ptr = create_enum_declaration(enm, nullptr); + if (!e_ptr) + return true; + + auto id = e_ptr->id(); + + auto &enum_model = diagram().find(id).has_value() + ? *diagram().find(id).get() + : *e_ptr; + + if (enm->isCompleteDefinition() && !enum_model.complete()) + process_enum_declaration(*enm, enum_model); + + if (!enm->isCompleteDefinition()) { + enum_forward_declarations_.emplace(id, std::move(e_ptr)); + return true; + } + enum_forward_declarations_.erase(id); + + e_ptr->complete(true); if (e_ptr && diagram().should_include(*e_ptr)) add_enum(std::move(e_ptr)); @@ -206,11 +225,15 @@ translation_unit_visitor::create_enum_declaration( e.set_style(e.style_spec()); - for (const auto &ev : enm->enumerators()) { + return e_ptr; +} + +void translation_unit_visitor::process_enum_declaration( + const clang::EnumDecl &enm, clanguml::class_diagram::model::enum_ &e) +{ + for (const auto &ev : enm.enumerators()) { e.constants().push_back(ev->getNameAsString()); } - - return e_ptr; } bool translation_unit_visitor::VisitClassTemplateSpecializationDecl( @@ -236,15 +259,28 @@ bool translation_unit_visitor::VisitClassTemplateSpecializationDecl( if (!template_specialization_ptr) return true; - auto &template_specialization = *template_specialization_ptr; + auto id = template_specialization_ptr->id(); + + auto &template_specialization = diagram().find(id).has_value() + ? *diagram().find(id).get() + : *template_specialization_ptr; - if (cls->hasDefinition()) { + if (cls->hasDefinition() && !template_specialization.complete()) { // Process template specialization bases process_class_bases(cls, template_specialization); // Process class child entities process_class_children(cls, template_specialization); + + template_specialization.complete(true); + } + + if (!cls->isCompleteDefinition()) { + forward_declarations_.emplace( + id, std::move(template_specialization_ptr)); + return true; } + forward_declarations_.erase(id); if (!template_specialization.template_specialization_found()) { // Only do this if we haven't found a better specialization during @@ -258,7 +294,6 @@ bool translation_unit_visitor::VisitClassTemplateSpecializationDecl( if (diagram().should_include(template_specialization)) { const auto full_name = template_specialization.full_name(false); - const auto id = template_specialization.id(); LOG_DBG("Adding class template specialization {} with id {}", full_name, id); @@ -347,11 +382,19 @@ bool translation_unit_visitor::VisitClassTemplateDecl( find_relationships_in_constraint_expression(*c_ptr, expr); } + auto &template_model = diagram().find(id).has_value() + ? *diagram().find(id).get() + : *c_ptr; + + if (cls->getTemplatedDecl()->hasDefinition() && + !template_model.complete()) { + process_class_declaration(*cls->getTemplatedDecl(), template_model); + } + if (!cls->getTemplatedDecl()->isCompleteDefinition()) { forward_declarations_.emplace(id, std::move(c_ptr)); return true; } - process_class_declaration(*cls->getTemplatedDecl(), *c_ptr); forward_declarations_.erase(id); if (diagram().should_include(*c_ptr)) { @@ -366,6 +409,9 @@ bool translation_unit_visitor::VisitClassTemplateDecl( bool translation_unit_visitor::VisitRecordDecl(clang::RecordDecl *rec) { + if (rec == nullptr) + return true; + if (clang::dyn_cast_or_null(rec) != nullptr) // This is handled by VisitCXXRecordDecl() return true; @@ -844,6 +890,7 @@ 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()); + LOG_DBG("== isCompleteDefinition() = {}", cls->isCompleteDefinition()); if (const auto *parent_record = clang::dyn_cast(cls->getParent()); @@ -2473,6 +2520,13 @@ void translation_unit_visitor::add_incomplete_forward_declarations() } } forward_declarations_.clear(); + + for (auto &[id, e] : enum_forward_declarations_) { + if (diagram().should_include(e->get_namespace())) { + add_enum(std::move(e)); + } + } + enum_forward_declarations_.clear(); } void translation_unit_visitor::resolve_local_to_global_ids() @@ -2553,8 +2607,6 @@ void translation_unit_visitor::add_diagram_element( void translation_unit_visitor::add_class(std::unique_ptr &&c) { - c->complete(true); - if ((config().generate_packages() && config().package_type() == config::package_type_t::kDirectory)) { assert(!c->file().empty()); @@ -2584,8 +2636,6 @@ void translation_unit_visitor::add_class(std::unique_ptr &&c) void translation_unit_visitor::add_objc_interface( std::unique_ptr &&c) { - c->complete(true); - if ((config().generate_packages() && config().package_type() == config::package_type_t::kDirectory)) { assert(!c->file().empty()); @@ -2605,8 +2655,6 @@ void translation_unit_visitor::add_objc_interface( void translation_unit_visitor::add_enum(std::unique_ptr &&e) { - e->complete(true); - if ((config().generate_packages() && config().package_type() == config::package_type_t::kDirectory)) { assert(!e->file().empty()); @@ -2635,8 +2683,6 @@ void translation_unit_visitor::add_enum(std::unique_ptr &&e) void translation_unit_visitor::add_concept(std::unique_ptr &&c) { - c->complete(true); - if ((config().generate_packages() && config().package_type() == config::package_type_t::kDirectory)) { assert(!c->file().empty()); @@ -2711,6 +2757,11 @@ void translation_unit_visitor::find_instantiation_relationships( templated_decl_global_id}); template_instantiation.template_specialization_found(true); } + else if (id_mapper().get_global_id(templated_decl_id).has_value()) { + template_instantiation.add_relationship( + {common::model::relationship_t::kInstantiation, templated_decl_id}); + template_instantiation.template_specialization_found(true); + } else if (diagram().should_include(common::model::namespace_{full_name})) { LOG_DBG("Skipping instantiation relationship from {} to {}", template_instantiation, templated_decl_global_id); diff --git a/src/class_diagram/visitor/translation_unit_visitor.h b/src/class_diagram/visitor/translation_unit_visitor.h index c3ebeae5..12cbb5fa 100644 --- a/src/class_diagram/visitor/translation_unit_visitor.h +++ b/src/class_diagram/visitor/translation_unit_visitor.h @@ -241,6 +241,15 @@ class translation_unit_visitor void process_class_declaration(const clang::CXXRecordDecl &cls, clanguml::class_diagram::model::class_ &c); + /** + * @brief Process enum declaration + * + * @param enm Enum declaration + * @param e Enum diagram element returned from `create_enum_declaration` + */ + void process_enum_declaration( + const clang::EnumDecl &enm, clanguml::class_diagram::model::enum_ &e); + /** * @brief Process Objective-C category declaration * @@ -561,6 +570,8 @@ class translation_unit_visitor std::map> forward_declarations_; + std::map> + enum_forward_declarations_; std::map + +namespace clanguml::t00091 { +struct B; + +template struct C; + +template <> struct C; + +enum class D; + +struct R { + B *b; + C *c; + D d; +}; +} // namespace clanguml::t00091 \ No newline at end of file diff --git a/tests/t00091/t00091_b.cc b/tests/t00091/t00091_b.cc new file mode 100644 index 00000000..f784bf99 --- /dev/null +++ b/tests/t00091/t00091_b.cc @@ -0,0 +1,17 @@ +#include + +namespace clanguml::t00091 { +struct B { + int value; +}; + +template struct C { + T value; +}; + +template <> struct C { + std::string value; +}; + +enum class D { one, two, three }; +} // namespace clanguml::t00091 \ No newline at end of file diff --git a/tests/t00091/test_case.h b/tests/t00091/test_case.h new file mode 100644 index 00000000..c4154172 --- /dev/null +++ b/tests/t00091/test_case.h @@ -0,0 +1,45 @@ +/** + * tests/t00091/test_case.h + * + * Copyright (c) 2021-2025 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("t00091") +{ + using namespace clanguml::test; + using namespace std::string_literals; + + auto [config, db, diagram, model] = + CHECK_CLASS_MODEL("t00091", "t00091_class"); + + CHECK_CLASS_DIAGRAM(*config, diagram, *model, [](const auto &src) { + REQUIRE(IsClass(src, "B")); + REQUIRE(IsClassTemplate(src, "C")); + REQUIRE(IsClassTemplate(src, "C")); + REQUIRE(IsClassTemplate(src, "C")); + REQUIRE(IsEnum(src, "D")); + + REQUIRE(IsField(src, "R", "b", "B *")); + REQUIRE(IsField(src, "R", "c", "C *")); + REQUIRE(IsField(src, "R", "d", "D")); + + REQUIRE(IsField(src, "B", "value", "int")); + REQUIRE(IsField(src, "C", "value", "T")); + REQUIRE(IsField(src, "C", "value", "std::string")); + + REQUIRE(IsInstantiation(src, "C", "C")); + REQUIRE(IsInstantiation(src, "C", "C")); + }); +} \ No newline at end of file diff --git a/tests/test_cases.cc b/tests/test_cases.cc index 1bf70b4b..1c828beb 100644 --- a/tests/test_cases.cc +++ b/tests/test_cases.cc @@ -594,6 +594,9 @@ void CHECK_INCLUDE_DIAGRAM(const clanguml::config::config &config, #if defined(ENABLE_CXX_STD_20_TEST_CASES) #include "t00090/test_case.h" #endif + +#include "t00091/test_case.h" + /// /// Sequence diagram tests /// diff --git a/tests/test_cases.yaml b/tests/test_cases.yaml index bc926715..dfc3cb6b 100644 --- a/tests/test_cases.yaml +++ b/tests/test_cases.yaml @@ -267,6 +267,9 @@ test_cases: - name: t00090 title: Metaprogramming test case with recursive type list description: + - name: t00091 + title: Declaration forwarding test case + description: Sequence diagrams: - name: t20001 title: Basic sequence diagram test case