[clangd] Use ToolExecutor to write the global-symbol-builder tool.

[ioeric]

Summary

This enables more execution modes like standalone and Mapreduce-style execution.

See also D41729

[hokein]

The implementation looks good, but to see whether sam has high-level comments on this.

clangd/index/SymbolYAML.cpp

140

The function could be simplified by using the SymbolToYAML below.

std::string Str;
for (const Symbol& S : Symbols) {
   Str += SymbolToYAML(S);
}

145

might be use Symbol as parameter type.

clangd/index/SymbolYAML.h

30

might be also mention The YAML result is safe to concatenate.

[ioeric]

Address review comments.

clangd/index/SymbolYAML.cpp

140

This saves two lines of code but would regress the performance, so I am inclined to the current approach.

[sammccall]

This makes SymbolCollector (production code) depend on YAML (explicitly experimental), as well as on Tooling interfaces.
At least we should invert this by making the thing passed to SymbolCollector a function<void(const Symbol&)> or so.

Moreover, it's unfortunate we'd need to change SymbolCollector at all. Currently it's a nice abstraction that does one thing (AST -> SymbolSlab), now we're making it do two things. It can't be for scalability reasons - there's no problem fitting all the symbols from a TU in memory. So why do we need to do this?
I don't really understand the ToolExecutor interfaces well, but ISTM overriding SymbolIndexActionFactory::runInvocation to flush the symbols in the slab out to the ExecutionContext would allow you to use SymbolCollector unchanged?

[ioeric]

Address review comment: move all ToolExecutor and YAML-specific logics out of SymbolCollector.

[ioeric]

In D41730#970704, @sammccall wrote:

This makes SymbolCollector (production code) depend on YAML (explicitly experimental), as well as on Tooling interfaces.
At least we should invert this by making the thing passed to SymbolCollector a function<void(const Symbol&)> or so.

Thanks for the catch! I completely missed these perspectives...

Moreover, it's unfortunate we'd need to change SymbolCollector at all. Currently it's a nice abstraction that does one thing (AST -> SymbolSlab), now we're making it do two things. It can't be for scalability reasons - there's no problem fitting all the symbols from a TU in memory. So why do we need to do this?
I don't really understand the ToolExecutor interfaces well, but ISTM overriding SymbolIndexActionFactory::runInvocation to flush the symbols in the slab out to the ExecutionContext would allow you to use SymbolCollector unchanged?

WrapperFrontendAction is helpful here. All changes have been moved into the tool now. PTAL

clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp

 // production code.
 //
 //===---------------------------------------------------------------------===//
 
 #include "index/Index.h"
 #include "index/SymbolCollector.h"
 #include "index/SymbolYAML.h"
 #include "clang/Frontend/FrontendActions.h"
-#include "clang/Index/IndexingAction.h"
 #include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexingAction.h"
 #include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Execution.h"
 #include "clang/Tooling/Tooling.h"
-#include "clang/Tooling/CommonOptionsParser.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/MemoryBuffer.h"
-#include "llvm/Support/Signals.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Signals.h"
 #include "llvm/Support/ThreadPool.h"
 
 using namespace llvm;
 using namespace clang::tooling;
 using clang::clangd::SymbolSlab;
 
 namespace clang {
 namespace clangd {
 
 class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
 public:
-  SymbolIndexActionFactory() = default;
+  SymbolIndexActionFactory(tooling::ExecutionContext *Ctx) : Ctx(Ctx) {}
 
   clang::FrontendAction *create() override {
     index::IndexingOptions IndexOpts;
     IndexOpts.SystemSymbolFilter =
         index::IndexingOptions::SystemSymbolFilterKind::All;
     IndexOpts.IndexFunctionLocals = false;
-    Collector = std::make_shared<SymbolCollector>();
-    return index::createIndexingAction(Collector, IndexOpts, nullptr).release();
+    return index::createIndexingAction(std::make_shared<SymbolCollector>(Ctx),
+                                       IndexOpts, nullptr)
+        .release();
   }
 
   std::shared_ptr<SymbolCollector> Collector;
+  tooling::ExecutionContext *Ctx;
 };
 
 } // namespace clangd
 } // namespace clang
 
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
   const char* Overview =
       "This is an **experimental** tool to generate YAML-format "
       "project-wide symbols for clangd (global code completion). It would be "
       "changed and deprecated eventually. Don't use it in production code!";
