From 72a5e999cbc40e981929d1011f71f621f3b0a4c9 Mon Sep 17 00:00:00 2001 From: Jean-Marie HENAFF Date: Fri, 16 Feb 2024 18:09:03 +0100 Subject: [PATCH 1/3] fix(grpcgen): remove ambiguous message about missing --file when using compilation database When using a compilation database, a message stating that the "File was missing" was printed, leading to think something was wrong whereas it's expected behavior. Also: - perform option validation to keep --file and --database_dir mutually exclusive - handle parsing error (used to be ignored for --database_dir path) --- tools/generators/grpc/xpcf_grpc_gen.cpp | 32 +++++++++++++++---------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/tools/generators/grpc/xpcf_grpc_gen.cpp b/tools/generators/grpc/xpcf_grpc_gen.cpp index f50268e..d9cf1b3 100755 --- a/tools/generators/grpc/xpcf_grpc_gen.cpp +++ b/tools/generators/grpc/xpcf_grpc_gen.cpp @@ -220,6 +220,14 @@ try serviceGenerator->setDestinationFolder(options["output"].as()); } + bool fileOptIsEmpty = !options.count("file") || options["file"].as().empty(); + bool databaseDirOptIsEmpty = !options.count("database_dir") || options["database_dir"].as().empty(); + + if (!fileOptIsEmpty && !databaseDirOptIsEmpty) { + print_error("--file and --database_dir cannot be both set"); + return 1; + } + auto astParser = cmpMgr->resolve("astParser"); int result = astParser->initOptions(options); @@ -229,21 +237,19 @@ try cppast::cpp_entity_index idx; // the entity index is used to resolve cross references in the AST - if (!options.count("file") || options["file"].as().empty()) { - if (options.count("database_dir") && !options["database_dir"].as().empty()) { - std::cout<<"File argument is missing : parsing every file listed in database"<parse_database(options["database_dir"].as(),options); - //parse_database(options["database_dir"].as(),idx,options, [&](const cppast::cpp_entity_index& idx, std::ostream& out, const cppast::cpp_file& file) { astParser->parseAst(idx,out,file); }); + if (!databaseDirOptIsEmpty) { + if (int res = astParser->parse_database(options["database_dir"].as(),options) != 0) { + return res; } - else { - print_error("missing one of file or database dir argument"); - return 1; + //parse_database(options["database_dir"].as(),idx,options, [&](const cppast::cpp_entity_index& idx, std::ostream& out, const cppast::cpp_file& file) { astParser->parseAst(idx,out,file); }); + } else if (!fileOptIsEmpty) { + assert(databaseDirOptIsEmpty); + if (int res = astParser->parse_file(options["file"].as(), options.count("fatal_errors") == 1) != 0) { + return res; } - } - else { - result = astParser->parse_file(options["file"].as(), options.count("fatal_errors") == 1); - if (result != 0) - return result; + } else { + print_error("missing one of --file or --database_dir argument"); + return 1; } //update types : try to qualify non fqdn types in parameters ... from classes found during parsing From fafb0bb8e36db334fe9b7873e16c7dc1705fa9c1 Mon Sep 17 00:00:00 2001 From: Jean-Marie HENAFF Date: Fri, 16 Feb 2024 18:19:12 +0100 Subject: [PATCH 2/3] fix: make proper assert about --file/--database_dir mutual exclusivity --- tools/generators/grpc/xpcf_grpc_gen.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/generators/grpc/xpcf_grpc_gen.cpp b/tools/generators/grpc/xpcf_grpc_gen.cpp index d9cf1b3..4c6dbd6 100755 --- a/tools/generators/grpc/xpcf_grpc_gen.cpp +++ b/tools/generators/grpc/xpcf_grpc_gen.cpp @@ -238,12 +238,12 @@ try cppast::cpp_entity_index idx; // the entity index is used to resolve cross references in the AST if (!databaseDirOptIsEmpty) { + assert(fileOptIsEmpty); if (int res = astParser->parse_database(options["database_dir"].as(),options) != 0) { return res; } //parse_database(options["database_dir"].as(),idx,options, [&](const cppast::cpp_entity_index& idx, std::ostream& out, const cppast::cpp_file& file) { astParser->parseAst(idx,out,file); }); } else if (!fileOptIsEmpty) { - assert(databaseDirOptIsEmpty); if (int res = astParser->parse_file(options["file"].as(), options.count("fatal_errors") == 1) != 0) { return res; } From bc064b53cd01133a9d723f67a9b76259180c5978 Mon Sep 17 00:00:00 2001 From: Jean-Marie HENAFF Date: Mon, 26 Feb 2024 14:10:13 +0100 Subject: [PATCH 3/3] fix: update "missing file" message and remove mutual exclusion change Options --file and --database_dir are actually not mutually exclusive. When parsing a file, the compilation database is used to resolve symbols when passed to initOptions(). Change "File is missing" by a message that sounds less as an error. --- tools/generators/grpc/xpcf_grpc_gen.cpp | 32 ++++++++++--------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/tools/generators/grpc/xpcf_grpc_gen.cpp b/tools/generators/grpc/xpcf_grpc_gen.cpp index 4c6dbd6..fa0bbe7 100755 --- a/tools/generators/grpc/xpcf_grpc_gen.cpp +++ b/tools/generators/grpc/xpcf_grpc_gen.cpp @@ -220,14 +220,6 @@ try serviceGenerator->setDestinationFolder(options["output"].as()); } - bool fileOptIsEmpty = !options.count("file") || options["file"].as().empty(); - bool databaseDirOptIsEmpty = !options.count("database_dir") || options["database_dir"].as().empty(); - - if (!fileOptIsEmpty && !databaseDirOptIsEmpty) { - print_error("--file and --database_dir cannot be both set"); - return 1; - } - auto astParser = cmpMgr->resolve("astParser"); int result = astParser->initOptions(options); @@ -237,19 +229,21 @@ try cppast::cpp_entity_index idx; // the entity index is used to resolve cross references in the AST - if (!databaseDirOptIsEmpty) { - assert(fileOptIsEmpty); - if (int res = astParser->parse_database(options["database_dir"].as(),options) != 0) { - return res; + if (!options.count("file") || options["file"].as().empty()) { + if (options.count("database_dir") && !options["database_dir"].as().empty()) { + std::cout<<"--file is not set : parsing every file listed in database"<parse_database(options["database_dir"].as(),options); + //parse_database(options["database_dir"].as(),idx,options, [&](const cppast::cpp_entity_index& idx, std::ostream& out, const cppast::cpp_file& file) { astParser->parseAst(idx,out,file); }); } - //parse_database(options["database_dir"].as(),idx,options, [&](const cppast::cpp_entity_index& idx, std::ostream& out, const cppast::cpp_file& file) { astParser->parseAst(idx,out,file); }); - } else if (!fileOptIsEmpty) { - if (int res = astParser->parse_file(options["file"].as(), options.count("fatal_errors") == 1) != 0) { - return res; + else { + print_error("missing one of file or database dir argument"); + return 1; } - } else { - print_error("missing one of --file or --database_dir argument"); - return 1; + } + else { + result = astParser->parse_file(options["file"].as(), options.count("fatal_errors") == 1); + if (result != 0) + return result; } //update types : try to qualify non fqdn types in parameters ... from classes found during parsing