From 2bbdf6c02bd8f608df967a1a2db5a34666c2ef60 Mon Sep 17 00:00:00 2001 From: ykiko Date: Wed, 8 Apr 2026 22:18:25 +0800 Subject: [PATCH] =?UTF-8?q?refactor(command):=20split=20CompilationContext?= =?UTF-8?q?=20into=20ResolvedFlags=20=E2=86=92=20CompileCommand=20?= =?UTF-8?q?=E2=86=92=20to=5Fargv()=20(#408)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Replace flat `CompilationContext { directory, arguments }` with a three-layer abstraction: `ResolvedFlags` (file-independent flags) → `CompileCommand` (+ source file) → `to_argv()` (full argv on demand) - `ResolvedFlags.flags` never contains source file path or `-main-file-name`, making it directly usable as a clean cache key input (e.g. PCH sharing across files with identical preambles) - `to_argv()` handles `-main-file-name` insertion for cc1 mode automatically — consumers no longer need to search/replace in the argument list - Eliminates the pollute-then-clean anti-pattern in `lookup()` and the manual source-file replacement in `fill_header_context_args()` ## Test plan - [x] `pixi run format` — no changes - [x] `pixi run unit-test` — 481 passed - [x] `pixi run integration-test` — 113 passed 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **Refactor** * Unified compile-command handling across the server and tools for more consistent argument and flag behavior (driver vs frontend modes). * **New Features** * Added an LRU-backed in-memory cache to improve performance and eviction control. * **Chores** * Added an option to control injection of resource-directory flags (enabled by default). * **Tests** * Updated unit and integration tests to adopt the new command representation and verify cache behavior. --------- Co-authored-by: Claude Opus 4.6 --- src/command/command.cpp | 157 ++++++++++++------ src/command/command.h | 40 ++++- src/server/compiler.cpp | 50 ++---- src/server/master_server.cpp | 15 +- src/syntax/dependency_graph.cpp | 5 +- tests/unit/command/command_tests.cpp | 126 +++++++------- tests/unit/compile/compilation_tests.cpp | 4 +- .../compile_graph_integration_tests.cpp | 16 +- tests/unit/test/tester.cpp | 4 +- 9 files changed, 239 insertions(+), 178 deletions(-) diff --git a/src/command/command.cpp b/src/command/command.cpp index a7197f57..ab48141d 100644 --- a/src/command/command.cpp +++ b/src/command/command.cpp @@ -23,6 +23,44 @@ namespace ranges = std::ranges; } // namespace +std::vector CompileCommand::to_argv() const { + std::vector argv; + argv.reserve(resolved.flags.size() + 4); + + if(resolved.is_cc1 && source_file) { + // cc1 mode requires TWO file-related arguments (both are needed): + // 1. -main-file-name — used by clang for diagnostics/debug info + // 2. at the end — the actual input file path + // These are NOT duplicates: (1) is just the basename, (2) is the full path. + for(std::size_t i = 0; i < resolved.flags.size(); ++i) { + argv.push_back(resolved.flags[i]); + if(resolved.flags[i] == llvm::StringRef("-cc1")) { + argv.push_back("-main-file-name"); + // path::filename returns a suffix of source_file (a pointer into + // the same buffer), so .data() is null-terminated because source_file is. + argv.push_back(path::filename(source_file).data()); + } + } + } else { + argv.insert(argv.end(), resolved.flags.begin(), resolved.flags.end()); + } + + if(source_file) { + argv.push_back(source_file); + } + return argv; +} + +std::vector CompileCommand::to_string_argv() const { + auto argv = to_argv(); + std::vector result; + result.reserve(argv.size()); + for(auto* arg: argv) { + result.emplace_back(arg); + } + return result; +} + CompilationDatabase::CompilationDatabase() = default; CompilationDatabase::~CompilationDatabase() = default; @@ -329,8 +367,8 @@ std::size_t CompilationDatabase::load(llvm::StringRef path) { return entries.size(); } -llvm::SmallVector CompilationDatabase::lookup(llvm::StringRef file, - const CommandOptions& options) { +llvm::SmallVector CompilationDatabase::lookup(llvm::StringRef file, + const CommandOptions& options) { auto path_id = paths.intern(file); auto matched = find_entries(path_id); @@ -338,17 +376,18 @@ llvm::SmallVector CompilationDatabase::lookup(llvm::StringRe render_arg_to([&](llvm::StringRef s) { out.push_back(strings.save(s).data()); }, arg); }; - /// Build one CompilationContext from a single CompilationInfo. - auto build_context = [&](object_ptr info) -> CompilationContext { + /// Build one CompileCommand from a single CompilationInfo. + auto build_command = [&](object_ptr info) -> CompileCommand { llvm::StringRef directory = info->directory; - std::vector arguments; + std::vector flags; + bool is_cc1 = false; auto append_arg = [&](llvm::StringRef s) { - arguments.emplace_back(strings.save(s).data()); + flags.emplace_back(strings.save(s).data()); }; auto append_args = [&](llvm::ArrayRef args) { - arguments.insert(arguments.end(), args.begin(), args.end()); + flags.insert(flags.end(), args.begin(), args.end()); }; if(options.query_toolchain) { @@ -361,23 +400,20 @@ llvm::SmallVector CompilationDatabase::lookup(llvm::StringRe append_args(info->canonical->arguments); append_args(info->patch); } else { - arguments.assign(cached.begin(), cached.end()); - // TODO: add an assertion that the last arg is the temp source - // file (e.g., contains "query-toolchain") to guard against - // future changes in clang cc1 argument ordering. - arguments.pop_back(); // remove temp source file + flags.assign(cached.begin(), cached.end()); + flags.pop_back(); // remove temp source file // Replace resource dir if needed. if(!resource_dir().empty()) { llvm::StringRef old_resource_dir; - for(std::size_t i = 0; i + 1 < arguments.size(); ++i) { - if(arguments[i] == llvm::StringRef("-resource-dir")) { - old_resource_dir = arguments[i + 1]; + for(std::size_t i = 0; i + 1 < flags.size(); ++i) { + if(flags[i] == llvm::StringRef("-resource-dir")) { + old_resource_dir = flags[i + 1]; break; } } if(!old_resource_dir.empty() && old_resource_dir != resource_dir()) { - for(auto& arg: arguments) { + for(auto& arg: flags) { llvm::StringRef s(arg); if(s.starts_with(old_resource_dir)) { auto replaced = @@ -390,39 +426,42 @@ llvm::SmallVector CompilationDatabase::lookup(llvm::StringRe append_args(info->patch); - // Fix -main-file-name to match the actual file. - bool next_main_file = false; - for(auto& arg: arguments) { - if(arg == llvm::StringRef("-main-file-name")) { - next_main_file = true; + // Strip -main-file-name and its value from flags (to_argv() will + // re-inject it with the correct basename when is_cc1 is set). + std::vector cleaned; + cleaned.reserve(flags.size()); + for(std::size_t i = 0; i < flags.size(); ++i) { + if(flags[i] == llvm::StringRef("-main-file-name") && i + 1 < flags.size()) { + ++i; // skip the value continue; } - if(next_main_file) { - arg = strings.save(path::filename(file)).data(); - next_main_file = false; - } + cleaned.push_back(flags[i]); } - } + flags = std::move(cleaned); - // Inject our resource dir if not already present. - if(!resource_dir().empty()) { - bool has_resource_dir = false; - for(auto& arg: arguments) { - if(arg == llvm::StringRef("-resource-dir")) { - has_resource_dir = true; - break; - } - } - if(!has_resource_dir) { - append_arg("-resource-dir"); - append_arg(resource_dir()); - } + // Detect cc1 mode (search rather than assuming index). + is_cc1 = ranges::contains(flags, llvm::StringRef("-cc1")); } } else { append_args(info->canonical->arguments); append_args(info->patch); } + // Inject our resource dir if not already present. + if(options.inject_resource_dir && !resource_dir().empty()) { + bool has_resource_dir = false; + for(auto& arg: flags) { + if(arg == llvm::StringRef("-resource-dir")) { + has_resource_dir = true; + break; + } + } + if(!has_resource_dir) { + append_arg("-resource-dir"); + append_arg(resource_dir()); + } + } + // Apply remove filter. if(!options.remove.empty()) { using Arg = std::unique_ptr; @@ -440,12 +479,12 @@ llvm::SmallVector CompilationDatabase::lookup(llvm::StringRe }; std::ranges::sort(remove_args, {}, get_id); - auto saved_args = std::move(arguments); - arguments.clear(); - arguments.push_back(saved_args.front()); + auto saved_flags = std::move(flags); + flags.clear(); + flags.push_back(saved_flags.front()); parser->parse( - llvm::ArrayRef(saved_args).drop_front(), + llvm::ArrayRef(saved_flags).drop_front(), [&](Arg arg) { auto id = arg->getOption().getID(); auto range = std::ranges::equal_range(remove_args, id, {}, get_id); @@ -461,7 +500,7 @@ llvm::SmallVector CompilationDatabase::lookup(llvm::StringRe return; } } - render_arg(arguments, *arg); + render_arg(flags, *arg); }, [](int, int) {}); } @@ -470,26 +509,34 @@ llvm::SmallVector CompilationDatabase::lookup(llvm::StringRe append_arg(arg); } - arguments.emplace_back(paths.resolve(path_id).data()); - return CompilationContext(directory, std::move(arguments)); + return CompileCommand{ + ResolvedFlags{directory, std::move(flags), is_cc1}, + paths.resolve(path_id).data() + }; }; - llvm::SmallVector results; + llvm::SmallVector results; if(!matched.empty()) { for(auto& entry: matched) { - results.push_back(build_context(entry.info)); + results.push_back(build_command(entry.info)); } } else { // No matching entry — synthesize a default command. - std::vector arguments; + std::vector flags; if(file.ends_with(".cpp") || file.ends_with(".hpp") || file.ends_with(".cc")) { - arguments = {"clang++", "-std=c++20"}; + flags = {"clang++", "-std=c++20"}; } else { - arguments = {"clang"}; + flags = {"clang"}; } - arguments.emplace_back(paths.resolve(path_id).data()); - results.push_back(CompilationContext({}, std::move(arguments))); + if(options.inject_resource_dir && !resource_dir().empty()) { + flags.push_back(strings.save("-resource-dir").data()); + flags.push_back(strings.save(resource_dir()).data()); + } + results.push_back(CompileCommand{ + ResolvedFlags{{}, std::move(flags), false}, + paths.resolve(path_id).data() + }); } return results; @@ -513,8 +560,8 @@ SearchConfig CompilationDatabase::lookup_search_config(llvm::StringRef file, } auto results = lookup(file, options); - auto& ctx = results.front(); - auto config = extract_search_config(ctx.arguments, ctx.directory); + auto& cmd = results.front(); + auto config = extract_search_config(cmd.to_argv(), cmd.resolved.directory); if(cacheable) { auto key = ConfigCacheKey{matched.front().info.ptr, options_bits(options)}; diff --git a/src/command/command.h b/src/command/command.h index e51335ab..7e23ecfa 100644 --- a/src/command/command.h +++ b/src/command/command.h @@ -29,6 +29,11 @@ struct CommandOptions { /// Set true in unittests to avoid cluttering test output. bool suppress_logging = false; + /// Inject our resource dir into the flags if not already present. + /// Enabled by default so clang tools always use matching builtin headers. + /// Disable in unit tests that assert exact argument counts. + bool inject_resource_dir = true; + /// Extra arguments to remove from the original command line. llvm::ArrayRef remove; @@ -36,12 +41,35 @@ struct CommandOptions { llvm::ArrayRef append; }; -struct CompilationContext { +/// File-independent compilation flags (shareable, suitable as cache key input). +/// Does NOT contain source file path or -main-file-name. +struct ResolvedFlags { /// The working directory of compilation. llvm::StringRef directory; - /// The compilation arguments. - std::vector arguments; + /// All flags excluding source file path and -main-file-name. + std::vector flags; + + /// Whether flags come from toolchain query (cc1 mode). + /// When true, flags are cc1 frontend args (resolved clang binary + "-cc1" + ...), + /// NOT the original driver command. to_argv() scans for "-cc1" in flags and + /// inserts -main-file-name immediately after it. + bool is_cc1 = false; +}; + +/// Compilation command = resolved flags + source file identity. +struct CompileCommand { + ResolvedFlags resolved; + + /// Interned, pointer-stable. Must be null-terminated (required by to_argv() + /// and path::filename().data() which relies on the suffix being null-terminated). + const char* source_file = nullptr; + + /// Produce full argv: flags + [-main-file-name if cc1] + source_file. + std::vector to_argv() const; + + /// Convenience: to_argv() converted to vector. + std::vector to_string_argv() const; }; /// Shared compiler identity — driver + all semantics-affecting flags. @@ -174,10 +202,10 @@ public: /// but toolchain cache survives. Returns the number of entries loaded. std::size_t load(llvm::StringRef path); - /// Lookup the compilation contexts for a file. A file may have multiple + /// Lookup the compile commands for a file. A file may have multiple /// compilation commands (e.g. different build configurations); all are returned. - llvm::SmallVector lookup(llvm::StringRef file, - const CommandOptions& options = {}); + llvm::SmallVector lookup(llvm::StringRef file, + const CommandOptions& options = {}); /// Combined lookup + extract_search_config with internal caching. SearchConfig lookup_search_config(llvm::StringRef file, const CommandOptions& options = {}); diff --git a/src/server/compiler.cpp b/src/server/compiler.cpp index afdd7207..d830d2f4 100644 --- a/src/server/compiler.cpp +++ b/src/server/compiler.cpp @@ -52,8 +52,8 @@ void Compiler::init_compile_graph() { if(results.empty()) return {}; - auto& ctx = results[0]; - auto scan_result = scan_precise(ctx.arguments, ctx.directory); + auto& cmd = results[0]; + auto scan_result = scan_precise(cmd.to_argv(), cmd.resolved.directory); llvm::SmallVector deps; for(auto& mod_name: scan_result.modules) { @@ -158,12 +158,9 @@ bool Compiler::fill_compile_args(llvm::StringRef path, // 2. Normal CDB lookup for the file itself. auto results = workspace.cdb.lookup(path, {.query_toolchain = true}); if(!results.empty()) { - auto& ctx = results.front(); - directory = ctx.directory.str(); - arguments.clear(); - for(auto* arg: ctx.arguments) { - arguments.emplace_back(arg); - } + auto& cmd = results.front(); + directory = cmd.resolved.directory.str(); + arguments = cmd.to_string_argv(); return true; } @@ -214,33 +211,20 @@ bool Compiler::fill_header_context_args(llvm::StringRef path, return false; } - auto& host_ctx = host_results.front(); - directory = host_ctx.directory.str(); - arguments.clear(); + auto& host_cmd = host_results.front(); + directory = host_cmd.resolved.directory.str(); - // Copy host arguments, replacing the host source file path with the header. - bool replaced = false; - for(auto& arg: host_ctx.arguments) { - if(llvm::StringRef(arg) == host_path) { - arguments.emplace_back(path); - replaced = true; - } else { - arguments.emplace_back(arg); - } - } - if(!replaced) { - LOG_WARN("fill_header_context_args: host path {} not found in arguments, appending header", - host_path); - arguments.emplace_back(path); - } + // Replace source_file and inject -include preamble into flags directly. + CompileCommand header_cmd = host_cmd; + header_cmd.source_file = workspace.path_pool.resolve(path_id).data(); - // Inject preamble: for cc1 args insert after "-cc1", otherwise after driver. - std::size_t inject_pos = 1; - if(arguments.size() >= 2 && arguments[1] == "-cc1") { - inject_pos = 2; - } - arguments.insert(arguments.begin() + inject_pos, ctx_ptr->preamble_path); - arguments.insert(arguments.begin() + inject_pos, "-include"); + // Inject -include into flags: after "-cc1" for cc1, after driver otherwise. + std::size_t inject_pos = header_cmd.resolved.is_cc1 ? 2 : 1; + header_cmd.resolved.flags.insert(header_cmd.resolved.flags.begin() + inject_pos, + ctx_ptr->preamble_path.c_str()); + header_cmd.resolved.flags.insert(header_cmd.resolved.flags.begin() + inject_pos, "-include"); + + arguments = header_cmd.to_string_argv(); LOG_INFO("fill_compile_args: header context for {} (host={}, preamble={})", path, diff --git a/src/server/master_server.cpp b/src/server/master_server.cpp index 6a9302e0..6bface11 100644 --- a/src/server/master_server.cpp +++ b/src/server/master_server.cpp @@ -735,17 +735,18 @@ void MasterServer::register_handlers() { if(hosts.empty()) { auto entries = workspace.cdb.lookup(path, {.suppress_logging = true}); for(std::size_t i = 0; i < entries.size(); ++i) { - auto& entry = entries[i]; + auto& cmd = entries[i]; + auto argv = cmd.to_argv(); std::string desc; - for(std::size_t j = 0; j < entry.arguments.size(); ++j) { - llvm::StringRef a(entry.arguments[j]); + for(std::size_t j = 0; j < argv.size(); ++j) { + llvm::StringRef a(argv[j]); if(a.starts_with("-D") || a.starts_with("-O") || a.starts_with("-std=") || a.starts_with("-g")) { if(!desc.empty()) desc += ' '; - desc += entry.arguments[j]; - if((a == "-D" || a == "-O") && j + 1 < entry.arguments.size()) { - desc += entry.arguments[++j]; + desc += argv[j]; + if((a == "-D" || a == "-O") && j + 1 < argv.size()) { + desc += argv[++j]; } } } @@ -757,7 +758,7 @@ void MasterServer::register_handlers() { continue; ext::ContextItem item; item.label = desc; - item.description = entry.directory.str(); + item.description = cmd.resolved.directory.str(); item.uri = uri_opt->str(); all_items.push_back(std::move(item)); } diff --git a/src/syntax/dependency_graph.cpp b/src/syntax/dependency_graph.cpp index 47fbee66..6b7edc33 100644 --- a/src/syntax/dependency_graph.cpp +++ b/src/syntax/dependency_graph.cpp @@ -623,8 +623,9 @@ et::task<> scan_impl(CompilationDatabase& cdb, auto contexts = cdb.lookup(file_path, {.query_toolchain = true, .suppress_logging = true}); if(!contexts.empty()) { - auto& ctx = contexts[0]; - auto fallback = scan_module_decl(ctx.arguments, ctx.directory, /*content=*/{}); + auto& cmd = contexts[0]; + auto fallback = + scan_module_decl(cmd.to_argv(), cmd.resolved.directory, /*content=*/{}); if(!fallback.module_name.empty()) { scan_result.scan_result.module_name = std::move(fallback.module_name); scan_result.scan_result.is_interface_unit = fallback.is_interface_unit; diff --git a/tests/unit/command/command_tests.cpp b/tests/unit/command/command_tests.cpp index e712a8a5..4750f74d 100644 --- a/tests/unit/command/command_tests.cpp +++ b/tests/unit/command/command_tests.cpp @@ -14,6 +14,7 @@ using namespace std::literals; CommandOptions quiet_options() { CommandOptions options; options.suppress_logging = true; + options.inject_resource_dir = false; return options; } @@ -27,7 +28,7 @@ void EXPECT_STRIP(llvm::StringRef argv, llvm::StringRef result) { CompilationDatabase database; llvm::StringRef file = "main.cpp"; database.add_command("fake/", file, argv); - ASSERT_EQ(result, print_argv(database.lookup(file, quiet_options()).front().arguments)); + ASSERT_EQ(result, print_argv(database.lookup(file, quiet_options()).front().to_argv())); }; TEST_CASE(DefaultFilters) { @@ -58,8 +59,8 @@ TEST_CASE(Reuse) { database.add_command("fake", "test2.cpp", "clang++ -std=c++23 test2.cpp"sv); auto options = quiet_options(); - auto command1 = database.lookup("test.cpp", options).front().arguments; - auto command2 = database.lookup("test2.cpp", options).front().arguments; + auto command1 = database.lookup("test.cpp", options).front().to_argv(); + auto command2 = database.lookup("test2.cpp", options).front().to_argv(); ASSERT_EQ(command1.size(), 3U); ASSERT_EQ(command2.size(), 3U); @@ -93,70 +94,71 @@ TEST_CASE(RemoveAppend) { remove = {"-DA"}; options.remove = remove; - auto result = database.lookup("main.cpp", options).front().arguments; + auto result = database.lookup("main.cpp", options).front().to_argv(); ASSERT_EQ(print_argv(result), "clang++ -D B=0 main.cpp"); remove = {"-D", "A"}; options.remove = remove; - result = database.lookup("main.cpp", options).front().arguments; + result = database.lookup("main.cpp", options).front().to_argv(); ASSERT_EQ(print_argv(result), "clang++ -D B=0 main.cpp"); remove = {"-DA", "-D", "B=0"}; options.remove = remove; - result = database.lookup("main.cpp", options).front().arguments; + result = database.lookup("main.cpp", options).front().to_argv(); ASSERT_EQ(print_argv(result), "clang++ main.cpp"); remove = {"-D*"}; options.remove = remove; - result = database.lookup("main.cpp", options).front().arguments; + result = database.lookup("main.cpp", options).front().to_argv(); ASSERT_EQ(print_argv(result), "clang++ main.cpp"); remove = {"-D", "*"}; options.remove = remove; - result = database.lookup("main.cpp", options).front().arguments; + result = database.lookup("main.cpp", options).front().to_argv(); ASSERT_EQ(print_argv(result), "clang++ main.cpp"); append = {"-D", "C"}; options.append = append; - result = database.lookup("main.cpp", options).front().arguments; + result = database.lookup("main.cpp", options).front().to_argv(); ASSERT_EQ(print_argv(result), "clang++ -D C main.cpp"); }; TEST_CASE(DefaultFallback) { /// Lookup for a file not in the CDB should synthesize a default command. CompilationDatabase database; + auto options = quiet_options(); /// C++ files get "clang++ -std=c++20 ". - auto cpp_results = database.lookup("unknown.cpp"); + auto cpp_results = database.lookup("unknown.cpp", options); ASSERT_EQ(cpp_results.size(), 1U); - auto& cpp_ctx = cpp_results.front(); - ASSERT_EQ(cpp_ctx.arguments.size(), 3U); - ASSERT_EQ(cpp_ctx.arguments[0], "clang++"sv); - ASSERT_EQ(cpp_ctx.arguments[1], "-std=c++20"sv); - ASSERT_EQ(cpp_ctx.arguments[2], "unknown.cpp"sv); + auto cpp_argv = cpp_results.front().to_argv(); + ASSERT_EQ(cpp_argv.size(), 3U); + ASSERT_EQ(cpp_argv[0], "clang++"sv); + ASSERT_EQ(cpp_argv[1], "-std=c++20"sv); + ASSERT_EQ(cpp_argv[2], "unknown.cpp"sv); /// .hpp files also get C++ default. - auto hpp_results = database.lookup("header.hpp"); - ASSERT_EQ(hpp_results.front().arguments.size(), 3U); - ASSERT_EQ(hpp_results.front().arguments[0], "clang++"sv); + auto hpp_results = database.lookup("header.hpp", options); + ASSERT_EQ(hpp_results.front().to_argv().size(), 3U); + ASSERT_EQ(hpp_results.front().to_argv()[0], "clang++"sv); /// .cc files also get C++ default. - auto cc_results = database.lookup("file.cc"); - ASSERT_EQ(cc_results.front().arguments.size(), 3U); - ASSERT_EQ(cc_results.front().arguments[0], "clang++"sv); + auto cc_results = database.lookup("file.cc", options); + ASSERT_EQ(cc_results.front().to_argv().size(), 3U); + ASSERT_EQ(cc_results.front().to_argv()[0], "clang++"sv); /// C files get "clang ". - auto c_results = database.lookup("unknown.c"); + auto c_results = database.lookup("unknown.c", options); ASSERT_EQ(c_results.size(), 1U); - auto& c_ctx = c_results.front(); - ASSERT_EQ(c_ctx.arguments.size(), 2U); - ASSERT_EQ(c_ctx.arguments[0], "clang"sv); - ASSERT_EQ(c_ctx.arguments[1], "unknown.c"sv); + auto c_argv = c_results.front().to_argv(); + ASSERT_EQ(c_argv.size(), 2U); + ASSERT_EQ(c_argv[0], "clang"sv); + ASSERT_EQ(c_argv[1], "unknown.c"sv); /// Other extensions also get plain clang. - auto h_results = database.lookup("foo.h"); - ASSERT_EQ(h_results.front().arguments.size(), 2U); - ASSERT_EQ(h_results.front().arguments[0], "clang"sv); + auto h_results = database.lookup("foo.h", options); + ASSERT_EQ(h_results.front().to_argv().size(), 2U); + ASSERT_EQ(h_results.front().to_argv()[0], "clang"sv); }; TEST_CASE(MultiCommand) { @@ -173,8 +175,8 @@ TEST_CASE(MultiCommand) { /// Both commands are present (order depends on insert position). bool has_17 = false, has_20 = false; - for(auto& ctx: results) { - auto argv = print_argv(ctx.arguments); + for(auto& cmd: results) { + auto argv = print_argv(cmd.to_argv()); if(llvm::StringRef(argv).contains("-std=c++17")) has_17 = true; if(llvm::StringRef(argv).contains("-std=c++20")) @@ -196,7 +198,7 @@ TEST_CASE(CodegenFilter) { "main.cpp", "clang++ -std=c++20 -fPIC -fno-omit-frame-pointer -fstack-protector-strong " "-fdata-sections -ffunction-sections -flto -fcolor-diagnostics -g main.cpp"sv); - auto result = database.lookup("main.cpp", quiet_options()).front().arguments; + auto result = database.lookup("main.cpp", quiet_options()).front().to_argv(); auto argv = print_argv(result); /// -std=c++20 must survive (semantic). @@ -220,7 +222,7 @@ TEST_CASE(DependencyScanFilter) { "main.cpp", "clang++ -std=c++20 -MD -MF main.d -MT main.o main.cpp"sv); - auto result = database.lookup("main.cpp", quiet_options()).front().arguments; + auto result = database.lookup("main.cpp", quiet_options()).front().to_argv(); auto argv = print_argv(result); EXPECT_CONTAINS(argv, "-std=c++20"); @@ -247,8 +249,8 @@ TEST_CASE(UserContentClassification) { auto options = quiet_options(); - auto a_argv = print_argv(database.lookup("a.cpp", options).front().arguments); - auto b_argv = print_argv(database.lookup("b.cpp", options).front().arguments); + auto a_argv = print_argv(database.lookup("a.cpp", options).front().to_argv()); + auto b_argv = print_argv(database.lookup("b.cpp", options).front().to_argv()); /// Both must contain canonical flags. EXPECT_CONTAINS(a_argv, "-std=c++20"); @@ -277,7 +279,7 @@ TEST_CASE(IncludePathAbsolutize) { "main.cpp", "clang++ -Iinclude -isystem sys/inc -iquote ../src main.cpp"sv); - auto result = database.lookup("main.cpp", quiet_options()).front().arguments; + auto result = database.lookup("main.cpp", quiet_options()).front().to_argv(); /// Check each argument individually with separator normalization /// (print_argv escapes backslashes, breaking convert_to_slash on Windows). @@ -299,7 +301,7 @@ TEST_CASE(IncludePathAbsolutize) { CompilationDatabase database2; database2.add_command("/project/build", "main.cpp", "clang++ -I/usr/include main.cpp"sv); - auto result2 = database2.lookup("main.cpp", quiet_options()).front().arguments; + auto result2 = database2.lookup("main.cpp", quiet_options()).front().to_argv(); EXPECT_TRUE(has_path(result2, "/usr/include")); }; @@ -337,7 +339,7 @@ TEST_CASE(ResolvePath) { database.add_command("fake", "test/main.cpp", "clang++ test/main.cpp"sv); /// After add_command, lookup should work and resolve_path via the file in arguments. - auto result = database.lookup("test/main.cpp", quiet_options()).front().arguments; + auto result = database.lookup("test/main.cpp", quiet_options()).front().to_argv(); /// The last argument is the file, resolved from PathPool. ASSERT_EQ(result.back(), "test/main.cpp"sv); }; @@ -350,14 +352,14 @@ TEST_CASE(MoveSemantics) { CompilationDatabase db2 = std::move(db1); auto options = quiet_options(); - auto result = db2.lookup("main.cpp", options).front().arguments; + auto result = db2.lookup("main.cpp", options).front().to_argv(); ASSERT_EQ(result.size(), 3U); ASSERT_EQ(result[1], "-std=c++23"sv); /// Move assign. CompilationDatabase db3; db3 = std::move(db2); - result = db3.lookup("main.cpp", options).front().arguments; + result = db3.lookup("main.cpp", options).front().to_argv(); ASSERT_EQ(result.size(), 3U); ASSERT_EQ(result[1], "-std=c++23"sv); }; @@ -398,11 +400,11 @@ TEST_CASE(LoadMixedFormats) { auto a = database.lookup(path::join("/build", "a.cpp"), options); ASSERT_EQ(a.size(), 1U); - EXPECT_CONTAINS(print_argv(a.front().arguments), "-std=c++20"); + EXPECT_CONTAINS(print_argv(a.front().to_argv()), "-std=c++20"); auto b = database.lookup(path::join("/build", "b.cpp"), options); ASSERT_EQ(b.size(), 1U); - EXPECT_CONTAINS(print_argv(b.front().arguments), "-std=c++23"); + EXPECT_CONTAINS(print_argv(b.front().to_argv()), "-std=c++23"); }; TEST_CASE(LoadErrorRecovery) { @@ -428,11 +430,11 @@ TEST_CASE(LoadErrorRecovery) { auto good = database.lookup(path::join("/build", "good.cpp"), options); ASSERT_EQ(good.size(), 1U); - EXPECT_CONTAINS(print_argv(good.front().arguments), "-std=c++20"); + EXPECT_CONTAINS(print_argv(good.front().to_argv()), "-std=c++20"); auto also = database.lookup(path::join("/build", "also_good.cpp"), options); ASSERT_EQ(also.size(), 1U); - EXPECT_CONTAINS(print_argv(also.front().arguments), "-Wall"); + EXPECT_CONTAINS(print_argv(also.front().to_argv()), "-Wall"); }; TEST_CASE(LoadEmptyCommand) { @@ -450,7 +452,7 @@ TEST_CASE(LoadEmptyCommand) { auto ok = database.lookup(path::join("/build", "ok.cpp"), quiet_options()); ASSERT_EQ(ok.size(), 1U); - EXPECT_CONTAINS(print_argv(ok.front().arguments), "-std=c++20"); + EXPECT_CONTAINS(print_argv(ok.front().to_argv()), "-std=c++20"); }; TEST_CASE(LoadReload) { @@ -469,7 +471,7 @@ TEST_CASE(LoadReload) { auto a = database.lookup(file_a, options); ASSERT_EQ(a.size(), 1U); - EXPECT_CONTAINS(print_argv(a.front().arguments), "-std=c++17"); + EXPECT_CONTAINS(print_argv(a.front().to_argv()), "-std=c++17"); /// Reload with different content. auto count = load_json(database, R"([ @@ -482,12 +484,12 @@ TEST_CASE(LoadReload) { /// Old entry gone (falls back to default). auto a2 = database.lookup(file_a, options); ASSERT_EQ(a2.size(), 1U); - EXPECT_NOT_CONTAINS(print_argv(a2.front().arguments), "-std=c++17"); + EXPECT_NOT_CONTAINS(print_argv(a2.front().to_argv()), "-std=c++17"); /// New entry present. auto b = database.lookup(file_b, options); ASSERT_EQ(b.size(), 1U); - EXPECT_CONTAINS(print_argv(b.front().arguments), "-std=c++23"); + EXPECT_CONTAINS(print_argv(b.front().to_argv()), "-std=c++23"); }; TEST_CASE(LoadCommandQuoting) { @@ -502,7 +504,7 @@ TEST_CASE(LoadCommandQuoting) { auto result = database.lookup(path::join("/build", "main.cpp"), quiet_options()); ASSERT_EQ(result.size(), 1U); - auto argv = print_argv(result.front().arguments); + auto argv = print_argv(result.front().to_argv()); /// The define and include path should be present after shell tokenization. EXPECT_CONTAINS(argv, "hello world"); @@ -526,17 +528,17 @@ TEST_CASE(LoadRelativePath) { /// Lookup by the resolved absolute path (use path::join for correct separator). auto results = database.lookup(path::join("/project/build", "src/main.cpp"), options); ASSERT_EQ(results.size(), 1U); - EXPECT_CONTAINS(print_argv(results.front().arguments), "-std=c++20"); + EXPECT_CONTAINS(print_argv(results.front().to_argv()), "-std=c++20"); auto results2 = database.lookup(path::join("/other/build", "src/main.cpp"), options); ASSERT_EQ(results2.size(), 1U); - EXPECT_CONTAINS(print_argv(results2.front().arguments), "-std=c++17"); + EXPECT_CONTAINS(print_argv(results2.front().to_argv()), "-std=c++17"); /// Relative path lookup should not match (different path_id). auto results3 = database.lookup("src/main.cpp", options); ASSERT_EQ(results3.size(), 1U); /// Falls back to default command since no match. - EXPECT_CONTAINS(print_argv(results3.front().arguments), "clang"); + EXPECT_CONTAINS(print_argv(results3.front().to_argv()), "clang"); }; TEST_CASE(Module) { @@ -544,19 +546,21 @@ TEST_CASE(Module) { } TEST_CASE(ResourceDir) { - // When query_toolchain is enabled, resource dir is injected automatically. CompilationDatabase database; database.add_command("/fake", "main.cpp", "clang++ -std=c++23 test.cpp"sv); - // Without query_toolchain, no resource dir injection. - auto args_no_tc = database.lookup("main.cpp").front().arguments; - ASSERT_EQ(args_no_tc.size(), 3U); - ASSERT_EQ(args_no_tc[0], "clang++"sv); - ASSERT_EQ(args_no_tc[1], "-std=c++23"sv); - ASSERT_EQ(args_no_tc[2], "main.cpp"sv); + // With inject_resource_dir disabled, no resource dir in result. + auto args_no_rd = + database.lookup("main.cpp", {.suppress_logging = true, .inject_resource_dir = false}) + .front() + .to_argv(); + ASSERT_EQ(args_no_rd.size(), 3U); + ASSERT_EQ(args_no_rd[0], "clang++"sv); + ASSERT_EQ(args_no_rd[1], "-std=c++23"sv); + ASSERT_EQ(args_no_rd[2], "main.cpp"sv); - // With query_toolchain, resource dir is present in the result. - auto args_tc = database.lookup("main.cpp", {.query_toolchain = true}).front().arguments; + // With inject_resource_dir enabled (default), resource dir is present. + auto args_tc = database.lookup("main.cpp").front().to_argv(); bool has_resource_dir = false; for(size_t i = 0; i + 1 < args_tc.size(); ++i) { if(args_tc[i] == "-resource-dir"sv) { diff --git a/tests/unit/compile/compilation_tests.cpp b/tests/unit/compile/compilation_tests.cpp index 79954b54..a62cc932 100644 --- a/tests/unit/compile/compilation_tests.cpp +++ b/tests/unit/compile/compilation_tests.cpp @@ -203,7 +203,7 @@ export int a_value() { return b_value() + 1; } CompilationParams params_b; params_b.kind = CompilationKind::ModuleInterface; - params_b.arguments = cdb.lookup(tmp.path("mod_b.cppm"), cmd_opts).front().arguments; + params_b.arguments = cdb.lookup(tmp.path("mod_b.cppm"), cmd_opts).front().to_argv(); auto pcm_b_path = fs::createTemporaryFile("mod_b", "pcm"); ASSERT_TRUE(pcm_b_path.operator bool()); @@ -221,7 +221,7 @@ export int a_value() { return b_value() + 1; } CompilationParams params_a; params_a.kind = CompilationKind::ModuleInterface; - params_a.arguments = cdb.lookup(tmp.path("mod_a.cppm"), cmd_opts).front().arguments; + params_a.arguments = cdb.lookup(tmp.path("mod_a.cppm"), cmd_opts).front().to_argv(); params_a.pcms.try_emplace("mod_b", info_b.path); auto pcm_a_path = fs::createTemporaryFile("mod_a", "pcm"); diff --git a/tests/unit/server/compile_graph_integration_tests.cpp b/tests/unit/server/compile_graph_integration_tests.cpp index 801d7ca2..1a9cc432 100644 --- a/tests/unit/server/compile_graph_integration_tests.cpp +++ b/tests/unit/server/compile_graph_integration_tests.cpp @@ -29,10 +29,8 @@ CompileGraph::dispatch_fn make_dispatch(CompilationDatabase& cdb, CompilationParams cp; cp.kind = CompilationKind::ModuleInterface; - cp.directory = results[0].directory.str(); - for(auto* arg: results[0].arguments) { - cp.arguments.push_back(arg); - } + cp.directory = results[0].resolved.directory.str(); + cp.arguments = results[0].to_argv(); // Fill ALL available PCM paths (clang needs transitive deps too). for(auto& [pid, pcm_path]: pcm_paths) { @@ -72,7 +70,7 @@ CompileGraph::resolve_fn make_resolver(CompilationDatabase& cdb, return {}; } - auto scan_result = scan_precise(results[0].arguments, results[0].directory); + auto scan_result = scan_precise(results[0].to_argv(), results[0].resolved.directory); llvm::SmallVector deps; for(auto& mod_name: scan_result.modules) { @@ -1034,7 +1032,7 @@ TEST_CASE(ReResolveAfterUpdate) { if(results.empty()) { return {}; } - auto scan_result = scan_precise(results[0].arguments, results[0].directory); + auto scan_result = scan_precise(results[0].to_argv(), results[0].resolved.directory); llvm::SmallVector deps; for(auto& mod_name: scan_result.modules) { auto mod_ids = env.graph.lookup_module(mod_name); @@ -1150,10 +1148,8 @@ TEST_CASE(ModuleImplementationUnit) { CompilationParams cp; cp.kind = CompilationKind::Content; - cp.directory = results[0].directory.str(); - for(auto* arg: results[0].arguments) { - cp.arguments.push_back(arg); - } + cp.directory = results[0].resolved.directory.str(); + cp.arguments = results[0].to_argv(); // Pass the built PCM so clang can resolve `module Greeter;`. for(auto& [pid, pcm_path]: env.pcm_paths) { for(auto& [mod_name, mod_ids]: env.graph.modules()) { diff --git a/tests/unit/test/tester.cpp b/tests/unit/test/tester.cpp index 5c1e934a..1f49defc 100644 --- a/tests/unit/test/tester.cpp +++ b/tests/unit/test/tester.cpp @@ -162,7 +162,7 @@ void Tester::prepare_driver(llvm::StringRef standard) { options.suppress_logging = true; auto commands = database.lookup(src_path, options); assert(!commands.empty() && "lookup failed after add_command"); - params.arguments = commands.front().arguments; + params.arguments = commands.front().to_argv(); params.kind = CompilationKind::Content; @@ -214,7 +214,7 @@ bool Tester::compile_driver_with_pch(llvm::StringRef standard) { options.suppress_logging = true; auto commands = database.lookup(src_path, options); assert(!commands.empty() && "lookup failed after add_command"); - params.arguments = commands.front().arguments; + params.arguments = commands.front().to_argv(); auto pch_path = fs::createTemporaryFile("clice", "pch"); if(!pch_path) {