[clangd] DO NOT SUBMIT. Draft interfaces for build system integration.

[ilya-biryukov]

Summary

A proposal for the interface of the new build system integration.
Similar to CompilationDatabase, but will provide more functionality and live
in clangd instead of tooling until stabilized.

Main points of interest are:

• clangd::Integration is a user-facing API, it handles loading the proper build system integrations from the directory structure and notifying the clients about all changes. It implements clang::GlobalCompilationDatabase and is used by all of clangd to get compile commands, etc.

• clangd::BuildSystem interface is for build system implementations. It handles interactions with a single build system (initially only 'compile_commands.json', but we also plan to experiment with adding direct CMake or ninja support).

• The clangd::GlobalCompilationDatabase now also provides a VFS instance alongside the compile command. The idea is to allow build systems to overlay generated files, if they can need (this is Bazel-specific AFAIK). It is also the responsibility of the buildsystem to update any dependencies (e.g. the generated files) it knows about before providing the compile commands.

[klimek]

clangd/BuildSystem.h

29

'Integration' is a bit of a weird name, as it doesn't tell me what it does.
What I'd have expected is an abstraction like 'Workspace' (which I believe is a domain term in IDEs) that would:
-> be in some form coupled to 1 build system
-> provide a file system view on the workspace
-> provide the ability to watch for changed files

Then the BuildSystem would:
-> provide the ability to watch for changed inputs / commands
-> provide the ability to "prepare all inputs"
-> potentially provide the ability to get a build graph

If Integration wants to be that, it should:

• not extend GlobalCompilationDatabase
• not have getCompileCommand itself

clangd/BuildSystemPlugin.h

31

Do you actually foresee more than 1 being available in a single workspace? Feels a bit premature generalization to me :)

[ilya-biryukov]

Thanks for taking a look and sorry for the confusion with naming. I should've renamed Integration before sending this patch.

clangd/BuildSystem.h

29

'Integration' is a bit of a weird name, as it doesn't tell me what it does.

It is, sorry about that. This is not final. I intended to put this into the 'buildsystem' namespace, so it'd be used as 'buildsystem::Integration', which would make more sense.

What I'd have expected is an abstraction like 'Workspace' (which I believe is a domain term in IDEs) that would:
-> be in some form coupled to 1 build system
-> provide a file system view on the workspace
-> provide the ability to watch for changed files

The abstraction you're looking for is called BuildSystem in this patch.
It is decoupled from watching for files (this is handled by FileSystemProvider), it can watch files internally but tells the users about the changes in terms of source files that had their inputs changed.

Then the BuildSystem would:
-> provide the ability to watch for changed inputs / commands
-> provide the ability to "prepare all inputs"

This is exactly what BuildSystem does.

-> potentially provide the ability to get a build graph

I was thinking about this as well, but still not sure if this information is better inferred by clangd when running the compiler, so decided to leave it out until we have a use-case for that.

If Integration wants to be that, it should:
not extend GlobalCompilationDatabase
not have getCompileCommand itself

There is a discrepancy between the needs of the build system implementations and what clangd has at the protocol level.

• At the protocol level, clangd operates on the files and it needs to provide features on a per-file manner: get diagnostics for a file, run code completion in a file, format a file, etc.
• At the build system implementation level, we care about different things: discovering which build system is used, watching for changes to important files (BUILD, CMakeLists.txt, compiler_commands.json, etc.), reporting those changes to the build system users, building the generated files, providing compile commands, adding build depenndecies, etc.

buildsystem::Integration serves as a layer that ties those two together and provides an interface that higher layers of clangd are more comfortable with, e.g. a file-based interface that GlobalCompilatioDatabase provides.

My initial goal was to replace the GlobalCompilationDatabase with Integration and make it a non-abstract class, making it clear it just wraps build system integration into a form that's a bit more convenient for clangd uses. This turned out to be some work (we have OverlayCDB and test compilation databases that rely on this interface, so it's some work and it's not clear if that would make the code simpler at the end, so I decided to postpone it for now.

clangd/BuildSystemPlugin.h

31

It's not a problem with compile_commands.json, but I foresee that being a problem if we start supporting the actual build systems like cmake and bazel. Sadly it's not uncommon for a C++ project to have multiple build systems (in a transition phase, to support more platforms, etc).

[klimek]

clangd/BuildSystem.h

29

So, I would still vote for separating a Workspace from a BuildSystem, as I'd expect a Workspace over time to grow VCS capabilities, and I'd expect having one abstractions that covers both builds and VCSs confusing.

I now understand the "Integration" layer better, but it still looks like it's doing too many things:
specificially, it seems like it's basically at the same time a single view of all build systems / workspaces AND the thing that manages them - and I'd definitely split that up; that is, both of these are useful but they should be different classes.

[ilya-biryukov]

clangd/BuildSystem.h

29

So, I would still vote for separating a Workspace from a BuildSystem, as I'd expect a Workspace over time to grow VCS capabilities, and I'd expect having one abstractions that covers both builds and VCSs confusing.

