Fix: improve some case that user's environment do not have driver installed (#185)

Guo-Shiyu · web-flow · commit 260c268af75a · 2025-08-24T19:46:22.000+08:00
diff --git a/include/Compiler/Command.h b/include/Compiler/Command.h
@@ -1,14 +1,27 @@
 #pragma once
 
+#include "Support/Enum.h"
+#include "Support/Format.h"
 #include <expected>
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/ADT/ArrayRef.h"
-#include <deque>
 
 namespace clice {
 
+struct CommandOptions {
+    /// 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;
@@ -49,6 +62,23 @@ class CompilationDatabase {
         std::vector<const char*> arguments;
     };
 
+    struct QueryDriverError {
+        struct ErrorKind : refl::Enum<ErrorKind> {
+            enum Kind : std::uint8_t {
+                NotFoundInPATH,
+                FailToCreateTempFile,
+                InvokeDriverFail,
+                OutputFileNotReadable,
+                InvalidOutputFormat,
+            };
+
+            using Enum::Enum;
+        };
+
+        ErrorKind kind;
+        std::string detail;
+    };
+
     CompilationDatabase();
 
     auto save_string(this Self& self, llvm::StringRef string) -> llvm::StringRef;
@@ -61,7 +91,7 @@ class CompilationDatabase {
 
     /// Query the compiler driver and return its driver info.
     auto query_driver(this Self& self, llvm::StringRef driver)
-        -> std::expected<DriverInfo, std::string>;
+        -> std::expected<DriverInfo, QueryDriverError>;
 
     /// Update with arguments.
     auto update_command(this Self& self,
@@ -79,10 +109,8 @@ 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, llvm::StringRef file, CommandOptions options = {})
+        -> LookupInfo;
 
 private:
     /// The memory pool to hold all cstring and command list.
@@ -132,3 +160,14 @@ struct DenseMapInfo<llvm::ArrayRef<const char*>> {
 };
 
 }  // namespace llvm
+
+template <>
+struct std::formatter<clice::CompilationDatabase::QueryDriverError> :
+    std::formatter<llvm::StringRef> {
+
+    template <typename FormatContext>
+    auto format(clice::CompilationDatabase::QueryDriverError& e, FormatContext& ctx) const {
+        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,12 @@ 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.resource_dir = true;
+        options.query_driver = true;
+        options.suppress_log = true;
+        params.arguments = database.get_command(src_path, options).arguments;
 
         for(auto& [file, source]: sources.all_files) {
             if(file == src_path) {
@@ -67,7 +72,12 @@ 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.resource_dir = true;
+        options.query_driver = true;
+        options.suppress_log = true;
+        params.arguments = database.get_command(src_path, options).arguments;
 
         auto path = fs::createTemporaryFile("clice", "pch");
         if(!path) {
diff --git a/src/Compiler/Command.cpp b/src/Compiler/Command.cpp
@@ -95,21 +95,42 @@ std::optional<std::uint32_t> CompilationDatabase::get_option_id(llvm::StringRef
     }
 }
 
+namespace {
+
+llvm::SmallVector<llvm::StringRef, 4> driver_invocation_argv(llvm::StringRef driver) {
+    /// FIXME: MSVC command:` cl /Bv`, should we support it?
+    /// if (driver.starts_with("gcc") || driver.starts_with("g++") || driver.starts_with("clang")) {
+    ///      return {"-E", "-v", "-xc++", "/dev/null"};
+    /// } else if (driver.starts_with("cl") || driver.starts_with("clang-cl")) {
+    ///      return {"/Bv"};
+    /// }
+    return {driver, "-E", "-v", "-xc++", "/dev/null"};
+}
+
+using QueryDriverError = CompilationDatabase::QueryDriverError;
+using ErrorKind = CompilationDatabase::QueryDriverError::ErrorKind;
+
+auto unexpected(ErrorKind kind, std::string message) {
+    return std::unexpected<QueryDriverError>({kind, std::move(message)});
+};
+
+}  // namespace
+
 auto CompilationDatabase::query_driver(this Self& self, llvm::StringRef driver)
-    -> std::expected<DriverInfo, std::string> {
-    llvm::SmallString<128> buffer;
-
-    /// FIXME: Should we use a better way?
-    if(auto error = fs::real_path(driver, buffer)) {
-        auto result = llvm::sys::findProgramByName(driver);
-        if(!result) {
-            return std::unexpected(std::format("{}", result.getError()));
-        } else {
-            buffer = *result;
+    -> std::expected<DriverInfo, QueryDriverError> {
+    {
+        /// FIXME: Should we use a better way?
+        llvm::SmallString<128> absolute_path;
+        if(auto error = fs::real_path(driver, absolute_path)) {
+            auto result = llvm::sys::findProgramByName(driver);
+            if(!result) {
+                return unexpected(ErrorKind::NotFoundInPATH, result.getError().message());
+            }
+            absolute_path = *result;
         }
-    }
 
-    driver = self.save_string(buffer);
+        driver = self.save_string(absolute_path);
+    }
 
     auto it = self.driver_infos.find(driver.data());
     if(it != self.driver_infos.end()) {
@@ -120,18 +141,26 @@ 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 std::unexpected(std::format("{}", error));
+        return unexpected(ErrorKind::FailToCreateTempFile, error.message());
     }
 
-    auto clean_up = llvm::make_scope_exit([&output_path]() { fs::remove(output_path); });
+    // If we fail to get the driver infomation, keep the output file for user to debug.
+    bool keep_output_file = true;
+    auto clean_up = llvm::make_scope_exit([&output_path, &keep_output_file]() {
+        if(keep_output_file) {
+            return;
+        }
 
-    bool is_std_err = true;
+        if(auto errc = llvm::sys::fs::remove(output_path)) {
+            log::warn("Fail to remove temporary file: {}", errc.message());
+        }
+    });
 
+    bool is_std_err = true;
     std::optional<llvm::StringRef> redirects[] = {{""}, {""}, {""}};
     redirects[is_std_err ? 2 : 1] = output_path.str();
 
-    llvm::StringRef argv[] = {driver, "-E", "-v", "-xc++", "/dev/null"};
-
+    llvm::SmallVector argv = driver_invocation_argv(driver);
     std::string message;
     if(int RC = llvm::sys::ExecuteAndWait(driver,
                                           argv,
@@ -140,12 +169,12 @@ auto CompilationDatabase::query_driver(this Self& self, llvm::StringRef driver)
                                           /*SecondsToWait=*/0,
                                           /*MemoryLimit=*/0,
                                           &message)) {
-        return std::unexpected(std::format("{}", message));
+        return unexpected(ErrorKind::InvokeDriverFail, std::move(message));
     }
 
     auto file = llvm::MemoryBuffer::getFile(output_path);
     if(!file) {
-        return std::unexpected(std::format("{}", file.getError()));
+        return unexpected(ErrorKind::OutputFileNotReadable, file.getError().message());
     }
 
     llvm::StringRef content = file.get()->getBuffer();
@@ -191,13 +220,16 @@ auto CompilationDatabase::query_driver(this Self& self, llvm::StringRef driver)
     }
 
     if(!found_start_marker) {
-        return std::unexpected("Start marker not found...");
+        return unexpected(ErrorKind::InvalidOutputFormat, "Start marker not found...");
     }
 
     if(in_includes_block) {
-        return std::unexpected("End marker not found...");
+        return unexpected(ErrorKind::InvalidOutputFormat, "End marker not found...");
     }
 
+    // Get driver information success, remove temporary file.
+    keep_output_file = false;
+
     llvm::SmallVector<const char*, 8> includes;
     for(auto include: system_includes) {
         llvm::SmallString<64> buffer;
@@ -415,10 +447,8 @@ auto CompilationDatabase::load_commands(this Self& self, llvm::StringRef json_co
     return infos;
 }
 
-auto CompilationDatabase::get_command(this Self& self,
-                                      llvm::StringRef file,
-                                      bool resource_dir,
-                                      bool query_driver) -> LookupInfo {
+auto CompilationDatabase::get_command(this Self& self, llvm::StringRef file, CommandOptions options)
+    -> LookupInfo {
     LookupInfo info;
 
     file = self.save_string(file);
@@ -439,8 +469,9 @@ auto CompilationDatabase::get_command(this Self& self,
         info.arguments.emplace_back(self.save_string(argument).data());
     };
 
-    if(query_driver) {
-        if(auto driver_info = self.query_driver(info.arguments[0])) {
+    if(options.query_driver) {
+        llvm::StringRef driver = info.arguments[0];
+        if(auto driver_info = self.query_driver(driver)) {
             append_argument("-nostdlibinc");
 
             /// FIXME: Use target information here, this is useful for cross compilation.
@@ -450,18 +481,16 @@ auto CompilationDatabase::get_command(this Self& self,
                 append_argument("-I");
                 append_argument(system_header);
             }
-        } else {
-            /// FIXME: Error handle here.
-            log::warn("Fail query info for {}, because", info.arguments[0], 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));
     }
 
     info.arguments.emplace_back(file.data());
-
     return info;
 }
 
diff --git a/src/Server/Document.cpp b/src/Server/Document.cpp
@@ -110,7 +110,7 @@ void Server::save_cache_info() {
     }
 
     auto clean_up = llvm::make_scope_exit([&temp_path]() {
-        if(auto errc = llvm::sys::fs::remove(temp_path); errc != std::error_code{}) {
+        if(auto errc = llvm::sys::fs::remove(temp_path)) {
             log::warn("Fail to remove temporary file: {}", errc.message());
         }
     });
@@ -245,8 +245,12 @@ 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.resource_dir = true;
+    options.query_driver = true;
+    auto info = database.get_command(file, 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 ...
@@ -299,9 +303,13 @@ async::Task<> Server::build_ast(std::string path, std::string content) {
         log::fatal("Expected PCH built at this point");
     }
 
+    CommandOptions options;
+    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(path, 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,13 @@ auto Server::on_completion(proto::CompletionParams params) -> Result {
     auto offset = to_offset(kind, content, params.position);
     auto& pch = opening_file->pch;
     {
+        CommandOptions options;
+        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(path).arguments;
         params.add_remapped_file(path, content);
         params.pch = {pch->path, pch->preamble.size()};
         params.completion = {path, offset};
@@ -79,10 +82,13 @@ 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.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(path, options).arguments;
         params.add_remapped_file(path, content);
         params.pch = {pch->path, pch->preamble.size()};
         params.completion = {path, offset};
diff --git a/tests/unit/Compiler/Command.cpp b/tests/unit/Compiler/Command.cpp
