kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman.
Herald added a project: clang.
kadircet requested review of this revision.
Herald added subscribers: ormris, MaskRay, ilya-biryukov.
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 <https://reviews.llvm.org/D90748>.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D90749
Files:
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -10,8 +10,11 @@
#include "ConfigFragment.h"
#include "ConfigTesting.h"
#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 +23,8 @@
namespace clangd {
namespace config {
namespace {
+using ::testing::AllOf;
+using ::testing::Contains;
using ::testing::ElementsAre;
using ::testing::IsEmpty;
using ::testing::SizeIs;
@@ -176,6 +181,97 @@
ASSERT_THAT(Diags.Diagnostics, IsEmpty());
}
}
+
+TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) {
+ Fragment::IndexBlock::ExternalBlock External;
+ External.File.emplace("");
+ External.Server.emplace("");
+ Frag.Index.External = std::move(External);
+ compileAndApply();
+ EXPECT_THAT(
+ Diags.Diagnostics,
+ Contains(
+ AllOf(DiagMessage("Both File and Server specified, ignoring Server."),
+ DiagKind(llvm::SourceMgr::DK_Warning))));
+}
+
+TEST_F(ConfigCompileTests, ExternalBlockErrOnNoSource) {
+ Frag.Index.External.emplace();
+ compileAndApply();
+ EXPECT_THAT(
+ Diags.Diagnostics,
+ Contains(AllOf(
+ DiagMessage("At least one of File or Server must be specified."),
+ 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."),
+ DiagKind(llvm::SourceMgr::DK_Error))));
+ EXPECT_THAT(Conf.Index.External.MountPoint, IsEmpty());
+
+ // Ok when relative.
+ Frag = GetFrag("/", "foo");
+ compileAndApply();
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ EXPECT_THAT(Conf.Index.External.MountPoint, "/foo");
+
+ // None defaults to ".".
+ Frag = GetFrag("/", llvm::None);
+ compileAndApply();
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ EXPECT_THAT(Conf.Index.External.MountPoint, "/");
+
+ // Without a file, external index is empty.
+ Parm.Path = "";
+ Frag = GetFrag("", "/foo");
+ compileAndApply();
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ EXPECT_THAT(Conf.Index.External.MountPoint, IsEmpty());
+
+ // File outside MountPoint, no index.
+ Parm.Path = "/bar/baz.h";
+ Frag = GetFrag("", "/foo");
+ compileAndApply();
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ EXPECT_THAT(Conf.Index.External.MountPoint, IsEmpty());
+
+ // File under MountPoint, index should be set.
+ Parm.Path = "/foo/baz.h";
+ Frag = GetFrag("", "/foo");
+ compileAndApply();
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ EXPECT_THAT(Conf.Index.External.MountPoint, "/foo");
+}
} // namespace
} // namespace config
} // namespace clangd
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -30,6 +30,7 @@
#include "support/Logger.h"
#include "support/Trace.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/Support/Path.h"
@@ -242,6 +243,70 @@
Out.Apply.push_back(
[Val](const Params &, Config &C) { C.Index.Background = *Val; });
}
+ if (F.External)
+ compile(std::move(*F.External));
+ }
+
+ void compile(Fragment::IndexBlock::ExternalBlock &&External) {
+ llvm::SmallString<256> AbsPath;
+ // Make sure exactly one of File or Server is set, and File is an absolute
+ // native path.
+ if (!External.File && !External.Server) {
+ diag(Error, "At least one of File or Server must be specified.",
+ llvm::SMRange());
+ return;
+ }
+ if (External.File && External.Server) {
+ diag(Warning, "Both File and Server specified, ignoring Server.",
+ External.Server->Range);
+ External.Server.reset();
+ }
+ if (External.File && !llvm::sys::path::is_absolute(**External.File)) {
+ if (FragmentDirectory.empty()) {
+ diag(Error, "File must be an absolute path.", External.File->Range);
+ return;
+ }
+ AbsPath.clear();
+ llvm::sys::path::append(AbsPath, FragmentDirectory, **External.File);
+ External.File.emplace(AbsPath.str().str());
+ }
+ // 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.", llvm::SMRange());
+ return;
+ }
+ if (!llvm::sys::path::is_absolute(**External.MountPoint,
+ llvm::sys::path::Style::posix)) {
+ if (FragmentDirectory.empty()) {
+ diag(Error, "MountPoint must be an absolute path.",
+ External.MountPoint->Range);
+ return;
+ }
+ AbsPath.clear();
+ llvm::sys::path::append(AbsPath, llvm::sys::path::Style::posix,
+ FragmentDirectory, **External.MountPoint);
+ External.MountPoint.emplace(AbsPath.str().str());
+ }
+ Out.Apply.push_back(
+ [External(std::move(External))](const Params &P, Config &C) {
+ if (!P.Path.startswith(**External.MountPoint))
+ return;
+ C.Index.External.File.reset();
+ C.Index.External.Server.reset();
+
+ C.Index.External.MountPoint = std::move(**External.MountPoint);
+ if (External.File)
+ C.Index.External.File.emplace(std::move(**External.File));
+ else
+ C.Index.External.Server.emplace(std::move(**External.Server));
+
+ // 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) {
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ 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 <string>
#include <vector>
@@ -61,6 +62,17 @@
struct {
/// Whether this TU should be indexed.
BackgroundPolicy Background = BackgroundPolicy::Build;
+ /// Describes an external index configuration. At most one of Server or File
+ /// will be set at a time.
+ struct {
+ /// Address of a remote-index-server, in the form of "ip:port".
+ llvm::Optional<std::string> Server;
+ /// Absolute path to a monolithic index.
+ llvm::Optional<std::string> File;
+ /// Absolute path to source root this index is associated with. Empty
+ /// means external index isn't configured.
+ std::string MountPoint;
+ } External;
} Index;
/// Style of the codebase.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