That makes sense to me too, but I don't have the full understanding of the VCS capabilities we might want to have. Something that popped up before in various discussions was tracking a changing VCS branch to keep separate versions of the index to hotswap. Will have to think more about it, thanks for the input.

specificially, it seems like it's basically at the same time a single view of all build systems / workspaces AND the thing that manages them - and I'd definitely split that up; that is, both of these are useful but they should be different classes.

That makes total sense, the implementation is actually split even though it's not visible in this patch: the private Discovery class handles the loading of the build system tree, etc.

clangd/BuildSystem.h

+//===--- BuildSystem.h --------------------------------------------*-C++-*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// Interfaces for buildsystem integration.
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_BUILD_SYSTEM_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_BUILD_SYSTEM_H
+
+#include "FSProvider.h"
+#include "Function.h"
+#include "GlobalCompilationDatabase.h"
+#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/FunctionExtras.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include <memory>
+#include <mutex>
+
+namespace clang {
+namespace clangd {
+
+class BuildSystem;
+
+/// Default compilation database used by clangd, based on the build system.
+class Integration : public GlobalCompilationDatabase {
+public:
+  /// Loads the build systems by consulting the registry of available
+  /// BuildSystemPlugin instances.
+  static std::unique_ptr<Integration> PluginBased(FileSystemProvider &FS);
+
+  using GlobalLoaderFunc =
+      llvm::unique_function<std::shared_ptr<BuildSystem>()>;
+  /// Will load a single build system once and reuse it for all files that are
+  /// required.
+  static std::unique_ptr<Integration> Global(FileSystemProvider &FS,
+                                             GlobalLoaderFunc Loader);
+
+  ~Integration();
+
+  FileSystemProvider &fs() const override;
+
+  llvm::Optional<CompileInputs> getCompileCommand(PathRef File) const override;
+
+  CommandChanged::Subscription watch(CommandChanged::Listener L) const override;
+
+  class Discovery;
+
+private:
+  Integration(std::unique_ptr<Discovery> D, FileSystemProvider &FS);
+
+  FileSystemProvider &FS;
+  std::unique_ptr<Discovery> D;
+
+  /// An event firing when the build system is loaded.
+  using BSLoadedEvent = Event<std::shared_ptr<BuildSystem>>;
+  BSLoadedEvent::Subscription Subscription;
+
+  mutable CommandChanged OnFilesAdded;
+};
+
+/// Handles interaction between clangd and a build system.
+/// Similar to a CompilationDatabase, but additionally supports change tracking
+/// facilities.
+/// The abstraction is highly experimental, we expect to change the interface in
+/// the future.
+class BuildSystem {
+public:
+  virtual ~BuildSystem() = default;
+
+  /// Provide compilation command for the provided \p Path, possibly building
+  /// all prerequisites if the build system supports it.
+  virtual llvm::Optional<CompileInputs> buildInputs(FileSystemProvider &FS,
+                                                    llvm::StringRef Path) = 0;
+
+  /// Notification that fires when inputs for a set of files might have changed.
+  virtual CommandChanged::Subscription watch(CommandChanged::Listener L) = 0;
+};
+
+/// Attempt to load a build system under \p Dir with a set of plugins.
+/// Does not traverse parent directories.
+std::unique_ptr<BuildSystem> loadWithPlugins(FileSystemProvider &FS,
+                                             llvm::StringRef Dir);
+
+} // namespace clangd
+} // namespace clang
+
+#endif
 No newline at end of file

clangd/BuildSystemPlugin.h

+//===--- BuildSystemPlugin.h --------------------------------------*-C++-*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// Interfaces for buildsystem integration.
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_BUILD_SYSTEM_PLUGIN_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_BUILD_SYSTEM_PLUGIN_H
+
+#include "FSProvider.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Registry.h"
+
+namespace clang {
+namespace clangd {
+
+class BuildSystem;
+
+/// A plugin to provide a build system integration for clangd.
+class BuildSystemPlugin {
+public:
+  virtual ~BuildSystemPlugin() = default;
+
+  /// Registered plugins are consulted in the decreasing order of their weights.
+  /// The first plugin that produces a non-null BuildSystem wins.
+  /// The 'compile_commands.json' has a weight of 1.
+  virtual float weight() = 0;
+  /// Attempt to detect and load the build system that resides in \p Dir.
+  /// Should return null if detection failed.
+  virtual std::unique_ptr<BuildSystem> tryLoad(FileSystemProvider &FS,
+                                               llvm::StringRef Dir) = 0;
+};
+
+using BuildSystemPluginRegistry = llvm::Registry<BuildSystemPlugin>;
+
+} // namespace clangd
+} // namespace clang
+
+#endif
 No newline at end of file

clangd/FSProvider.h

 //===--- FSProvider.h - VFS provider for ClangdServer ------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FSPROVIDER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FSPROVIDER_H
 
