[clang-tools-extra] c9776c8 - [clangd] Introduce config compilation for External blocks
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Sun Nov 22 12:13:18 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 cfe-commits
mailing list