-  CommonOptionsParser OptionsParser(argc, argv, cl::GeneralCategory,
-                                    /*Overview=*/Overview);
-
-  // No compilation database found, fallback to single TU analysis, this is
-  // mainly for debugging purpose:
-  //   global-symbol-buidler /tmp/t.cc -- -std=c++11.
-  if (OptionsParser.getCompilations().getAllFiles().empty()) {
-    llvm::errs() << "No compilation database found, processing individual "
-                    "files with flags from command-line\n.";
-    ClangTool Tool(OptionsParser.getCompilations(),
-                   OptionsParser.getSourcePathList());
-    clang::clangd::SymbolIndexActionFactory IndexAction;
-    Tool.run(&IndexAction);
-    llvm::outs() << SymbolToYAML(IndexAction.Collector->takeSymbols());
-    return 0;
-  }
+  auto Executor = clang::tooling::createExecutorFromCommandLineArgs(
+      argc, argv, cl::GeneralCategory, Overview);
 
-  // Found compilation database, we iterate all TUs from database to get all
-  // symbols, and then merge them into a single SymbolSlab.
-  SymbolSlab::Builder GlobalSymbols;
-  std::mutex SymbolMutex;
-  auto AddSymbols = [&](const SymbolSlab& NewSymbols) {
-    // Synchronize set accesses.
-    std::unique_lock<std::mutex> LockGuard(SymbolMutex);
-    for (auto Sym : NewSymbols) {
-      // FIXME: Better handling the overlap symbols, currently we overwrite it
-      // with the latest one, but we always want to good declarations (class
-      // definitions, instead of forward declarations).
-      GlobalSymbols.insert(Sym);
-    }
-  };
+  if (!Executor) {
+    llvm::errs() << llvm::toString(Executor.takeError()) << "\n";
+    return 1;
 
-  {
-    llvm::ThreadPool Pool;
-    for (auto& file : OptionsParser.getCompilations().getAllFiles()) {
-      Pool.async([&OptionsParser, &AddSymbols](llvm::StringRef Path) {
-        ClangTool Tool(OptionsParser.getCompilations(), {Path});
-        clang::clangd::SymbolIndexActionFactory IndexAction;
-        Tool.run(&IndexAction);
-        AddSymbols(IndexAction.Collector->takeSymbols());
-      }, file);
-    }
+  auto Err = Executor->get()->execute(
+      llvm::make_unique<clang::clangd::SymbolIndexActionFactory>(
+          Executor->get()->getExecutionContext()));
+  if (Err) {
+    llvm::errs() << llvm::toString(std::move(Err)) << "\n";
   }
 
-  llvm::outs() << SymbolToYAML(std::move(GlobalSymbols).build());
+  Executor->get()->getToolResults()->forEachResult(
+      [](llvm::StringRef, llvm::StringRef Value) { llvm::outs() << Value; });
   return 0;
 }

clangd/index/SymbolCollector.h

 //===--- SymbolCollector.h ---------------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Index.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexSymbol.h"
+#include "clang/Tooling/Execution.h"
 
 namespace clang {
 namespace clangd {
 
-// Collect all symbols from an AST.
-//
-// Clients (e.g. clangd) can use SymbolCollector together with
-// index::indexTopLevelDecls to retrieve all symbols when the source file is
-// changed.
+/// \brief Collect all symbols from an AST.
+///
+/// Clients (e.g. clangd) can use SymbolCollector together with
+/// index::indexTopLevelDecls to retrieve all symbols when the source file is
+/// changed.
 class SymbolCollector : public index::IndexDataConsumer {
 public:
-  SymbolCollector() = default;
+  /// \brief If Context is provided, collected symbols will also be reported via
+  /// the execution context with symbol IDs as keys and yamlized symbols as
+  /// values.
+  SymbolCollector(tooling::ExecutionContext *Context = nullptr)
+      : ExecutionCtx(Context) {}
 
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
                       ArrayRef<index::SymbolRelation> Relations, FileID FID,
                       unsigned Offset,
                       index::IndexDataConsumer::ASTNodeInfo ASTNode) override;
 
   SymbolSlab takeSymbols() { return std::move(Symbols).build(); }
 
 private:
   // All Symbols collected from the AST.
   SymbolSlab::Builder Symbols;
+  tooling::ExecutionContext *ExecutionCtx;
 };
 
 } // namespace clangd
 } // namespace clang

