[PATCH] D91351: [tooling] Implement determinsitic ordering of CompilationDatabasePlugins

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: klimek, nridge.
Herald added subscribers: cfe-commits, usaxena95, kadircet, mgrang.
Herald added a project: clang.
njames93 requested review of this revision.
Herald added a subscriber: ilya-biryukov.

Right now plugins appear to be registed in an undefined order based on what the linker decides to link in first.
This is undesireable as if there are multiple potential databases in a directory we can't specify which one should be treated as the defacto.

To address this there is a virtual getPriority method in the `CompilationDatabasePlugin`.
Plugins will then be sorted based on that(lowest first) and then search for a CompilationDatabase in the specified directory.

JSON database will now load first before Fixed database but there is plenty of room to insert plugins that would take priority over JSON, like ninja or make files.

Addresses https://github.com/clangd/clangd/issues/578


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91351

Files:
  clang/include/clang/Tooling/CompilationDatabasePluginRegistry.h
  clang/lib/Tooling/CompilationDatabase.cpp
  clang/lib/Tooling/JSONCompilationDatabase.cpp


Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===================================================================
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -173,6 +173,13 @@
                           std::move(Base), llvm::vfs::getRealFileSystem())))
                 : nullptr;
   }
+
+  unsigned int getPriority() const override {
+    // Pretty arbitrary. However we want to leave room for database sources
+    // which could have a higher priority, like directly supporting ninja or
+    // make files.
+    return 50;
+  }
 };
 
 } // namespace
Index: clang/lib/Tooling/CompilationDatabase.cpp
===================================================================
--- clang/lib/Tooling/CompilationDatabase.cpp
+++ clang/lib/Tooling/CompilationDatabase.cpp
@@ -64,14 +64,26 @@
 CompilationDatabase::loadFromDirectory(StringRef BuildDirectory,
                                        std::string &ErrorMessage) {
   llvm::raw_string_ostream ErrorStream(ErrorMessage);
+  using NameAndPlugin =
+      std::pair<StringRef, std::unique_ptr<CompilationDatabasePlugin>>;
+  llvm::SmallVector<NameAndPlugin, 4> Plugins;
+
   for (const CompilationDatabasePluginRegistry::entry &Database :
        CompilationDatabasePluginRegistry::entries()) {
+    Plugins.emplace_back(Database.getName(), Database.instantiate());
+  }
+  llvm::sort(Plugins, [](const NameAndPlugin &LHS, const NameAndPlugin &RHS) {
+    assert(LHS.second->getPriority() != RHS.second->getPriority() &&
+           "Undeterministic priority");
+    return LHS.second->getPriority() < RHS.second->getPriority();
+  });
+  for (auto &Plugin : Plugins) {
     std::string DatabaseErrorMessage;
-    std::unique_ptr<CompilationDatabasePlugin> Plugin(Database.instantiate());
     if (std::unique_ptr<CompilationDatabase> DB =
-            Plugin->loadFromDirectory(BuildDirectory, DatabaseErrorMessage))
+            Plugin.second->loadFromDirectory(BuildDirectory,
+                                             DatabaseErrorMessage))
       return DB;
-    ErrorStream << Database.getName() << ": " << DatabaseErrorMessage << "\n";
+    ErrorStream << Plugin.first << ": " << DatabaseErrorMessage << "\n";
   }
   return nullptr;
 }
@@ -405,6 +417,12 @@
     llvm::sys::path::append(DatabasePath, "compile_flags.txt");
     return FixedCompilationDatabase::loadFromFile(DatabasePath, ErrorMessage);
   }
+
+  unsigned int getPriority() const override {
+    // Give this a high value as it should be the fallback and any other Plugin
+    // should be used first.
+    return 100;
+  }
 };
 
 } // namespace
Index: clang/include/clang/Tooling/CompilationDatabasePluginRegistry.h
===================================================================
--- clang/include/clang/Tooling/CompilationDatabasePluginRegistry.h
+++ clang/include/clang/Tooling/CompilationDatabasePluginRegistry.h
@@ -34,6 +34,10 @@
   /// \see CompilationDatabase::loadFromDirectory().
   virtual std::unique_ptr<CompilationDatabase>
   loadFromDirectory(StringRef Directory, std::string &ErrorMessage) = 0;
+
+  /// Used to sort the plugins before attempting to load. Plugins with a lower
+  /// value here will run over a directory first.
+  virtual unsigned getPriority() const = 0;
 };
 
 using CompilationDatabasePluginRegistry =


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D91351.304821.patch
Type: text/x-patch
Size: 3335 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201112/8266c72d/attachment-0001.bin>