refactor: Introduce separate CommandObject type (#378)

Author: varungandhi-src

README.md

@@ -88,7 +88,7 @@ See the [Usage](#usage) section for step-by-step instructions.
 
 ## System Requirements
 
-1. About 2MB of temporary space per compilation database entry.
+1. About 2MB of temporary space for every TU in the compilation database.
    ```bash
    echo "$(perl -e "print $(jq 'length' build/compile_commands.json) / 512.0") GB"
    ```

indexer/CompilationDatabase.cc

@@ -157,6 +157,21 @@ ToolchainPathsResult static determineToolchainPaths(
 
 namespace scip_clang {
 namespace compdb {
+
+llvm::json::Value toJSON(const CommandObject &cmd) {
+  return llvm::json::Object{{"directory", cmd.workingDirectory},
+                            {"file", cmd.filePath},
+                            {"arguments", cmd.arguments}};
+}
+
+bool fromJSON(const llvm::json::Value &jsonValue, CommandObject &cmd,
+              llvm::json::Path path) {
+  llvm::json::ObjectMapper mapper(jsonValue, path);
+  return mapper && mapper.map("directory", cmd.workingDirectory)
+         && mapper.map("file", cmd.filePath)
+         && mapper.map("arguments", cmd.arguments);
+}
+
 namespace {
 
 // Handler to validate a compilation database in a streaming fashion.
@@ -287,7 +302,7 @@ class ValidateHandler
       if (this->options.checkDirectoryPathsAreAbsolute
           && this->lastKey == Key::Directory) {
         auto dirPath = std::string_view(str, length);
-        // NOTE(ref: directory-field-is-absolute): While the JSON compilation
+        // NOTE(def: directory-field-is-absolute): While the JSON compilation
         // database schema
         // (https://clang.llvm.org/docs/JSONCompilationDatabase.html) does not
         // specify if the "directory" key should be an absolute path or not, if
@@ -456,21 +471,20 @@ bool CommandObjectHandler::String(const char *str, rapidjson::SizeType length,
     ENFORCE(false, "unexpected input");
     return false;
   case Key::Directory:
-    this->wipCommand.Directory = std::string(str, length);
+    this->wipCommand.workingDirectory = std::string(str, length);
     break;
   case Key::File:
-    this->wipCommand.Filename = std::string(str, length);
+    this->wipCommand.filePath = std::string(str, length);
     break;
   case Key::Command:
-    this->wipCommand.CommandLine = scip_clang::unescapeCommandLine(
+    this->wipCommand.arguments = scip_clang::unescapeCommandLine(
         clang::tooling::JSONCommandLineSyntax::AutoDetect,
         std::string_view(str, length));
     break;
   case Key::Arguments: // Validator makes sure we have an array outside.
-    this->wipCommand.CommandLine.emplace_back(str, length);
+    this->wipCommand.arguments.emplace_back(str, length);
     break;
-  case Key::Output:
-    this->wipCommand.Output = std::string(str, length);
+  case Key::Output: // Do nothing
     break;
   }
   return true;
@@ -506,11 +520,10 @@ bool CommandObjectHandler::reachedLimit() const {
   return this->commands.size() == this->parseLimit;
 }
 
-CompilationDatabaseFile
-CompilationDatabaseFile::open(const StdPath &path,
-                              ValidationOptions validationOptions,
-                              std::error_code &fileSizeError) {
-  CompilationDatabaseFile compdbFile{};
+compdb::File compdb::File::open(const StdPath &path,
+                                ValidationOptions validationOptions,
+                                std::error_code &fileSizeError) {
+  compdb::File compdbFile{};
   compdbFile.file = std::fopen(path.c_str(), "rb");
   if (!compdbFile.file) {
     return compdbFile;
@@ -525,11 +538,11 @@ CompilationDatabaseFile::open(const StdPath &path,
   return compdbFile;
 }
 
-CompilationDatabaseFile CompilationDatabaseFile::openAndExitOnErrors(
-    const StdPath &path, ValidationOptions validationOptions) {
+compdb::File
+compdb::File::openAndExitOnErrors(const StdPath &path,
+                                  ValidationOptions validationOptions) {
   std::error_code fileSizeError;
-  auto compdbFile =
-      CompilationDatabaseFile::open(path, validationOptions, fileSizeError);
+  auto compdbFile = compdb::File::open(path, validationOptions, fileSizeError);
   if (!compdbFile.file) {
     spdlog::error("failed to open '{}': {}", path.string(),
                   std::strerror(errno));
@@ -548,8 +561,8 @@ CompilationDatabaseFile CompilationDatabaseFile::openAndExitOnErrors(
   return compdbFile;
 }
 
-void ResumableParser::initialize(CompilationDatabaseFile compdb,
-                                 size_t refillCount, bool inferResourceDir) {
+void ResumableParser::initialize(compdb::File compdb, size_t refillCount,
+                                 bool inferResourceDir) {
   auto averageJobSize = compdb.sizeInBytes() / compdb.commandCount();
   // Some customers have averageJobSize = 150KiB.
   // If numWorkers == 300 (very high core count machine),
@@ -567,8 +580,8 @@ void ResumableParser::initialize(CompilationDatabaseFile compdb,
   this->inferResourceDir = inferResourceDir;
 }
 
-void ResumableParser::parseMore(
-    std::vector<clang::tooling::CompileCommand> &out, bool checkFilesExist) {
+void ResumableParser::parseMore(std::vector<compdb::CommandObject> &out,
+                                bool checkFilesExist) {
   if (this->reader.IterativeParseComplete()) {
     if (this->reader.HasParseError()) {
       spdlog::error(
@@ -615,7 +628,8 @@ void ResumableParser::parseMore(
     }
     std::string pathBuffer;
     for (auto &cmd : this->handler->commands) {
-      if (checkFilesExist && !doesFileExist(cmd.Filename, cmd.Directory)) {
+      if (checkFilesExist
+          && !doesFileExist(cmd.filePath, cmd.workingDirectory)) {
         continue;
       }
       out.emplace_back(std::move(cmd));
@@ -625,10 +639,10 @@ void ResumableParser::parseMore(
 
   if (this->inferResourceDir) {
     for (auto &cmd : out) {
-      if (cmd.CommandLine.empty()) {
+      if (cmd.arguments.empty()) {
         continue;
       }
-      this->tryInferResourceDir(cmd.Directory, cmd.CommandLine);
+      this->tryInferResourceDir(cmd.workingDirectory, cmd.arguments);
     }
   }
 }

indexer/CompilationDatabase.h

@@ -15,26 +15,28 @@
 #include "spdlog/fmt/fmt.h"
 #include "spdlog/spdlog.h"
 
-#include "clang/Tooling/CompilationDatabase.h"
-
+#include "indexer/Derive.h"
 #include "indexer/FileSystem.h"
 
+namespace clang::tooling {
+struct CompileCommand;
+} // namespace clang::tooling
+
 namespace scip_clang {
 namespace compdb {
 
 struct ValidationOptions {
   bool checkDirectoryPathsAreAbsolute;
 };
 
-class CompilationDatabaseFile {
+class File {
   size_t _sizeInBytes;
   size_t _commandCount;
 
 public:
   FILE *file;
 
-  static CompilationDatabaseFile openAndExitOnErrors(const StdPath &,
-                                                     ValidationOptions);
+  static File openAndExitOnErrors(const StdPath &, ValidationOptions);
 
   size_t sizeInBytes() const {
     return this->_sizeInBytes;
@@ -44,8 +46,8 @@ class CompilationDatabaseFile {
   }
 
 private:
-  static CompilationDatabaseFile open(const StdPath &, ValidationOptions,
-                                      std::error_code &fileSizeError);
+  static File open(const StdPath &, ValidationOptions,
+                   std::error_code &fileSizeError);
 };
 
 // Key to identify fields in a command object
@@ -58,16 +60,30 @@ enum class Key : uint32_t {
   Output = 1 << 5,
 };
 
+/// The 'command object' terminology is taken from the official Clang docs.
+/// https://clang.llvm.org/docs/JSONCompilationDatabase.html
+struct CommandObject {
+  /// Strictly speaking, this should be an absolute directory in an actual
+  /// compilation database (see NOTE(ref: directory-field-is-absolute)),
+  /// but we use a std::string instead as it may be a relative path for
+  /// test cases.
+  std::string workingDirectory;
+  // May be relative or absolute
+  std::string filePath;
+  std::vector<std::string> arguments;
+};
+SERIALIZABLE(CommandObject)
+
 // Handler for extracting command objects from compilation database.
 class CommandObjectHandler
     : public rapidjson::BaseReaderHandler<rapidjson::UTF8<>,
                                           CommandObjectHandler> {
   compdb::Key previousKey;
-  clang::tooling::CompileCommand wipCommand;
+  compdb::CommandObject wipCommand;
   size_t parseLimit;
 
 public:
-  std::vector<clang::tooling::CompileCommand> commands;
+  std::vector<compdb::CommandObject> commands;
 
   CommandObjectHandler(size_t parseLimit)
       : previousKey(Key::Unset), wipCommand(), parseLimit(parseLimit),
@@ -97,7 +113,7 @@ class ResumableParser {
   absl::flat_hash_set<std::string> emittedErrors;
 
   /// Mapping from compiler/wrapper path to extra information needed
-  /// to tweak the compilation database entry before invoking the driver.
+  /// to tweak the command object before invoking the driver.
   ///
   /// For example, Bazel uses a compiler wrapper, but scip-clang needs
   /// to use the full path to the compiler driver when running semantic
@@ -113,13 +129,12 @@ class ResumableParser {
   /// If \param inferResourceDir is set, then the parser will automatically
   /// add extra '-resource-dir' '<path>' arguments to the parsed
   /// CompileCommands' CommandLine field.
-  void initialize(CompilationDatabaseFile compdb, size_t refillCount,
+  void initialize(compdb::File compdb, size_t refillCount,
                   bool inferResourceDir);
 
   // Parses at most refillCount elements (passed during initialization)
   // from the compilation database passed during initialization.
-  void parseMore(std::vector<clang::tooling::CompileCommand> &out,
-                 bool checkFilesExist = true);
+  void parseMore(std::vector<CommandObject> &out, bool checkFilesExist = true);
 
 private:
   void tryInferResourceDir(const std::string &directoryPath,

indexer/Driver.cc

@@ -35,6 +35,7 @@
 #include "spdlog/spdlog.h"
 
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/StringSaver.h"
 
 #include "proto/fwd_decls.pb.h"
@@ -467,7 +468,7 @@ class Scheduler final {
   uint32_t nextTaskId = 0;
   /// Monotonically growing map of all jobs that have been created so far.
   /// This number will generally be unrelated to \c compdbCommandCount
-  /// because a single compilation database entry will typically lead
+  /// because a single command object will typically lead
   /// to creation of multiple jobs.
   ///
   /// In principle, after a job is completed, we could start removing
@@ -538,7 +539,7 @@ class Scheduler final {
       switch (job.kind) {
       case IndexJob::Kind::SemanticAnalysis:
         return fmt::format("running semantic analysis for '{}'",
-                           job.semanticAnalysis.command.Filename);
+                           job.semanticAnalysis.command.filePath);
       case IndexJob::Kind::EmitIndex:
         auto &fileInfos = job.emitIndex.filesToBeIndexed;
         auto fileInfoIt = absl::c_find_if(
@@ -920,7 +921,7 @@ class Driver {
       ENFORCE(job.kind == IndexJob::Kind::SemanticAnalysis);
       perJobStats.emplace_back(
           jobId.taskId(),
-          StatsEntry{job.semanticAnalysis.command.Filename, std::move(stats)});
+          StatsEntry{job.semanticAnalysis.command.filePath, std::move(stats)});
     }
     absl::c_sort(perJobStats, [](const auto &p1, const auto &p2) -> bool {
       return p1.first < p2.first;
@@ -1142,7 +1143,7 @@ class Driver {
   }
 
   size_t refillJobs() {
-    std::vector<clang::tooling::CompileCommand> commands{};
+    std::vector<compdb::CommandObject> commands{};
     this->compdbParser.parseMore(commands);
     for (auto &command : commands) {
       this->scheduler.queueNewTask(
@@ -1184,7 +1185,7 @@ class Driver {
   FileGuard openCompilationDatabase() {
     std::error_code error;
     StdPath compdbStdPath{this->compdbPath().asStringRef()};
-    auto compdbFile = compdb::CompilationDatabaseFile::openAndExitOnErrors(
+    auto compdbFile = compdb::File::openAndExitOnErrors(
         compdbStdPath,
         compdb::ValidationOptions{.checkDirectoryPathsAreAbsolute =
                                       !this->options.isTesting});

indexer/IpcMessages.cc

@@ -15,26 +15,6 @@
 #include "indexer/Derive.h"
 #include "indexer/IpcMessages.h"
 
-namespace clang::tooling {
-
-llvm::json::Value toJSON(const clang::tooling::CompileCommand &cc) {
-  return llvm::json::Object{{"directory", cc.Directory},
-                            {"file", cc.Filename},
-                            {"output", cc.Output},
-                            {"arguments", cc.CommandLine}};
-}
-
-bool fromJSON(const llvm::json::Value &jsonValue,
-              clang::tooling::CompileCommand &cc, llvm::json::Path path) {
-  llvm::json::ObjectMapper mapper(jsonValue, path);
-  return mapper && mapper.map("directory", cc.Directory)
-         && mapper.map("file", cc.Filename)
-         && mapper.mapOptional("output", cc.Output)
-         && mapper.map("arguments", cc.CommandLine);
-}
-
-} // namespace clang::tooling
-
 namespace scip_clang {
 
 std::string driverToWorkerQueueName(std::string_view driverId,

indexer/IpcMessages.h

@@ -8,9 +8,9 @@
 #include <string>
 #include <string_view>
 
-#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/JSON.h"
 
+#include "indexer/CompilationDatabase.h"
 #include "indexer/Derive.h"
 #include "indexer/Hash.h"
 #include "indexer/Path.h"
@@ -100,7 +100,7 @@ template <> struct fmt::formatter<scip_clang::JobId> {
 namespace scip_clang {
 
 struct SemanticAnalysisJobDetails {
-  clang::tooling::CompileCommand command;
+  compdb::CommandObject command;
 };
 SERIALIZABLE(SemanticAnalysisJobDetails)
 

indexer/LlvmCommandLineParsing.cc

@@ -8,10 +8,9 @@
 #include "indexer/Enforce.h"
 #include "indexer/LlvmCommandLineParsing.h"
 
-// ----------------------------------- ATTENTION
-// --------------------------------- The code in this file is vendored from
-// Clang's JSONCompilationDatabase.cpp because the parser is not exposed in any
-// header.
+// --------------------------- ATTENTION -------------------------------------
+// The code in this file is vendored from Clang's JSONCompilationDatabase.cpp
+// because the parser is not exposed in any header.
 //
 // We could potentially have instead reused the APIs TokenizeWindowsCommandLine
 // and TokenizeGNUCommandLine from llvm/Support/CommandLine.h, but it is unclear