kadircet updated this revision to Diff 303956.
kadircet added a comment.
- Define an ExternalIndexSpec structure, and populate that during compile.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90749/new/
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
@@ -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 clangd {
namespace config {
namespace {
+using ::testing::AllOf;
+using ::testing::Contains;
using ::testing::ElementsAre;
using ::testing::IsEmpty;
using ::testing::SizeIs;
@@ -176,6 +182,105 @@
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();
+ 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();
+ 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
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -27,11 +27,14 @@
#include "Config.h"
#include "ConfigFragment.h"
#include "ConfigProvider.h"
+#include "Features.inc"
#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/Error.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Regex.h"
#include "llvm/Support/SMLoc.h"
@@ -242,6 +245,84 @@
Out.Apply.push_back(
[Val](const Params &, Config &C) { C.Index.Background = *Val; });
}
+ if (F.External)
+ compile(std::move(*F.External));
+ }
+
+ llvm::Expected<std::string> makeAbsolute(std::string Path,
+ llvm::StringRef Base,
+ llvm::StringLiteral Description,
+ llvm::sys::path::Style Style) {
+ if (llvm::sys::path::is_absolute(Path))
+ return Path;
+ if (Base.empty())
+ return llvm::createStringError(
+ llvm::inconvertibleErrorCode(),
+ "%s must be an absolute path, because this fragment is not "
+ "associated with any directory.",
+ Description.data());
+ llvm::SmallString<256> AbsPath;
+ llvm::sys::path::append(AbsPath, FragmentDirectory, Path);
+ llvm::sys::path::native(AbsPath, Style);
+ return AbsPath.str().str();
+ }
+
+ void compile(Fragment::IndexBlock::ExternalBlock &&External) {
+#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.",
+ llvm::SMRange());
+ 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;
+ // Make sure File is an absolute native path.
+ auto AbsPath = makeAbsolute(std::move(**External.File), FragmentDirectory,
+ "File", llvm::sys::path::Style::native);
+ if (!AbsPath) {
+ diag(Error, llvm::toString(AbsPath.takeError()), External.File->Range);
+ 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.", llvm::SMRange());
+ return;
+ }
+ auto AbsPath =
+ makeAbsolute(std::move(**External.MountPoint), FragmentDirectory,
+ "MountPoint", llvm::sys::path::Style::posix);
+ if (!AbsPath) {
+ diag(Error, llvm::toString(AbsPath.takeError()),
+ External.MountPoint->Range);
+ 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) {
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>
@@ -57,10 +58,22 @@
} 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.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