+#include "FSWatch.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Support/VirtualFileSystem.h"
 
 namespace clang {
 namespace clangd {
 
 // Wrapper for vfs::FileSystem for use in multithreaded programs like clangd.
 // As FileSystem is not threadsafe, concurrent threads must each obtain one.
 class FileSystemProvider {
 public:
   virtual ~FileSystemProvider() = default;
+
   /// Called by ClangdServer to obtain a vfs::FileSystem to be used for parsing.
   /// Context::current() will be the context passed to the clang entrypoint,
   /// such as addDocument(), and will also be propagated to result callbacks.
   /// Embedders may use this to isolate filesystem accesses.
   virtual llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
   getFileSystem() const = 0;
+
+  virtual FileSystemWatcher &watcher() = 0;
 };
 
 class RealFileSystemProvider : public FileSystemProvider {
 public:
   // FIXME: returns the single real FS instance, which is not threadsafe.
   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
   getFileSystem() const override {
     return llvm::vfs::getRealFileSystem();
   }
 };
 
 } // namespace clangd
 } // namespace clang
 
 #endif

clangd/FSWatch.h

+//===--- FSWatch.h ------------------------------------------------*-C++-*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FS_WATCH_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FS_WATCH_H
+
+#include "Function.h"
+#include "llvm/ADT/FunctionExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include <memory>
+
+namespace clang {
+namespace clangd {
+
+enum class FSEventKind { Added, Modified, Removed };
+
+/// An event that fires when the file has changed.
+using FileChangedEvent = Event<FSEventKind>;
+
+/// An interface to allow watching for changes in the filesystem.
+class FileSystemWatcher {
+public:
+  virtual ~FileSystemWatcher() = default;
+
+  /// Add a watch for a single file or directory.
+  virtual FileChangedEvent::Subscription
+  watchFile(llvm::StringRef Path, FileChangedEvent::Listener L) = 0;
+};
+
+/// Polls the underlying VFS for changes every \p PollIntervalSecs seconds on a
+/// separate thread. Should only be used when a small number of files.
+std::unique_ptr<FileSystemWatcher>
+createPollingWatcher(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
+                     std::chrono::duration<double> PollIntervalSecs);
+
+/// Never reports any changes.
+std::unique_ptr<FileSystemWatcher> createSilentWatcher();
+} // namespace clangd
+} // namespace clang
+
+#endif
 No newline at end of file

clangd/GlobalCompilationDatabase.h

 //===--- GlobalCompilationDatabase.h -----------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_GLOBALCOMPILATIONDATABASE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_GLOBALCOMPILATIONDATABASE_H
 
+#include "FSProvider.h"
 #include "Function.h"
 #include "Path.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/StringMap.h"
 #include <memory>
 #include <mutex>
 #include <vector>
 
 namespace clang {
 
 namespace tooling {
 class CompilationDatabase;
 struct CompileCommand;
 } // namespace tooling
 
 namespace clangd {
 
 class Logger;
 
 struct ProjectInfo {
   // The directory in which the compilation database was discovered.
   // Empty if directory-based compilation database discovery was not used.
   std::string SourceRoot;
 };
 
+/// Full information required to run clang frontend on the file.
+struct CompileInputs {
+  /// Command used to build the AST.
+  tooling::CompileCommand Command;
+  /// An instance of VFS that should be used for compilations. Must have proper
+  /// working directory set in advance, can contain overlays that include the
+  /// generated files.
+  IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS;
+  /// Information about the project that the file belongs to.
+  llvm::Optional<ProjectInfo> Project;
+};
+
+using CommandChanged = Event<std::vector<std::string>>;
+
 /// Provides compilation arguments used for parsing C and C++ files.
 class GlobalCompilationDatabase {
 public:
   virtual ~GlobalCompilationDatabase() = default;
 
+  virtual FileSystemProvider &fs() const = 0;
+
   /// If there are any known-good commands for building this file, returns one.
-  /// If the ProjectInfo pointer is set, it will also be populated.
-  virtual llvm::Optional<tooling::CompileCommand>
-  getCompileCommand(PathRef File, ProjectInfo * = nullptr) const = 0;
+  virtual llvm::Optional<CompileInputs>
+  getCompileCommand(PathRef File) const = 0;
 
   /// Makes a guess at how to build a file.
   /// The default implementation just runs clang on the file.
   /// Clangd should treat the results as unreliable.
   virtual tooling::CompileCommand getFallbackCommand(PathRef File) const;
 
   using CommandChanged = Event<std::vector<std::string>>;
   /// The callback is notified when files may have new compile commands.
   /// The argument is a list of full file paths.
-  CommandChanged::Subscription watch(CommandChanged::Listener L) const {
-    return OnCommandChanged.observe(std::move(L));
-  }
-
-protected:
-  mutable CommandChanged OnCommandChanged;
+  virtual CommandChanged::Subscription watch(CommandChanged::Listener L) const;
 };
 
 /// Gets compile args from tooling::CompilationDatabases built for parent
 /// directories.
 class DirectoryBasedGlobalCompilationDatabase
     : public GlobalCompilationDatabase {
 public:
   DirectoryBasedGlobalCompilationDatabase(