Chore: extract struct CommandOptions.

Guo-Shiyu · Guo-Shiyu · commit 422077370bbc · 2025-08-24T14:12:41.000+08:00
diff --git a/include/Compiler/Command.h b/include/Compiler/Command.h
@@ -10,6 +10,21 @@
 
 namespace clice {
 
+struct CommandOptions {
+    /// The file to get command for.
+    llvm::StringRef file;
+
+    /// Attach resource directory to the command.
+    bool resource_dir = false;
+
+    /// Query the compiler driver for additional information, such as system includes and target.
+    bool query_driver = false;
+
+    /// Suppress the warning log if failed to query driver info.
+    /// Set true in unittests to avoid cluttering test output.
+    bool suppress_log = false;
+};
+
 class CompilationDatabase {
 public:
     using Self = CompilationDatabase;
@@ -21,18 +36,6 @@ class CompilationDatabase {
         Delete,
     };
 
-    struct QueryDriverErrorKind : refl::Enum<QueryDriverErrorKind> {
-        enum Kind : std::uint8_t {
-            NotFoundInPATH,
-            FailToCreateTempFile,
-            InvokeDriverFail,
-            OutputFileNotReadable,
-            InvalidOutputFormat,
-        };
-
-        using Enum::Enum;
-    };
-
     struct CommandInfo {
         /// TODO: add sysroot or no stdinc command info.
         llvm::StringRef dictionary;
@@ -63,10 +66,20 @@ class CompilationDatabase {
     };
 
     struct QueryDriverError {
-        QueryDriverErrorKind kind;
-        std::string driver;
+        struct ErrorKind : refl::Enum<ErrorKind> {
+            enum Kind : std::uint8_t {
+                NotFoundInPATH,
+                FailToCreateTempFile,
+                InvokeDriverFail,
+                OutputFileNotReadable,
+                InvalidOutputFormat,
+            };
+
+            using Enum::Enum;
+        };
+
+        ErrorKind kind;
         std::string detail;
-        std::optional<std::string> output_file;
     };
 
     CompilationDatabase();
@@ -99,10 +112,7 @@ class CompilationDatabase {
     auto load_commands(this Self& self, llvm::StringRef json_content)
         -> std::expected<std::vector<UpdateInfo>, std::string>;
 
-    auto get_command(this Self& self,
-                     llvm::StringRef file,
-                     bool resource_dir = false,
-                     bool query_driver = false) -> LookupInfo;
+    auto get_command(this Self& self, CommandOptions options) -> LookupInfo;
 
 private:
     /// The memory pool to hold all cstring and command list.
@@ -159,13 +169,7 @@ struct std::formatter<clice::CompilationDatabase::QueryDriverError> :
 
     template <typename FormatContext>
     auto format(clice::CompilationDatabase::QueryDriverError& e, FormatContext& ctx) const {
-        return std::format_to(
-            ctx.out(),
-            "QueryDriverError{{ kind: {}, driver: {}, detail: {}, output_file: {} }}",
-            e.kind.name(),
-            e.driver,
-            e.detail,
-            e.output_file.value_or("<no file>"));
+        return std::format_to(ctx.out(), "{} {}", e.kind.name(), e.detail);
     }
 };
 
diff --git a/include/Test/Tester.h b/include/Test/Tester.h
@@ -36,7 +36,13 @@ struct Tester {
 
         database.update_command("fake", src_path, command);
         params.kind = CompilationUnit::Content;
-        params.arguments = database.get_command(src_path, true, true).arguments;
+
+        CommandOptions options;
+        options.file = src_path;
+        options.resource_dir = true;
+        options.query_driver = true;
+        options.suppress_log = true;
+        params.arguments = database.get_command(options).arguments;
 
         for(auto& [file, source]: sources.all_files) {
             if(file == src_path) {
@@ -67,7 +73,13 @@ struct Tester {
 
         database.update_command("fake", src_path, command);
         params.kind = CompilationUnit::Preamble;
-        params.arguments = database.get_command(src_path, true, true).arguments;
+
+        CommandOptions options;
+        options.file = src_path;
+        options.resource_dir = true;
+        options.query_driver = true;
+        options.suppress_log = true;
+        params.arguments = database.get_command(options).arguments;
 
         auto path = fs::createTemporaryFile("clice", "pch");
         if(!path) {
diff --git a/src/Compiler/Command.cpp b/src/Compiler/Command.cpp
@@ -108,25 +108,10 @@ llvm::SmallVector<llvm::StringRef, 4> driver_invocation_argv(llvm::StringRef dri
 }
 
 using QueryDriverError = CompilationDatabase::QueryDriverError;
-using ErrorKind = CompilationDatabase::QueryDriverErrorKind;
-
-auto unexpected(ErrorKind kind,
-                llvm::StringRef driver,
-                std::string&& message,
-                std::optional<std::string> output_file = std::nullopt)
-    -> std::expected<CompilationDatabase::DriverInfo, QueryDriverError> {
-    QueryDriverError err{kind,
-                         driver.str(),
-                         std::forward<std::string>(message),
-                         std::move(output_file)};
-    return std::unexpected(std::move(err));
-};
+using ErrorKind = CompilationDatabase::QueryDriverError::ErrorKind;
 
-template <typename T>
-auto unexpected(ErrorKind kind, llvm::StringRef driver, const llvm::ErrorOr<T>& error)
-    -> std::expected<CompilationDatabase::DriverInfo, QueryDriverError> {
-    QueryDriverError err{kind, driver.str(), std::format("{}", error.getError())};
-    return std::unexpected(std::move(err));
+auto unexpected(ErrorKind kind, std::string message) {
+    return std::unexpected<QueryDriverError>({kind, std::move(message)});
 };
 
 }  // namespace
@@ -139,7 +124,7 @@ auto CompilationDatabase::query_driver(this Self& self, llvm::StringRef driver)
         if(auto error = fs::real_path(driver, absolute_path)) {
             auto result = llvm::sys::findProgramByName(driver);
             if(!result) {
-                return unexpected(ErrorKind::NotFoundInPATH, driver, result);
+                return unexpected(ErrorKind::NotFoundInPATH, result.getError().message());
             }
             absolute_path = *result;
         }
@@ -156,7 +141,7 @@ auto CompilationDatabase::query_driver(this Self& self, llvm::StringRef driver)
 
     llvm::SmallString<128> output_path;
     if(auto error = llvm::sys::fs::createTemporaryFile("system-includes", "clice", output_path)) {
-        return unexpected(ErrorKind::FailToCreateTempFile, driver, error.message());
+        return unexpected(ErrorKind::FailToCreateTempFile, error.message());
     }
 
     // If we fail to get the driver infomation, keep the output file for user to debug.
@@ -184,12 +169,12 @@ auto CompilationDatabase::query_driver(this Self& self, llvm::StringRef driver)
                                           /*SecondsToWait=*/0,
                                           /*MemoryLimit=*/0,
                                           &message)) {
-        return unexpected(ErrorKind::InvokeDriverFail, driver, std::move(message));
+        return unexpected(ErrorKind::InvokeDriverFail, std::move(message));
     }
 
     auto file = llvm::MemoryBuffer::getFile(output_path);
     if(!file) {
-        return unexpected(ErrorKind::OutputFileNotReadable, driver, file.getError().message());
+        return unexpected(ErrorKind::OutputFileNotReadable, file.getError().message());
     }
 
     llvm::StringRef content = file.get()->getBuffer();
@@ -235,17 +220,11 @@ auto CompilationDatabase::query_driver(this Self& self, llvm::StringRef driver)
     }
 
     if(!found_start_marker) {
-        return unexpected(ErrorKind::InvalidOutputFormat,
-                          driver,
-                          "Start marker not found...",
-                          output_path.str().str());
+        return unexpected(ErrorKind::InvalidOutputFormat, "Start marker not found...");
     }
 
     if(in_includes_block) {
-        return unexpected(ErrorKind::InvalidOutputFormat,
-                          driver,
-                          "End marker not found...",
-                          output_path.str().str());
+        return unexpected(ErrorKind::InvalidOutputFormat, "End marker not found...");
     }
 
     // Get driver information success, remove temporary file.
@@ -468,45 +447,10 @@ auto CompilationDatabase::load_commands(this Self& self, llvm::StringRef json_co
     return infos;
 }
 
-namespace {
-
-/// Show warn and optional hint message for query driver error in log.
-void handle_query_driver_error(QueryDriverError error) {
-    log::warn("Failed to query driver info: {}", error);
-
-    llvm::SmallString<256> hint;
-    llvm::raw_svector_ostream os(hint);
-    if(error.kind == ErrorKind::NotFoundInPATH) {
-        os << "Make sure that " << error.driver
-           << " has been installed in your PATH, or update the compile_commands.json to use correct driver.";
-    } else if(error.kind == ErrorKind::InvokeDriverFail) {
-        os << "The failed invocation command is: " << error.driver;
-        for(llvm::StringRef arg: driver_invocation_argv(error.driver)) {
-            os << ' ' << arg;
-        }
-    } else if(error.kind == ErrorKind::InvalidOutputFormat) {
-        os << "Check the output format of output file: " << error.output_file.value_or("<unknown>")
-           << ", for GCC/Clang it should be like:\n\n"
-           << "Target: <target>\n"
-           << "#include <...> search starts here:\n"
-           << "<system includes>\n"
-           << "End of search list.";
-    }
-
-    if(!hint.empty()) {
-        log::warn("HINT: {}", hint);
-    }
-}
-
-}  // namespace
-
-auto CompilationDatabase::get_command(this Self& self,
-                                      llvm::StringRef file,
-                                      bool resource_dir,
-                                      bool query_driver) -> LookupInfo {
+auto CompilationDatabase::get_command(this Self& self, CommandOptions options) -> LookupInfo {
     LookupInfo info;
 
-    file = self.save_string(file);
+    llvm::StringRef file = self.save_string(options.file);
     auto it = self.command_infos.find(file.data());
     if(it != self.command_infos.end()) {
         info.dictionary = it->second.dictionary;
@@ -524,7 +468,7 @@ auto CompilationDatabase::get_command(this Self& self,
         info.arguments.emplace_back(self.save_string(argument).data());
     };
 
-    if(query_driver) {
+    if(options.query_driver) {
         llvm::StringRef driver = info.arguments[0];
         if(auto driver_info = self.query_driver(driver)) {
             append_argument("-nostdlibinc");
@@ -536,14 +480,12 @@ auto CompilationDatabase::get_command(this Self& self,
                 append_argument("-I");
                 append_argument(system_header);
             }
-        } else {
-            /// FIXME: Show log in  `handle_query_driver_error` will cause many unsed output in
-            /// unittest. We should find a way to suppress this output in unittest.
-            handle_query_driver_error(std::move(driver_info).error());
+        } else if(!options.suppress_log) {
+            log::warn("Failed to query driver:{}, error:{}", driver, driver_info.error());
         }
     }
 
-    if(resource_dir) {
+    if(options.resource_dir) {
         append_argument(std::format("-resource-dir={}", fs::resource_dir));
     }
 
diff --git a/src/Server/Document.cpp b/src/Server/Document.cpp
@@ -240,8 +240,13 @@ async::Task<bool> build_pch_task(CompilationDatabase::LookupInfo& info,
 }  // namespace
 
 async::Task<bool> Server::build_pch(std::string file, std::string content) {
+    CommandOptions options;
+    options.file = file;
+    options.resource_dir = true;
+    options.query_driver = true;
+    auto info = database.get_command(options);
+
     auto bound = compute_preamble_bound(content);
-    auto info = database.get_command(file, true, true);
     auto& open_file = opening_files.get_or_add(file);
 
     /// Check update ...
@@ -294,9 +299,14 @@ async::Task<> Server::build_ast(std::string path, std::string content) {
         log::fatal("Expected PCH built at this point");
     }
 
+    CommandOptions options;
+    options.file = path;
+    options.resource_dir = true;
+    options.query_driver = true;
+
     CompilationParams params;
     params.kind = CompilationUnit::Content;
-    params.arguments = database.get_command(path, true, true).arguments;
+    params.arguments = database.get_command(options).arguments;
     params.add_remapped_file(path, content);
     params.pch = {pch->path, pch->preamble.size()};
     file->diagnostics->clear();
diff --git a/src/Server/Feature.cpp b/src/Server/Feature.cpp
@@ -25,10 +25,14 @@ auto Server::on_completion(proto::CompletionParams params) -> Result {
     auto offset = to_offset(kind, content, params.position);
     auto& pch = opening_file->pch;
     {
+        CommandOptions options;
+        options.file = path;
+        options.resource_dir = true;
+
         /// Set compilation params ... .
         CompilationParams params;
         params.kind = CompilationUnit::Completion;
-        params.arguments = database.get_command(path, true).arguments;
+        params.arguments = database.get_command(options).arguments;
         params.add_remapped_file(path, content);
         params.pch = {pch->path, pch->preamble.size()};
         params.completion = {path, offset};
@@ -74,10 +78,14 @@ async::Task<json::Value> Server::on_signature_help(proto::SignatureHelpParams pa
     auto offset = to_offset(kind, content, params.position);
     auto& pch = opening_file->pch;
     {
+        CommandOptions options;
+        options.file = path;
+        options.resource_dir = true;
+
         /// Set compilation params ... .
         CompilationParams params;
         params.kind = CompilationUnit::Completion;
-        params.arguments = database.get_command(path, true).arguments;
+        params.arguments = database.get_command(options).arguments;
         params.add_remapped_file(path, content);
         params.pch = {pch->path, pch->preamble.size()};
         params.completion = {path, offset};
diff --git a/src/Server/Indexer.cpp b/src/Server/Indexer.cpp
@@ -46,9 +46,12 @@ async::Task<> Indexer::index(CompilationUnit& unit) {
 }
 
 async::Task<> Indexer::index(llvm::StringRef file) {
+    CommandOptions options;
+    options.file = file;
+
     CompilationParams params;
     params.kind = CompilationUnit::Indexing;
-    params.arguments = database.get_command(file).arguments;
+    params.arguments = database.get_command(options).arguments;
 
     auto AST = co_await async::submit([&] { return compile(params); });
 
diff --git a/tests/unit/Compiler/Command.cpp b/tests/unit/Compiler/Command.cpp
@@ -57,8 +57,13 @@ void parse_and_dump(llvm::StringRef command) {
 suite<"Command"> command = [] {
     auto expect_strip = [](llvm::StringRef argv, llvm::StringRef result) {
         CompilationDatabase database;
-        database.update_command("fake/", "main.cpp", argv);
-        expect(that % printArgv(database.get_command("main.cpp").arguments) == result);
+        llvm::StringRef file = "main.cpp";
+        database.update_command("fake/", file, argv);
+
+        CommandOptions options;
+        options.file = "main.cpp";
+        options.suppress_log = true;
+        expect(that % printArgv(database.get_command(options).arguments) == result);
     };
 
     test("GetOptionID") = [] {
@@ -140,8 +145,13 @@ suite<"Command"> command = [] {
         database.update_command("fake", "test.cpp", "clang++ -std=c++23 test.cpp"sv);
         database.update_command("fake", "test2.cpp", "clang++ -std=c++23 test2.cpp"sv);
 
-        auto command1 = database.get_command("test.cpp").arguments;
-        auto command2 = database.get_command("test2.cpp").arguments;
+        CommandOptions options;
+        options.suppress_log = true;
+
+        options.file = "test.cpp";
+        auto command1 = database.get_command(options).arguments;
+        options.file = "test2.cpp";
+        auto command2 = database.get_command(options).arguments;
         expect(that % command1.size() == 3);
         expect(that % command2.size() == 3);
 
@@ -161,7 +171,7 @@ suite<"Command"> command = [] {
     test("QueryDriver") = [] {
 #ifdef _GLIBCXX_RELEASE
         using namespace std::literals;
-        using ErrorKind = CompilationDatabase::QueryDriverErrorKind;
+        using ErrorKind = CompilationDatabase::QueryDriverError::ErrorKind;
 
         CompilationDatabase database;
         auto info = database.query_driver("g++");