clangd/index/SymbolCollector.cpp

 //===--- SymbolCollector.cpp -------------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "SymbolCollector.h"
+#include "index/SymbolYAML.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/USRGeneration.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
if (const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D)) {
     auto ScopeAndName = splitQualifiedName(QName);
 
     Symbol S;
     S.ID = std::move(ID);
     S.Scope = ScopeAndName.first;
     S.Name = ScopeAndName.second;
     S.SymInfo = index::getSymbolInfo(D);
     S.CanonicalDeclaration = Location;
+    if (ExecutionCtx) {
+      std::string IDStr;
+      llvm::raw_string_ostream OS(IDStr);
+      OS << S.ID;
+      ExecutionCtx->reportResult(OS.str(), SymbolToYAML(S));
+    }
     Symbols.insert(S);
   }
 
   return true;
 }
 
 } // namespace clangd
 } // namespace clang

clangd/index/SymbolYAML.h

 #include "llvm/Support/Error.h"
 
 namespace clang {
 namespace clangd {
 
 // Read symbols from a YAML-format string.
 SymbolSlab SymbolFromYAML(llvm::StringRef YAMLContent);
 
+// Convert a single symbol to YAML-format string.
+// The YAML result is safe to concatenate.
+std::string SymbolToYAML(Symbol Sym);
+
 // Convert symbols to a YAML-format string.
 // The YAML result is safe to concatenate if you have multiple symbol slabs.
-std::string SymbolToYAML(const SymbolSlab& Symbols);
+std::string SymbolsToYAML(const SymbolSlab& Symbols);
 
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_FROM_YAML_H

clangd/index/SymbolYAML.cpp

SymbolSlab SymbolFromYAML(llvm::StringRef YAMLContent) {
   llvm::yaml::Input Yin(YAMLContent);
   Yin >> S;
   SymbolSlab::Builder Syms;
   for (auto& Sym : S)
     Syms.insert(Sym);
   return std::move(Syms).build();
 }
 
-std::string SymbolToYAML(const SymbolSlab& Symbols) {
+std::string SymbolsToYAML(const SymbolSlab& Symbols) {
   std::string Str;
   llvm::raw_string_ostream OS(Str);
   llvm::yaml::Output Yout(OS);
   for (Symbol S : Symbols) // copy: Yout<< requires mutability.
     Yout<< S;
   return OS.str();
 }
 
+std::string SymbolToYAML(Symbol Sym) {
+  std::string Str;
+  llvm::raw_string_ostream OS(Str);
+  llvm::yaml::Output Yout(OS);
+  Yout << Sym;
+  return OS.str();
+}
+
 } // namespace clangd
 } // namespace clang

unittests/clangd/SymbolCollectorTests.cpp

)";
   auto Symbols1 = SymbolFromYAML(YAML1);
   EXPECT_THAT(Symbols1,
               UnorderedElementsAre(QName("clang::Foo1")));
   auto Symbols2 = SymbolFromYAML(YAML2);
   EXPECT_THAT(Symbols2,
               UnorderedElementsAre(QName("clang::Foo2")));
 
   std::string ConcatenatedYAML =
-      SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
+      SymbolsToYAML(Symbols1) + SymbolsToYAML(Symbols2);
   auto ConcatenatedSymbols = SymbolFromYAML(ConcatenatedYAML);
   EXPECT_THAT(ConcatenatedSymbols,
               UnorderedElementsAre(QName("clang::Foo1"),
                                    QName("clang::Foo2")));
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang