From a69938f17f7ad4b035e42a7d16c8840223b8cf7b Mon Sep 17 00:00:00 2001 From: Bartek Kryza Date: Tue, 4 Jun 2024 20:49:50 +0200 Subject: [PATCH] Fixed tests after initial refactor --- src/common/types.h | 10 +++- .../mermaid/sequence_diagram_generator.cc | 7 ++- .../plantuml/sequence_diagram_generator.cc | 4 +- src/sequence_diagram/model/diagram.cc | 2 + .../visitor/translation_unit_visitor.cc | 57 ++++++++++--------- 5 files changed, 46 insertions(+), 34 deletions(-) diff --git a/src/common/types.h b/src/common/types.h index d1c3fa64..847283cc 100644 --- a/src/common/types.h +++ b/src/common/types.h @@ -79,6 +79,14 @@ public: friend bool operator!=(const id_t &lhs, const uint64_t &v) { + // This is sadly necessary to catch accidental comparisons to empty + // std::optional: + // + // std::optional id{}; + // if(id != 0) { /* id is nullopt, not 0 - so this executes... */ } + // + assert(v != 0); + return lhs.value_ != v; } @@ -115,7 +123,7 @@ private: * Type of output diagram format generator. */ enum class generator_type_t { - plantuml, /*!< Diagrams will be gnerated in PlantUML format */ + plantuml, /*!< Diagrams will be generated in PlantUML format */ json, /*!< Diagrams will be generated in JSON format */ mermaid /*!< Diagrams will be generated in MermaidJS format */ }; diff --git a/src/sequence_diagram/generators/mermaid/sequence_diagram_generator.cc b/src/sequence_diagram/generators/mermaid/sequence_diagram_generator.cc index 59e4546f..fdf78036 100644 --- a/src/sequence_diagram/generators/mermaid/sequence_diagram_generator.cc +++ b/src/sequence_diagram/generators/mermaid/sequence_diagram_generator.cc @@ -424,8 +424,9 @@ void generator::generate_participant( std::filesystem::path{file_path}, config().root_directory()) .string()); - ostr << indent(1) << "participant " << fmt::format("C_{:022}", file_id) - << " as " << render_participant_name(participant_name); + ostr << indent(1) << "participant " + << fmt::format("C_{:022}", file_id.value()) << " as " + << render_participant_name(participant_name); ostr << '\n'; generated_participants_.emplace(file_id); @@ -474,7 +475,7 @@ std::string generator::generate_alias( config().combine_free_functions_into_file_participants()) { const auto file_id = common::to_id(participant.file()); - return fmt::format("C_{:022}", file_id); + return fmt::format("C_{:022}", file_id.value()); } return participant.alias(); diff --git a/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.cc b/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.cc index 0494beca..ad1a6ac5 100644 --- a/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.cc +++ b/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.cc @@ -429,7 +429,7 @@ void generator::generate_participant( .string()); ostr << "participant \"" << render_name(participant_name) << "\" as " - << fmt::format("C_{:022}", file_id); + << fmt::format("C_{:022}", file_id.value()); ostr << '\n'; @@ -480,7 +480,7 @@ std::string generator::generate_alias( config().combine_free_functions_into_file_participants()) { const auto file_id = common::to_id(participant.file()); - return fmt::format("C_{:022}", file_id); + return fmt::format("C_{:022}", file_id.value()); } return participant.alias(); diff --git a/src/sequence_diagram/model/diagram.cc b/src/sequence_diagram/model/diagram.cc index 98880604..98d033b9 100644 --- a/src/sequence_diagram/model/diagram.cc +++ b/src/sequence_diagram/model/diagram.cc @@ -77,6 +77,8 @@ void diagram::add_participant(std::unique_ptr p) { const auto participant_id = p->id(); + assert(participant_id.is_global()); + if (participants_.find(participant_id) == participants_.end()) { LOG_DBG("Adding '{}' participant: {}, {} [{}]", p->type_name(), p->full_name(false), p->id(), diff --git a/src/sequence_diagram/visitor/translation_unit_visitor.cc b/src/sequence_diagram/visitor/translation_unit_visitor.cc index 90b44b52..b8fe4c8a 100644 --- a/src/sequence_diagram/visitor/translation_unit_visitor.cc +++ b/src/sequence_diagram/visitor/translation_unit_visitor.cc @@ -720,7 +720,7 @@ bool translation_unit_visitor::TraverseIfStmt(clang::IfStmt *stmt) std::any_of(current_elseifstmt->children().begin(), current_elseifstmt->children().end(), child_stmt_compare); - if ((current_caller_id != 0) && !stmt->isConstexpr()) { + if ((current_caller_id.value() != 0) && !stmt->isConstexpr()) { if (elseif_block) { context().enter_elseifstmt(stmt); @@ -745,7 +745,7 @@ bool translation_unit_visitor::TraverseIfStmt(clang::IfStmt *stmt) RecursiveASTVisitor::TraverseIfStmt(stmt); - if ((current_caller_id != 0) && !stmt->isConstexpr()) { + if ((current_caller_id.value() != 0) && !stmt->isConstexpr()) { if (!elseif_block) { diagram().end_block_message( {message_t::kIfEnd, current_caller_id}, message_t::kIf); @@ -768,7 +768,7 @@ bool translation_unit_visitor::TraverseWhileStmt(clang::WhileStmt *stmt) if (config().generate_condition_statements()) condition_text = common::get_condition_text(source_manager(), stmt); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { context().enter_loopstmt(stmt); message m{message_t::kWhile, current_caller_id}; set_source_location(*stmt, m); @@ -779,7 +779,7 @@ bool translation_unit_visitor::TraverseWhileStmt(clang::WhileStmt *stmt) } RecursiveASTVisitor::TraverseWhileStmt(stmt); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { diagram().end_block_message( {message_t::kWhileEnd, current_caller_id}, message_t::kWhile); context().leave_loopstmt(); @@ -800,7 +800,7 @@ bool translation_unit_visitor::TraverseDoStmt(clang::DoStmt *stmt) if (config().generate_condition_statements()) condition_text = common::get_condition_text(source_manager(), stmt); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { context().enter_loopstmt(stmt); message m{message_t::kDo, current_caller_id}; set_source_location(*stmt, m); @@ -812,7 +812,7 @@ bool translation_unit_visitor::TraverseDoStmt(clang::DoStmt *stmt) RecursiveASTVisitor::TraverseDoStmt(stmt); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { diagram().end_block_message( {message_t::kDoEnd, current_caller_id}, message_t::kDo); context().leave_loopstmt(); @@ -833,7 +833,7 @@ bool translation_unit_visitor::TraverseForStmt(clang::ForStmt *stmt) if (config().generate_condition_statements()) condition_text = common::get_condition_text(source_manager(), stmt); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { context().enter_loopstmt(stmt); message m{message_t::kFor, current_caller_id}; set_source_location(*stmt, m); @@ -847,7 +847,7 @@ bool translation_unit_visitor::TraverseForStmt(clang::ForStmt *stmt) RecursiveASTVisitor::TraverseForStmt(stmt); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { diagram().end_block_message( {message_t::kForEnd, current_caller_id}, message_t::kFor); context().leave_loopstmt(); @@ -864,7 +864,7 @@ bool translation_unit_visitor::TraverseCXXTryStmt(clang::CXXTryStmt *stmt) const auto current_caller_id = context().caller_id(); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { context().enter_trystmt(stmt); message m{message_t::kTry, current_caller_id}; set_source_location(*stmt, m); @@ -875,7 +875,7 @@ bool translation_unit_visitor::TraverseCXXTryStmt(clang::CXXTryStmt *stmt) RecursiveASTVisitor::TraverseCXXTryStmt(stmt); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { diagram().end_block_message( {message_t::kTryEnd, current_caller_id}, message_t::kTry); context().leave_trystmt(); @@ -892,7 +892,8 @@ bool translation_unit_visitor::TraverseCXXCatchStmt(clang::CXXCatchStmt *stmt) const auto current_caller_id = context().caller_id(); - if ((current_caller_id != 0) && (context().current_trystmt() != nullptr)) { + if ((current_caller_id.value() != 0) && + (context().current_trystmt() != nullptr)) { std::string caught_type; if (stmt->getCaughtType().isNull()) caught_type = "..."; @@ -923,7 +924,7 @@ bool translation_unit_visitor::TraverseCXXForRangeStmt( if (config().generate_condition_statements()) condition_text = common::get_condition_text(source_manager(), stmt); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { context().enter_loopstmt(stmt); message m{message_t::kFor, current_caller_id}; set_source_location(*stmt, m); @@ -936,7 +937,7 @@ bool translation_unit_visitor::TraverseCXXForRangeStmt( RecursiveASTVisitor::TraverseCXXForRangeStmt( stmt); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { diagram().end_block_message( {message_t::kForEnd, current_caller_id}, message_t::kFor); context().leave_loopstmt(); @@ -951,7 +952,7 @@ bool translation_unit_visitor::TraverseSwitchStmt(clang::SwitchStmt *stmt) const auto current_caller_id = context().caller_id(); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { context().enter_switchstmt(stmt); model::message m{message_t::kSwitch, current_caller_id}; set_source_location(*stmt, m); @@ -962,7 +963,7 @@ bool translation_unit_visitor::TraverseSwitchStmt(clang::SwitchStmt *stmt) RecursiveASTVisitor::TraverseSwitchStmt(stmt); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { context().leave_switchstmt(); diagram().end_block_message( {message_t::kSwitchEnd, current_caller_id}, message_t::kSwitch); @@ -977,7 +978,7 @@ bool translation_unit_visitor::TraverseCaseStmt(clang::CaseStmt *stmt) const auto current_caller_id = context().caller_id(); - if ((current_caller_id != 0) && + if ((current_caller_id.value() != 0) && (context().current_switchstmt() != nullptr)) { model::message m{message_t::kCase, current_caller_id}; m.set_message_name(common::to_string(stmt->getLHS())); @@ -995,7 +996,7 @@ bool translation_unit_visitor::TraverseDefaultStmt(clang::DefaultStmt *stmt) const auto current_caller_id = context().caller_id(); - if ((current_caller_id != 0) && + if ((current_caller_id.value() != 0) && (context().current_switchstmt() != nullptr)) { model::message m{message_t::kCase, current_caller_id}; m.set_message_name("default"); @@ -1018,7 +1019,7 @@ bool translation_unit_visitor::TraverseConditionalOperator( if (config().generate_condition_statements()) condition_text = common::get_condition_text(source_manager(), stmt); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { context().enter_conditionaloperator(stmt); model::message m{message_t::kConditional, current_caller_id}; set_source_location(*stmt, m); @@ -1034,7 +1035,7 @@ bool translation_unit_visitor::TraverseConditionalOperator( RecursiveASTVisitor::TraverseStmt( stmt->getTrueExpr()); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { model::message m{message_t::kConditionalElse, current_caller_id}; set_source_location(*stmt, m); diagram().add_message(std::move(m)); @@ -1043,7 +1044,7 @@ bool translation_unit_visitor::TraverseConditionalOperator( RecursiveASTVisitor::TraverseStmt( stmt->getFalseExpr()); - if (current_caller_id != 0) { + if (current_caller_id.value() != 0) { context().leave_conditionaloperator(); diagram().end_block_message( {message_t::kConditionalEnd, current_caller_id}, @@ -1929,11 +1930,10 @@ std::string translation_unit_visitor::make_lambda_name( const auto location = cls->getLocation(); const std::string source_location{lambda_source_location(location)}; - if (context().lambda_caller_id() != 0) { + if (context().lambda_caller_id().has_value()) { // Parent is also a lambda (this id points to a lambda operator()) std::string parent_lambda_class_name{"()"}; - if (context().lambda_caller_id() && - diagram().get_participant( + if (diagram().get_participant( context().lambda_caller_id().value())) { auto parent_lambda_class_id = diagram() @@ -1955,7 +1955,7 @@ std::string translation_unit_visitor::make_lambda_name( result = fmt::format( "{}##(lambda {})", parent_lambda_class_name, source_location); } - else if (context().caller_id() != 0 && + else if (context().caller_id().value() != 0 && get_participant(context().caller_id()).has_value()) { auto parent_full_name = get_participant(context().caller_id()).value().full_name_no_ns(); @@ -2044,7 +2044,7 @@ void translation_unit_visitor::ensure_lambda_messages_have_operator_as_target() auto participant = diagram().get_participant(m.to()); if (participant && participant.value().is_lambda() && - participant.value().lambda_operator_id() != 0) { + participant.value().lambda_operator_id().value() != 0) { LOG_DBG("Changing lambda expression target id from {} to {}", m.to(), participant.value().lambda_operator_id()); @@ -2067,7 +2067,7 @@ void translation_unit_visitor::resolve_ids_to_global() active_participants_unique.emplace( local_ast_id_map_.at(id.ast_local_value())); } - else { + else if (id.is_global()) { active_participants_unique.emplace(id); } } @@ -2077,7 +2077,7 @@ void translation_unit_visitor::resolve_ids_to_global() // Change all message callees AST local ids to diagram global ids for (auto &[id, activity] : diagram().sequences()) { for (auto &m : activity.messages()) { - if (!id.is_global() && + if (!m.to().is_global() && local_ast_id_map_.find(m.to().ast_local_value()) != local_ast_id_map_.end()) { m.set_to(local_ast_id_map_.at(m.to().ast_local_value())); @@ -2300,7 +2300,8 @@ std::optional translation_unit_visitor::get_expression_comment( if (raw_comment == nullptr) return {}; - if (!processed_comments_by_caller_id_ + if (!caller_id.is_global() && + !processed_comments_by_caller_id_ .emplace(caller_id.ast_local_value(), raw_comment) .second) { return {};