From dbb4dd3caa768069a7a62c840098329872b088c5 Mon Sep 17 00:00:00 2001 From: Bartek Kryza Date: Fri, 2 Sep 2022 23:18:16 +0200 Subject: [PATCH] Fixed glob resolution with multiple threads --- .../plantuml/class_diagram_generator.cc | 2 +- src/common/generators/plantuml/generator.h | 27 +++--- src/config/config.cc | 17 ++++ src/config/config.h | 3 + .../visitor/translation_unit_visitor.cc | 2 +- src/main.cc | 96 +++++++++++++++---- tests/test_cases.cc | 18 ++-- 7 files changed, 121 insertions(+), 44 deletions(-) diff --git a/src/class_diagram/generators/plantuml/class_diagram_generator.cc b/src/class_diagram/generators/plantuml/class_diagram_generator.cc index 86afa46e..68af5b36 100644 --- a/src/class_diagram/generators/plantuml/class_diagram_generator.cc +++ b/src/class_diagram/generators/plantuml/class_diagram_generator.cc @@ -278,7 +278,7 @@ void generator::generate_relationships( target_alias = m_model.to_alias(destination); } catch (...) { - LOG_ERROR("Failed to find alias to {}", destination); + LOG_DBG("Failed to find alias to {}", destination); continue; } diff --git a/src/common/generators/plantuml/generator.h b/src/common/generators/plantuml/generator.h index c818d777..ab0c73fd 100644 --- a/src/common/generators/plantuml/generator.h +++ b/src/common/generators/plantuml/generator.h @@ -335,7 +335,8 @@ template std::unique_ptr generate( const clang::tooling::CompilationDatabase &db, const std::string &name, - DiagramConfig &config, bool verbose = false) + DiagramConfig &config, const std::vector &translation_units, + bool verbose = false) { LOG_INFO("Generating diagram {}.puml", name); @@ -344,24 +345,19 @@ std::unique_ptr generate( diagram->set_filter( std::make_unique(*diagram, config)); - // Get all translation units matching the glob from diagram - // configuration - std::vector translation_units{}; - for (const auto &g : config.glob()) { - LOG_DBG("Processing glob: {}", g); - - const auto matches = glob::rglob(g); - std::copy(matches.begin(), matches.end(), - std::back_inserter(translation_units)); - } - - LOG_DBG("Found translation units: {}", fmt::join(translation_units, ", ")); + LOG_DBG("Found translation units for diagram {}: {}", name, + fmt::join(translation_units, ", ")); clang::tooling::ClangTool clang_tool(db, translation_units); auto action_factory = std::make_unique>(*diagram, config); - clang_tool.run(action_factory.get()); + + auto res = clang_tool.run(action_factory.get()); + + if (res != 0) { + throw std::runtime_error("Diagram " + name + " generation failed"); + } diagram->set_complete(true); @@ -455,5 +451,4 @@ template void generator::init_env() return res; }); } - -} +} \ No newline at end of file diff --git a/src/config/config.cc b/src/config/config.cc index 154fd692..5a286b9d 100644 --- a/src/config/config.cc +++ b/src/config/config.cc @@ -17,6 +17,7 @@ */ #include "config.h" +#include "glob/glob.hpp" #include @@ -101,6 +102,22 @@ void inheritable_diagram_options::inherit( relative_to.override(parent.relative_to); } +std::vector diagram::get_translation_units( + const std::filesystem::path &root_directory) const +{ + std::vector translation_units{}; + + for (const auto &g : glob()) { + const auto matches = glob::glob(g, root_directory); + for (const auto &match : matches) { + const auto path = root_directory / match; + translation_units.emplace_back(path.string()); + } + } + + return translation_units; +} + common::model::diagram_t class_diagram::type() const { return common::model::diagram_t::kClass; diff --git a/src/config/config.h b/src/config/config.h index 0c2ad6e9..e4f1699c 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -147,6 +147,9 @@ struct diagram : public inheritable_diagram_options { virtual common::model::diagram_t type() const = 0; + std::vector get_translation_units( + const std::filesystem::path &root_directory) const; + std::string name; }; diff --git a/src/include_diagram/visitor/translation_unit_visitor.cc b/src/include_diagram/visitor/translation_unit_visitor.cc index 344dab15..b7267df1 100644 --- a/src/include_diagram/visitor/translation_unit_visitor.cc +++ b/src/include_diagram/visitor/translation_unit_visitor.cc @@ -73,7 +73,7 @@ void translation_unit_visitor::include_visitor::InclusionDirective( visited_.find(include_path.string()) != visited_.end()) return; - LOG_INFO("Processing include file {} in file {}", include_path.string(), + LOG_DBG("Processing include file {} in file {}", include_path.string(), current_file.string()); visited_.emplace(include_path.string()); diff --git a/src/main.cc b/src/main.cc index 7558baf8..3fa4e34e 100644 --- a/src/main.cc +++ b/src/main.cc @@ -47,7 +47,8 @@ bool check_output_directory(const std::string &dir); void generate_diagram(const std::string &od, const std::string &name, std::shared_ptr diagram, - const clang::tooling::CompilationDatabase &db, bool verbose); + const clang::tooling::CompilationDatabase &db, + const std::vector &translation_units, bool verbose); int main(int argc, const char *argv[]) { @@ -105,17 +106,6 @@ int main(int argc, const char *argv[]) LOG_INFO("Loading compilation database from {} directory", config.compilation_database_dir()); - std::string err{}; - - auto db = clang::tooling::CompilationDatabase::autoDetectFromDirectory( - config.compilation_database_dir(), err); - - if (!err.empty()) { - LOG_ERROR("Failed to load compilation database from {}", - config.compilation_database_dir()); - return 1; - } - auto od = config.output_directory(); if (output_directory) od = output_directory.value(); @@ -126,15 +116,78 @@ int main(int argc, const char *argv[]) util::thread_pool_executor generator_executor{thread_count}; std::vector> futs; + std::string err{}; + auto db = clang::tooling::CompilationDatabase::autoDetectFromDirectory( + config.compilation_database_dir(), err); + + if (!err.empty()) { + LOG_ERROR("Failed to load compilation database from {}", + config.compilation_database_dir()); + return 1; + } + + const auto compilation_database_files = db->getAllFiles(); + + const auto current_directory = std::filesystem::current_path(); + + std::map /*translation units*/> + translation_units_map; + + // We have to generate the translation units list for each diagram before + // scheduling tasks, because std::filesystem::current_path cannot be trusted + // with multiple threads for (const auto &[name, diagram] : config.diagrams) { // If there are any specific diagram names provided on the command line, // and this diagram is not in that list - skip it if (!diagram_names.empty() && !util::contains(diagram_names, name)) continue; + // Get all translation units matching the glob from diagram + // configuration + std::vector translation_units = + diagram->get_translation_units(current_directory); + + std::vector valid_translation_units{}; + std::copy_if(compilation_database_files.begin(), + compilation_database_files.end(), + std::back_inserter(valid_translation_units), + [&translation_units](const auto &tu) { + return std::find(translation_units.begin(), + translation_units.end(), + tu) != translation_units.end(); + }); + + translation_units_map[name] = std::move(valid_translation_units); + } + + for (const auto &[name, diagram] : config.diagrams) { + // If there are any specific diagram names provided on the command line, + // and this diagram is not in that list - skip it + if (!diagram_names.empty() && !util::contains(diagram_names, name)) + continue; + + const auto& valid_translation_units = translation_units_map[name]; + + if (valid_translation_units.empty()) { + LOG_ERROR( + "Diagram {} generation failed: no translation units found", + name); + continue; + } + futs.emplace_back(generator_executor.add( - [&od, &name = name, &diagram = diagram, &db = db, verbose]() { - generate_diagram(od, name, diagram, *db, verbose); + [&od, &name = name, &diagram = diagram, &config = config, + db = std::ref(*db), + translation_units = std::move(valid_translation_units), + verbose]() { + try { + generate_diagram( + od, name, diagram, db, translation_units, verbose); + } + catch (std::runtime_error &e) { + LOG_ERROR(e.what()); + } })); } @@ -147,7 +200,8 @@ int main(int argc, const char *argv[]) void generate_diagram(const std::string &od, const std::string &name, std::shared_ptr diagram, - const clang::tooling::CompilationDatabase &db, bool verbose) + const clang::tooling::CompilationDatabase &db, + const std::vector &translation_units, bool verbose) { using clanguml::common::model::diagram_t; using clanguml::config::class_diagram; @@ -168,7 +222,8 @@ void generate_diagram(const std::string &od, const std::string &name, auto model = clanguml::common::generators::plantuml::generate(db, diagram->name, - dynamic_cast(*diagram), verbose); + dynamic_cast(*diagram), translation_units, + verbose); ofs << clanguml::class_diagram::generators::plantuml::generator( dynamic_cast(*diagram), *model); @@ -182,7 +237,8 @@ void generate_diagram(const std::string &od, const std::string &name, auto model = clanguml::common::generators::plantuml::generate(db, diagram->name, - dynamic_cast(*diagram), verbose); + dynamic_cast(*diagram), translation_units, + verbose); ofs << clanguml::sequence_diagram::generators::plantuml::generator( dynamic_cast(*diagram), @@ -197,7 +253,8 @@ void generate_diagram(const std::string &od, const std::string &name, auto model = clanguml::common::generators::plantuml::generate(db, diagram->name, - dynamic_cast(*diagram), verbose); + dynamic_cast(*diagram), translation_units, + verbose); ofs << clanguml::package_diagram::generators::plantuml::generator( dynamic_cast(*diagram), *model); @@ -211,7 +268,8 @@ void generate_diagram(const std::string &od, const std::string &name, auto model = clanguml::common::generators::plantuml::generate(db, diagram->name, - dynamic_cast(*diagram), verbose); + dynamic_cast(*diagram), translation_units, + verbose); ofs << clanguml::include_diagram::generators::plantuml::generator( dynamic_cast(*diagram), *model); diff --git a/tests/test_cases.cc b/tests/test_cases.cc index 62e9dfd7..ff8a159c 100644 --- a/tests/test_cases.cc +++ b/tests/test_cases.cc @@ -60,7 +60,8 @@ generate_sequence_diagram(clang::tooling::CompilationDatabase &db, auto model = clanguml::common::generators::plantuml::generate(db, diagram->name, - dynamic_cast(*diagram)); + dynamic_cast(*diagram), + diagram->get_translation_units(std::filesystem::current_path())); return model; } @@ -77,8 +78,9 @@ std::unique_ptr generate_class_diagram( inject_diagram_options(diagram); auto model = clanguml::common::generators::plantuml::generate( - db, diagram->name, dynamic_cast(*diagram)); + diagram_config, diagram_visitor>(db, diagram->name, + dynamic_cast(*diagram), + diagram->get_translation_units(std::filesystem::current_path())); return model; } @@ -95,8 +97,9 @@ generate_package_diagram(clang::tooling::CompilationDatabase &db, inject_diagram_options(diagram); return clanguml::common::generators::plantuml::generate( - db, diagram->name, dynamic_cast(*diagram)); + diagram_config, diagram_visitor>(db, diagram->name, + dynamic_cast(*diagram), + diagram->get_translation_units(std::filesystem::current_path())); } std::unique_ptr @@ -111,8 +114,9 @@ generate_include_diagram(clang::tooling::CompilationDatabase &db, inject_diagram_options(diagram); return clanguml::common::generators::plantuml::generate( - db, diagram->name, dynamic_cast(*diagram)); + diagram_config, diagram_visitor>(db, diagram->name, + dynamic_cast(*diagram), + diagram->get_translation_units(std::filesystem::current_path())); } std::string generate_sequence_puml(