[llvm-branch-commits] [clang-tools-extra] c9776c8 - [clangd] Introduce config compilation for External blocks

Kadir Cetinkaya via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Sun Nov 22 12:17:47 PST 2020


Author: Kadir Cetinkaya
Date: 2020-11-22T20:59:37+01:00
New Revision: c9776c8d4ef7c1d69b6d74b81627c4028396e7c1

URL: https://github.com/llvm/llvm-project/commit/c9776c8d4ef7c1d69b6d74b81627c4028396e7c1
DIFF: https://github.com/llvm/llvm-project/commit/c9776c8d4ef7c1d69b6d74b81627c4028396e7c1.diff

LOG: [clangd] Introduce config compilation for External blocks

Compilation logic for External blocks. A few of the high level points:
- Requires exactly one-of File/Server at a time:
  - Server is ignored in case of both, with a warning.
  - Having none is an error, would render ExternalBlock void.
- Ensures mountpoint is an absolute path:
  - Interprets it as relative to FragmentDirectory.
  - Defaults to FragmentDirectory when empty.
- Marks Background as Skip.

Depends on D90748.

Differential Revision: https://reviews.llvm.org/D90749

Added: 
    

Modified: 
    clang-tools-extra/clangd/Config.h
    clang-tools-extra/clangd/ConfigCompile.cpp
    clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index ff285d34633b..318220364e6e 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -26,6 +26,7 @@
 
 #include "support/Context.h"
 #include "llvm/ADT/FunctionExtras.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
 #include <string>
 #include <vector>
@@ -58,10 +59,22 @@ struct Config {
   } CompileFlags;
 
   enum class BackgroundPolicy { Build, Skip };
+  /// Describes an external index configuration.
+  struct ExternalIndexSpec {
+    enum { File, Server } Kind;
+    /// This is one of:
+    /// - Address of a clangd-index-server, in the form of "ip:port".
+    /// - Absolute path to an index produced by clangd-indexer.
+    std::string Location;
+    /// Absolute path to source root this index is associated with, uses
+    /// forward-slashes.
+    std::string MountPoint;
+  };
   /// Controls background-index behavior.
   struct {
     /// Whether this TU should be indexed.
     BackgroundPolicy Background = BackgroundPolicy::Build;
+    llvm::Optional<ExternalIndexSpec> External;
   } Index;
 
   /// Style of the codebase.

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index 3f4dcd3c036d..846c6a170b38 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -27,11 +27,19 @@
 #include "Config.h"
 #include "ConfigFragment.h"
 #include "ConfigProvider.h"
+#include "Features.inc"
 #include "support/Logger.h"
 #include "support/Trace.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/SMLoc.h"
@@ -102,6 +110,27 @@ struct FragmentCompiler {
     return Result;
   }
 
