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 =


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 =
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to