From 468393ddb8071994b216a85f4db74bdac6594d6c Mon Sep 17 00:00:00 2001 From: Bartek Kryza Date: Thu, 5 May 2022 00:34:23 +0200 Subject: [PATCH] WIP Refactoring alias template resolution based on clang canonical representation --- .../plantuml/class_diagram_generator.cc | 2 + src/class_diagram/model/class.cc | 9 +- src/class_diagram/model/class.h | 5 + src/class_diagram/model/class_template.cc | 16 +- src/class_diagram/model/class_template.h | 9 ++ .../visitor/translation_unit_visitor.cc | 152 ++++++++++++++---- .../visitor/translation_unit_visitor.h | 9 +- src/common/generators/plantuml/generator.cc | 2 + src/common/model/enums.cc | 2 + src/common/model/enums.h | 1 + src/cx/util.cc | 5 +- tests/t00014/t00014.cc | 20 ++- tests/t00014/test_case.h | 64 +++++--- 13 files changed, 235 insertions(+), 61 deletions(-) diff --git a/src/class_diagram/generators/plantuml/class_diagram_generator.cc b/src/class_diagram/generators/plantuml/class_diagram_generator.cc index 18d7aa3b..290eb540 100644 --- a/src/class_diagram/generators/plantuml/class_diagram_generator.cc +++ b/src/class_diagram/generators/plantuml/class_diagram_generator.cc @@ -64,6 +64,8 @@ void generator::generate_alias(const class_ &c, std::ostream &ostr) const else full_name = c.full_name(); + //util::replace_all(full_name, "std::basic_string", "std::string"); + if (full_name.empty()) full_name = "<>"; diff --git a/src/class_diagram/model/class.cc b/src/class_diagram/model/class.cc index c1d068ca..f295118c 100644 --- a/src/class_diagram/model/class.cc +++ b/src/class_diagram/model/class.cc @@ -139,10 +139,13 @@ std::ostringstream &class_::render_template_params( if (!templates_.empty()) { std::vector tnames; + std::vector tnames_simplified; + std::transform(templates_.cbegin(), templates_.cend(), - std::back_inserter(tnames), [this](const auto &tmplt) { - return tmplt.to_string(using_namespace()); - }); + std::back_inserter(tnames), + [ns = using_namespace()]( + const auto &tmplt) { return tmplt.to_string(ns); }); + ostr << fmt::format("<{}>", fmt::join(tnames, ",")); } return ostr; diff --git a/src/class_diagram/model/class.h b/src/class_diagram/model/class.h index ff1f194e..e9f17ef9 100644 --- a/src/class_diagram/model/class.h +++ b/src/class_diagram/model/class.h @@ -75,12 +75,17 @@ public: bool is_abstract() const; + bool is_alias() const { return is_alias_; } + + void is_alias(bool alias) { is_alias_ = alias; } + private: std::ostringstream &render_template_params(std::ostringstream &ostr) const; bool is_struct_{false}; bool is_template_{false}; bool is_template_instantiation_{false}; + bool is_alias_{false}; std::vector members_; std::vector methods_; std::vector bases_; diff --git a/src/class_diagram/model/class_template.cc b/src/class_diagram/model/class_template.cc index 589bdede..bb0d1a1f 100644 --- a/src/class_diagram/model/class_template.cc +++ b/src/class_diagram/model/class_template.cc @@ -24,16 +24,21 @@ namespace clanguml::class_diagram::model { class_template::class_template(const std::string &type, const std::string &name, const std::string &default_value, bool is_variadic) - : type_{type} - , name_{name} + : name_{name} , default_value_{default_value} , is_variadic_{is_variadic} { + set_type(type); if (is_variadic) name_ = name_ + "..."; } -void class_template::set_type(const std::string &type) { type_ = type; } +void class_template::set_type(const std::string &type) { + type_ = type; + // TODO: Add a configurable mapping for simplifying non-interesting + // std templates + util::replace_all(type_, "std::basic_string", "std::string"); +} std::string class_template::type() const { return type_; } @@ -60,6 +65,11 @@ bool operator==(const class_template &l, const class_template &r) return (l.name() == r.name()) && (l.type() == r.type()); } +bool operator!=(const class_template &l, const class_template &r) +{ + return !(l == r); +} + std::string class_template::to_string( const clanguml::common::model::namespace_ &using_namespace) const { diff --git a/src/class_diagram/model/class_template.h b/src/class_diagram/model/class_template.h index 603443e4..36cf42d8 100644 --- a/src/class_diagram/model/class_template.h +++ b/src/class_diagram/model/class_template.h @@ -42,9 +42,17 @@ public: bool is_variadic() const noexcept; friend bool operator==(const class_template &l, const class_template &r); + friend bool operator!=(const class_template &l, const class_template &r); std::vector template_params_; + bool is_template_parameter() const { return is_template_parameter_; } + + void is_template_parameter(bool is_template_parameter) + { + is_template_parameter_ = is_template_parameter; + } + std::string to_string( const clanguml::common::model::namespace_ &using_namespace) const; @@ -52,6 +60,7 @@ private: std::string type_; std::string name_; std::string default_value_; + bool is_template_parameter_{false}; bool is_variadic_{false}; }; } diff --git a/src/class_diagram/visitor/translation_unit_visitor.cc b/src/class_diagram/visitor/translation_unit_visitor.cc index bba81ad8..26393398 100644 --- a/src/class_diagram/visitor/translation_unit_visitor.cc +++ b/src/class_diagram/visitor/translation_unit_visitor.cc @@ -200,8 +200,9 @@ void translation_unit_visitor::process_type_alias_template( const cppast::cpp_alias_template &at) { auto alias_kind = at.type_alias().underlying_type().kind(); + if (alias_kind == cppast::cpp_type_kind::unexposed_t) { - LOG_DBG("Template alias has unexposed underlying type: {}", + LOG_DBG("Template alias has unexposed underlying type - ignoring: {}", static_cast( at.type_alias().underlying_type()) .name()); @@ -209,9 +210,16 @@ void translation_unit_visitor::process_type_alias_template( else { if (at.type_alias().underlying_type().kind() == cppast::cpp_type_kind::template_instantiation_t) { + found_relationships_t nested_relationships; auto tinst = build_template_instantiation( static_cast( - resolve_alias(at.type_alias().underlying_type()))); + resolve_alias(at.type_alias().underlying_type())), + nested_relationships); + + tinst->is_alias(true); + + if (tinst->get_namespace().is_empty()) + tinst->set_namespace(ctx.get_namespace()); ctx.add_type_alias_template( cx::util::full_name(ctx.get_namespace(), at), @@ -708,10 +716,20 @@ bool translation_unit_visitor::process_field_with_template_instantiation( auto tr_unaliased_declaration = cppast::to_string(unaliased); std::unique_ptr tinst_ptr; - if (util::contains(tr_declaration, "<")) - tinst_ptr = build_template_instantiation(template_instantiation_type); + // if (util::contains(tr_declaration, "<")) + // tinst_ptr = build_template_instantiation(template_instantiation_type); + // auto t_canon = cppast::to_string(tr.canonical()); + //(void)t_canon; + // else + found_relationships_t nested_relationships; + if (tr_declaration == tr_unaliased_declaration) + tinst_ptr = + build_template_instantiation(unaliased, nested_relationships); else - tinst_ptr = build_template_instantiation(unaliased); + tinst_ptr = build_template_instantiation( + static_cast( + tr.canonical()), + nested_relationships); auto &tinst = *tinst_ptr; @@ -739,6 +757,8 @@ bool translation_unit_visitor::process_field_with_template_instantiation( } } + // Add instantiation relationship from the generated template instantiation + // of the field type to its primary template if (ctx.diagram().should_include(tinst.get_namespace(), tinst.name())) { LOG_DBG("Adding field instantiation relationship {} {} {} : {}", rr.destination(), clanguml::common::model::to_string(rr.type()), @@ -746,18 +766,20 @@ bool translation_unit_visitor::process_field_with_template_instantiation( c.add_relationship(std::move(rr)); - if (tr_declaration != tr_unaliased_declaration) { - // Add template instantiation/specialization relationship; - tinst.add_relationship( - {relationship_t::kInstantiation, tr_unaliased_declaration}); - } - res = true; LOG_DBG("Created template instantiation: {}", tinst.full_name()); ctx.diagram().add_class(std::move(tinst_ptr)); } + else if (!nested_relationships.empty()) { + for (const auto &rel : nested_relationships) { + c.add_relationship({relationship_type, std::get<0>(rel), + detail::cpp_access_specifier_to_access(as), mv.name()}); + } + + res = true; + } return res; } @@ -805,6 +827,8 @@ void translation_unit_visitor::process_field( process_field_with_template_instantiation(mv, tr, c, m, as); } else if (tr.kind() == cppast::cpp_type_kind::template_instantiation_t) { + // This can be either template instantiation or an alias template + // instantiation template_instantiation_added_as_aggregation = process_field_with_template_instantiation(mv, tr, c, m, as); } @@ -1203,9 +1227,10 @@ void translation_unit_visitor:: c.add_relationship(std::move(rr)); } else { + found_relationships_t nested_relationships; // First check if tinst already exists - auto tinst_ptr = - build_template_instantiation(template_instantiation_type); + auto tinst_ptr = build_template_instantiation( + template_instantiation_type, nested_relationships); auto &tinst = *tinst_ptr; relationship rr{relationship_t::kDependency, tinst.full_name()}; @@ -1539,6 +1564,7 @@ bool translation_unit_visitor::find_relationships_in_unexposed_template_params( std::unique_ptr translation_unit_visitor::build_template_instantiation( const cppast::cpp_template_instantiation_type &t, + found_relationships_t &nested_relationships, std::optional parent) { // Create class_ instance to hold the template instantiation @@ -1546,13 +1572,31 @@ std::unique_ptr translation_unit_visitor::build_template_instantiation( auto &tinst = *tinst_ptr; std::string full_template_name; + auto tr_declaration = cppast::to_string(t); + + // If this is an alias - resolve the alias target + const auto &unaliased = + static_cast( + resolve_alias(t)); + auto t_unaliased_declaration = cppast::to_string(unaliased); + + bool t_is_alias = t_unaliased_declaration != tr_declaration; + + // Here we'll hold the template base params to replace with the instantiated + // values std::deque> template_base_params{}; tinst.set_namespace(ctx.get_namespace()); auto tinst_full_name = cppast::to_string(t); + // Typically, every template instantiation should have a primary_template() + // which should also be generated here if it doesn't exist yet in the model + // However if this is a template alias, then it does not have a primary + // template if (t.primary_template().get(ctx.entity_index()).size()) { + auto size = t.primary_template().get(ctx.entity_index()).size(); + (void)size; build_template_instantiation_primary_template( t, tinst, template_base_params, parent, full_template_name); } @@ -1573,11 +1617,8 @@ std::unique_ptr translation_unit_visitor::build_template_instantiation( else tinst.set_namespace(ns); - if (tinst_full_name.find('<') != std::string::npos) { - tinst.set_name(tinst_full_name.substr(0, tinst_full_name.find('<'))); - } - tinst.is_template_instantiation(true); + tinst.is_alias(t_is_alias); if (tinst.full_name().substr(0, tinst.full_name().find('<')).find("::") == std::string::npos) { @@ -1596,7 +1637,7 @@ std::unique_ptr translation_unit_visitor::build_template_instantiation( class_template ct; if (targ.type()) { build_template_instantiation_process_type_argument( - parent, tinst, targ, ct); + parent, tinst, targ, ct, nested_relationships); } else if (targ.expression()) { build_template_instantiation_process_expression_argument( @@ -1631,8 +1672,43 @@ std::unique_ptr translation_unit_visitor::build_template_instantiation( destination = cppast::to_string(ctx.get_type_alias(fn).get()); } else { - // Otherwise point to the base template - destination = tinst.base_template(); + std::string tmp_destination{}; + + int best_match = 0; + // First try to find the best match for this template in partially + // specialized templates + for (const auto &c : ctx.diagram().classes()) { + if (c->name_and_ns() == full_template_name && + // TODO: handle variadic templates + c->templates().size() == tinst.templates().size()) { + int tmp_match = 0; + for (int i = 0; i < c->templates().size(); i++) { + const auto& tinst_i = tinst.templates().at(i); + const auto& c_i = c->templates().at(i); + if ((c_i == tinst_i) && !c_i.is_template_parameter()) { + tmp_match++; + } + else if(c_i.is_template_parameter() || tinst_i.is_template_parameter()) { + // continue + } + else { + // Non-free types are not compatible + tmp_match = 0; + break; + } + } + if (tmp_match > best_match) { + best_match = tmp_match; + tmp_destination = c->full_name(false); + } + } + } + + if (!tmp_destination.empty()) + destination = tmp_destination; + else + // Otherwise point to the base template + destination = tinst.base_template(); } relationship r{relationship_t::kInstantiation, destination}; tinst.add_relationship(std::move(r)); @@ -1691,7 +1767,7 @@ void translation_unit_visitor:: build_template_instantiation_process_type_argument( const std::optional &parent, class_ &tinst, const cppast::cpp_template_argument &targ, - class_template &ct) + class_template &ct, found_relationships_t &nested_relationships) { ct.set_type(cppast::to_string(targ.type().value())); @@ -1702,7 +1778,10 @@ void translation_unit_visitor:: auto [fn_ns, fn_name] = cx::util::split_ns(fn); - if (targ.type().value().kind() == + if(targ.type().value().kind() == cppast::cpp_type_kind::template_parameter_t) { + ct.is_template_parameter(true); + } + else if (targ.type().value().kind() == cppast::cpp_type_kind::template_instantiation_t) { const auto &nested_template_parameter = @@ -1716,18 +1795,23 @@ void translation_unit_visitor:: auto [tinst_ns, tinst_name] = cx::util::split_ns(tinst.full_name(false)); - auto nested_tinst = - build_template_instantiation(nested_template_parameter, - ctx.diagram().should_include(tinst_ns, tinst_name) - ? std::make_optional(&tinst) - : parent); + auto nested_tinst = build_template_instantiation( + nested_template_parameter, nested_relationships, + ctx.diagram().should_include(tinst_ns, tinst_name) + ? std::make_optional(&tinst) + : parent); relationship tinst_dependency{ relationship_t::kDependency, nested_tinst->full_name()}; - auto nested_tinst_full_name = nested_tinst->full_name(); + auto nested_tinst_full_name = nested_tinst->full_name(false); - if (ctx.diagram().should_include(fn_ns, fn_name)) { + auto [nested_tinst_ns, nested_tinst_name] = + cx::util::split_ns(nested_tinst_full_name); + + if (ctx.diagram().should_include(nested_tinst_ns, nested_tinst_name)) { + nested_relationships.push_back( + {nested_tinst->full_name(false), relationship_t::kAggregation}); ctx.diagram().add_class(std::move(nested_tinst)); } @@ -1769,6 +1853,8 @@ void translation_unit_visitor:: if (ctx.diagram().should_include(fn_ns, fn_name)) { tinst.add_relationship(std::move(tinst_dependency)); + nested_relationships.push_back( + {tinst_dependency.destination(), relationship_t::kAggregation}); } else if (parent) { (*parent)->add_relationship(std::move(tinst_dependency)); @@ -1870,9 +1956,17 @@ const cppast::cpp_type &translation_unit_visitor::resolve_alias( if (util::contains(type_full_name, "<")) type_full_name = util::split(type_full_name, "<")[0]; + auto type_full_name_in_current_ns = ctx.get_namespace(); + type_full_name_in_current_ns |= common::model::namespace_{type_full_name}; + if (ctx.has_type_alias_template(type_full_name)) { return ctx.get_type_alias(type_full_name).get(); } + else if (ctx.has_type_alias_template( + type_full_name_in_current_ns.to_string())) { + return ctx.get_type_alias(type_full_name_in_current_ns.to_string()) + .get(); + } else if (ctx.has_type_alias(type_full_name)) { return ctx.get_type_alias_final(raw_type).get(); } diff --git a/src/class_diagram/visitor/translation_unit_visitor.h b/src/class_diagram/visitor/translation_unit_visitor.h index 1a7fcf6d..3a9b4213 100644 --- a/src/class_diagram/visitor/translation_unit_visitor.h +++ b/src/class_diagram/visitor/translation_unit_visitor.h @@ -163,8 +163,15 @@ private: std::unique_ptr build_template_instantiation( const cppast::cpp_template_instantiation_type &t, + found_relationships_t &relationships, std::optional parent = {}); + // std::vector> + std::unique_ptr + build_alias_template_instantiation( + const cppast::cpp_template_instantiation_type &alias_templ, + std::optional parent); + /** * Try to resolve a type instance into a type referenced through an alias. * If t does not represent an alias, returns t. @@ -208,7 +215,7 @@ private: void build_template_instantiation_process_type_argument( const std::optional &parent, model::class_ &tinst, const cppast::cpp_template_argument &targ, - model::class_template &ct); + model::class_template &ct, found_relationships_t &relationships); void build_template_instantiation_process_expression_argument( const cppast::cpp_template_argument &targ, diff --git a/src/common/generators/plantuml/generator.cc b/src/common/generators/plantuml/generator.cc index eea26a9c..b10fd4c4 100644 --- a/src/common/generators/plantuml/generator.cc +++ b/src/common/generators/plantuml/generator.cc @@ -37,6 +37,8 @@ std::string to_plantuml(relationship_t r, std::string style) return style.empty() ? "<.." : fmt::format("<.[{}].", style); case relationship_t::kDependency: return style.empty() ? "..>" : fmt::format(".[{}].>", style); + case relationship_t::kAlias: + return style.empty() ? ".." : fmt::format(".[{}].", style); default: return ""; } diff --git a/src/common/model/enums.cc b/src/common/model/enums.cc index 218844b9..d6fdce79 100644 --- a/src/common/model/enums.cc +++ b/src/common/model/enums.cc @@ -44,6 +44,8 @@ std::string to_string(relationship_t r) return "friendship"; case relationship_t::kDependency: return "dependency"; + case relationship_t::kAlias: + return "alias"; default: assert(false); } diff --git a/src/common/model/enums.h b/src/common/model/enums.h index 7c2501c2..e9c3cd2e 100644 --- a/src/common/model/enums.h +++ b/src/common/model/enums.h @@ -35,6 +35,7 @@ enum class relationship_t { kAssociation, kInstantiation, kFriendship, + kAlias, kDependency }; diff --git a/src/cx/util.cc b/src/cx/util.cc index 40f0ad71..c589f0e7 100644 --- a/src/cx/util.cc +++ b/src/cx/util.cc @@ -162,8 +162,9 @@ std::string ns(const cppast::cpp_type &t, const cppast::cpp_entity_index &idx) .get(idx)[0] .get()); } - else - return ""; + else { + return {}; + } } else { auto canon = cppast::to_string(t.canonical()); diff --git a/tests/t00014/t00014.cc b/tests/t00014/t00014.cc index a1e3824a..8b13a7b2 100644 --- a/tests/t00014/t00014.cc +++ b/tests/t00014/t00014.cc @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -27,6 +28,13 @@ template struct A { }; template using AString = A; +template using AStringPtr = A>; + +template using VectorPtr = std::unique_ptr>; +template using APtr = std::unique_ptr>; +template using ASharedPtr = std::shared_ptr>; + template using AAPtr = + std::unique_ptr, A>>; template using GeneralCallback = std::function; using VoidCallback = GeneralCallback<>; @@ -42,12 +50,19 @@ using AIntString = AString; using AStringString = AString; using BStringString = AStringString; +template using PairPairBA = std::pair>, long>; + class R { - // clang-uml: tinst A + PairPairBA bapair; + + APtr abool; + AAPtr aboolfloat; + ASharedPtr afloat; A boolstring; - AString floatstring; + AStringPtr floatstring; AIntString intstring; AStringString stringstring; + BStringString bstringstring; protected: BVector bs; @@ -56,6 +71,7 @@ public: BVector2 bs2; GeneralCallback cb; VoidCallback vcb; + VectorPtr vps; }; } } diff --git a/tests/t00014/test_case.h b/tests/t00014/test_case.h index 70a414af..6e9790e5 100644 --- a/tests/t00014/test_case.h +++ b/tests/t00014/test_case.h @@ -36,31 +36,53 @@ TEST_CASE("t00014", "[test-case][class]") REQUIRE_THAT(puml, EndsWith("@enduml\n")); REQUIRE_THAT(puml, IsClassTemplate("A", "T,P")); REQUIRE_THAT(puml, IsClassTemplate("A", "T,std::string")); + REQUIRE_THAT(puml, IsClassTemplate("A", "T,std::unique_ptr")); + REQUIRE_THAT(puml, IsClassTemplate("A", "double,T")); + REQUIRE_THAT(puml, IsClassTemplate("A", "long,U")); + REQUIRE_THAT(puml, IsClassTemplate("A", "long,T")); + REQUIRE_THAT(puml, IsClassTemplate("A", "long,bool")); + REQUIRE_THAT(puml, IsClassTemplate("A", "double,bool")); + REQUIRE_THAT(puml, IsClassTemplate("A", "long,float")); + REQUIRE_THAT(puml, IsClassTemplate("A", "double,float")); REQUIRE_THAT(puml, IsClassTemplate("A", "bool,std::string")); - REQUIRE_THAT(puml, IsClassTemplate("AString", "float")); - REQUIRE_THAT(puml, IsClassTemplate("AString", "int")); - REQUIRE_THAT(puml, IsClassTemplate("AString", "std::string")); + REQUIRE_THAT(puml, IsClass(_A("B"))); + REQUIRE_THAT( puml, !IsClassTemplate("std::std::function", "void(T...,int),int)")); - REQUIRE_THAT(puml, IsInstantiation(_A("A"), _A("A"))); - REQUIRE_THAT( - puml, IsInstantiation(_A("A"), _A("AString"))); - REQUIRE_THAT( - puml, IsInstantiation(_A("A"), _A("AString"))); - REQUIRE_THAT( - puml, !IsInstantiation(_A("AString"), _A("AString"))); - REQUIRE_THAT(puml, - IsInstantiation(_A("A"), _A("AString"))); - REQUIRE_THAT(puml, - !IsInstantiation( - _A("AString"), _A("AString"))); - REQUIRE_THAT( - puml, IsAggregation(_A("R"), _A("A"), "-boolstring")); - REQUIRE_THAT( - puml, IsAggregation(_A("R"), _A("AString"), "-floatstring")); - REQUIRE_THAT(puml, IsAggregation(_A("R"), _A("B"), "#bs")); - REQUIRE_THAT(puml, IsAggregation(_A("R"), _A("B"), "+bs2")); +// REQUIRE_THAT(puml, IsInstantiation(_A("A"), _A("A"))); +// REQUIRE_THAT( +// puml, IsInstantiation(_A("A"), _A("AString"))); +// REQUIRE_THAT( +// puml, IsInstantiation(_A("A"), _A("AString"))); +// REQUIRE_THAT( +// puml, !IsInstantiation(_A("AString"), _A("AString"))); +// REQUIRE_THAT(puml, +// IsInstantiation(_A("A"), _A("AString"))); +// REQUIRE_THAT( +// puml, IsInstantiation(_A("A"), _A("A"))); +// +// REQUIRE_THAT(puml, +// !IsInstantiation( +// _A("AString"), _A("AString"))); + + + REQUIRE_THAT(puml, IsAggregation(_A("R"), _A("B"), "+vps")); + REQUIRE_THAT(puml, IsAggregation(_A("R"), _A("B"), "-bapair")); + +// REQUIRE_THAT( +// puml, IsAggregation(_A("R"), _A("A"), "-boolstring")); +// +// +// +// REQUIRE_THAT( +// puml, IsAggregation(_A("R"), _A("A"), "-boolstring")); +// REQUIRE_THAT( +// puml, IsAggregation(_A("R"), _A("AString"), "-floatstring")); +// REQUIRE_THAT(puml, IsAggregation(_A("R"), _A("B"), "#bs")); +// REQUIRE_THAT(puml, IsAggregation(_A("R"), _A("B"), "+bs2")); +// REQUIRE_THAT(puml, IsAggregation(_A("R"), _A("B"), "+vsp")); +// REQUIRE_THAT(puml, IsAggregation(_A("R"), _A("A"), "+bvsp")); save_puml( "./" + config.output_directory() + "/" + diagram->name + ".puml", puml);