+  llvm::Optional<std::string> makeAbsolute(Located<std::string> Path,
+                                           llvm::StringLiteral Description,
+                                           llvm::sys::path::Style Style) {
+    if (llvm::sys::path::is_absolute(*Path))
+      return *Path;
+    if (FragmentDirectory.empty()) {
+      diag(Error,
+           llvm::formatv(
+               "{0} must be an absolute path, because this fragment is not "
+               "associated with any directory.",
+               Description)
+               .str(),
+           Path.Range);
+      return llvm::None;
+    }
+    llvm::SmallString<256> AbsPath = llvm::StringRef(*Path);
+    llvm::sys::fs::make_absolute(FragmentDirectory, AbsPath);
+    llvm::sys::path::native(AbsPath, Style);
+    return AbsPath.str().str();
+  }
+
   // Helper with similar API to StringSwitch, for parsing enum values.
   template <typename T> class EnumSwitch {
     FragmentCompiler &Outer;
@@ -243,6 +272,59 @@ struct FragmentCompiler {
         Out.Apply.push_back(
             [Val](const Params &, Config &C) { C.Index.Background = *Val; });
     }
+    if (F.External)
+      compile(std::move(**F.External), F.External->Range);
+  }
+
+  void compile(Fragment::IndexBlock::ExternalBlock &&External,
+               llvm::SMRange BlockRange) {
+#ifndef CLANGD_ENABLE_REMOTE
+    if (External.Server) {
+      diag(Error, "Clangd isn't compiled with remote index support, ignoring "
+                  "Server." External.Server->Range);
+      External.Server.reset();
+    }
+#endif
+    // Make sure exactly one of the Sources is set.
+    unsigned SourceCount =
+        External.File.hasValue() + External.Server.hasValue();
+    if (SourceCount != 1) {
+      diag(Error, "Exactly one of File or Server must be set.", BlockRange);
+      return;
+    }
+    Config::ExternalIndexSpec Spec;
+    if (External.Server) {
+      Spec.Kind = Config::ExternalIndexSpec::Server;
+      Spec.Location = std::move(**External.Server);
+    } else if (External.File) {
+      Spec.Kind = Config::ExternalIndexSpec::File;
+      auto AbsPath = makeAbsolute(std::move(*External.File), "File",
+                                  llvm::sys::path::Style::native);
+      if (!AbsPath)
+        return;
+      Spec.Location = std::move(*AbsPath);
+    }
+    // Make sure MountPoint is an absolute path with forward slashes.
+    if (!External.MountPoint)
+      External.MountPoint.emplace(FragmentDirectory);
+    if ((**External.MountPoint).empty()) {
+      diag(Error, "A mountpoint is required.", BlockRange);
+      return;
+    }
+    auto AbsPath = makeAbsolute(std::move(*External.MountPoint), "MountPoint",
+                                llvm::sys::path::Style::posix);
+    if (!AbsPath)
+      return;
+    Spec.MountPoint = std::move(*AbsPath);
+    Out.Apply.push_back([Spec(std::move(Spec))](const Params &P, Config &C) {
+      if (!P.Path.startswith(Spec.MountPoint))
+        return;
+      C.Index.External = Spec;
+      // Disable background indexing for the files under the mountpoint.
+      // Note that this will overwrite statements in any previous fragments
+      // (including the current one).
+      C.Index.Background = Config::BackgroundPolicy::Skip;
+    });
   }
 
   void compile(Fragment::StyleBlock &&F) {

diff  --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index 6e535f3bea97..9e7fc5a49868 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -9,9 +9,13 @@
 #include "Config.h"
 #include "ConfigFragment.h"
 #include "ConfigTesting.h"
+#include "Features.inc"
 #include "TestFS.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/SourceMgr.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <string>
@@ -20,6 +24,8 @@ namespace clang {
 namespace clangd {
 namespace config {
 namespace {
+using ::testing::AllOf;
+using ::testing::Contains;
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
 using ::testing::SizeIs;
@@ -196,6 +202,104 @@ TEST_F(ConfigCompileTests, Tidy) {
             "0");
 }
 
+TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) {
+  Fragment::IndexBlock::ExternalBlock External;
+  External.File.emplace("");
+  External.Server.emplace("");
+  Frag.Index.External = std::move(External);
+  compileAndApply();
+  llvm::StringLiteral ExpectedDiag =
+#ifdef CLANGD_ENABLE_REMOTE
+      "Exactly one of File or Server must be set.";
+#else
+      "Clangd isn't compiled with remote index support, ignoring Server.";
+#endif
+  EXPECT_THAT(Diags.Diagnostics,
+              Contains(AllOf(DiagMessage(ExpectedDiag),
+                             DiagKind(llvm::SourceMgr::DK_Error))));
+}
+
+TEST_F(ConfigCompileTests, ExternalBlockErrOnNoSource) {
+  Frag.Index.External.emplace(Fragment::IndexBlock::ExternalBlock{});
+  compileAndApply();
+  EXPECT_THAT(
+      Diags.Diagnostics,
+      Contains(AllOf(DiagMessage("Exactly one of File or Server must be set."),
+                     DiagKind(llvm::SourceMgr::DK_Error))));
+}
+
+TEST_F(ConfigCompileTests, ExternalBlockDisablesBackgroundIndex) {
+  Parm.Path = "/foo/bar/baz.h";
+  Frag.Index.Background.emplace("Build");
+  Fragment::IndexBlock::ExternalBlock External;
+  External.File.emplace("/foo");
+  External.MountPoint.emplace("/foo/bar");
+  Frag.Index.External = std::move(External);
+  compileAndApply();
+  EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Skip);
+}
+
+TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
+  auto GetFrag = [](llvm::StringRef Directory,
+                    llvm::Optional<const char *> MountPoint) {
+    Fragment Frag;
+    Frag.Source.Directory = Directory.str();
+    Fragment::IndexBlock::ExternalBlock External;
+    External.File.emplace("/foo");
+    if (MountPoint)
+      External.MountPoint.emplace(*MountPoint);
+    Frag.Index.External = std::move(External);
+    return Frag;
+  };
+
+  Parm.Path = "/foo/bar.h";
+  // Non-absolute MountPoint without a directory raises an error.
+  Frag = GetFrag("", "foo");
+  compileAndApply();
+  ASSERT_THAT(
+      Diags.Diagnostics,
+      ElementsAre(
+          AllOf(DiagMessage("MountPoint must be an absolute path, because this "
+                            "fragment is not associated with any directory."),
+                DiagKind(llvm::SourceMgr::DK_Error))));
+  ASSERT_FALSE(Conf.Index.External);
+
+  // Ok when relative.
+  Frag = GetFrag("/", "foo");
+  compileAndApply();
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_TRUE(Conf.Index.External);
+  EXPECT_THAT(Conf.Index.External->MountPoint, "/foo");
+
+  // None defaults to ".".
+  Frag = GetFrag("/", llvm::None);
+  compileAndApply();
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_TRUE(Conf.Index.External);
+  EXPECT_THAT(Conf.Index.External->MountPoint, "/");
+
+  // Without a file, external index is empty.
+  Parm.Path = "";
+  Frag = GetFrag("", "/foo");
+  compileAndApply();
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_FALSE(Conf.Index.External);
+
+  // File outside MountPoint, no index.
+  Parm.Path = "/bar/baz.h";
+  Frag = GetFrag("", "/foo");
+  compileAndApply();
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_FALSE(Conf.Index.External);
+
+  // File under MountPoint, index should be set.
+  Parm.Path = "/foo/baz.h";
+  Frag = GetFrag("", "/foo");
+  compileAndApply();
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_TRUE(Conf.Index.External);
+  EXPECT_THAT(Conf.Index.External->MountPoint, "/foo");
+}
 } // namespace
 } // namespace config
 } // namespace clangd


        


More information about the llvm-branch-commits mailing list