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.
This introduces a mechanism for providers to interpret paths specified
in a fragment either as absolute or relative to fragment location.
This information should be used during compile stage to handle blocks correctly.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D90270
Files:
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigProvider.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
Index: clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
@@ -10,6 +10,7 @@
#include "ConfigProvider.h"
#include "ConfigTesting.h"
#include "TestFS.h"
+#include "llvm/Support/Path.h"
#include "llvm/Support/SourceMgr.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -187,6 +188,36 @@
EXPECT_THAT(getAddedArgs(Cfg), IsEmpty());
}
+TEST(ProviderTest, SourceInfo) {
+ MockFS FS;
+
+ FS.Files["baz/foo.yaml"] = R"yaml(
+If:
+ PathMatch: .*
+ PathExclude: bar.h
+CompileFlags:
+ Add: bar
+)yaml";
+ const auto BarPath = testPath("baz/bar.h", llvm::sys::path::Style::posix);
+ CapturedDiags Diags;
+ Params Bar;
+ Bar.Path = BarPath;
+
+ // This should be an absolute match/exclude hence baz/bar.h should not be
+ // excluded.
+ auto P = Provider::fromYAMLFile(testPath("baz/foo.yaml"), FS);
+ auto Cfg = P->getConfig(Bar, Diags.callback());
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("bar"));
+ Diags.Diagnostics.clear();
+
+ // This should be a relative match/exclude hence baz/bar.h should be excluded.
+ P = Provider::fromAncestorRelativeYAMLFiles("foo.yaml", FS);
+ Cfg = P->getConfig(Bar, Diags.callback());
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ EXPECT_THAT(getAddedArgs(Cfg), IsEmpty());
+ Diags.Diagnostics.clear();
+}
} // namespace
} // namespace config
} // namespace clangd
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -116,6 +116,59 @@
"Invalid Background value 'Foo'. Valid values are Build, Skip.")));
}
+TEST_F(ConfigCompileTests, PathSpecMatch) {
+ const Fragment BaseFrag = [] {
+ Fragment Frag;
+ Frag.If.PathMatch.emplace_back(".*");
+ Frag.Source.FragmentDirectory = "/baz";
+ return Frag;
+ }();
+
+ Parm.Path = "/foo/bar.h";
+ // * matches everything with absolute spec.
+ Frag = BaseFrag;
+ Frag.Source.PathSpec = Fragment::SourceInfo::Absolute;
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ EXPECT_TRUE(compileAndApply());
+ // Relative should fail to match as /foo/bar.h doesn't reside under /baz/.
+ Frag = BaseFrag;
+ Frag.Source.PathSpec = Fragment::SourceInfo::Relative;
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ EXPECT_FALSE(compileAndApply());
+ // Relative should pass now.
+ Frag = BaseFrag;
+ Parm.Path = "/baz/anything";
+ Frag.Source.PathSpec = Fragment::SourceInfo::Relative;
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ EXPECT_TRUE(compileAndApply());
+}
+
+TEST_F(ConfigCompileTests, PathSpecExclude) {
+ const Fragment BaseFrag = [] {
+ Fragment Frag;
+ Frag.If.PathExclude.emplace_back(".*");
+ Frag.Source.FragmentDirectory = "/baz";
+ return Frag;
+ }();
+
+ Parm.Path = "/foo/bar.h";
+ // * matches everything with absolute spec.
+ Frag = BaseFrag;
+ Frag.Source.PathSpec = Fragment::SourceInfo::Absolute;
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ EXPECT_FALSE(compileAndApply());
+ // Relative should fail to match as /foo/bar.h doesn't reside under /baz/.
+ Frag = BaseFrag;
+ Frag.Source.PathSpec = Fragment::SourceInfo::Relative;
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ EXPECT_TRUE(compileAndApply());
+ // Relative should pass now.
+ Frag = BaseFrag;
+ Parm.Path = "/baz/anything";
+ Frag.Source.PathSpec = Fragment::SourceInfo::Relative;
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ EXPECT_FALSE(compileAndApply());
+}
} // namespace
} // namespace config
} // namespace clangd
Index: clang-tools-extra/clangd/ConfigProvider.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigProvider.cpp
+++ clang-tools-extra/clangd/ConfigProvider.cpp
@@ -13,6 +13,7 @@
#include "support/Trace.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Path.h"
#include <chrono>
#include <mutex>
@@ -31,7 +32,8 @@
// Called once we are sure we want to read the file.
// REQUIRES: Cache keys are set. Mutex must be held.
- void fillCacheFromDisk(llvm::vfs::FileSystem &FS, DiagnosticCallback DC) {
+ void fillCacheFromDisk(llvm::vfs::FileSystem &FS, DiagnosticCallback DC,
+ Fragment::SourceInfo::PathSpecification PathSpec) {
CachedValue.clear();
auto Buf = FS.getBufferForFile(Path);
@@ -49,10 +51,15 @@
MTime = {};
}
+ llvm::StringRef FragmentDir = llvm::sys::path::parent_path(Path);
+ FragmentDir.consume_back("/");
// Finally parse and compile the actual fragments.
for (auto &Fragment :
- Fragment::parseYAML(Buf->get()->getBuffer(), Path, DC))
+ Fragment::parseYAML(Buf->get()->getBuffer(), Path, DC)) {
+ Fragment.Source.FragmentDirectory = FragmentDir.str();
+ Fragment.Source.PathSpec = PathSpec;
CachedValue.push_back(std::move(Fragment).compile(DC));
+ }
}
public:
@@ -68,6 +75,7 @@
// - allow latency-sensitive operations to skip revalidating the cache
void read(const ThreadsafeFS &TFS, DiagnosticCallback DC,
llvm::Optional<std::chrono::steady_clock::time_point> FreshTime,
+ Fragment::SourceInfo::PathSpecification PathSpec,
std::vector<CompiledFragment> &Out) {
std::lock_guard<std::mutex> Lock(Mu);
// We're going to update the cache and return whatever's in it.
@@ -100,7 +108,7 @@
// OK, the file has actually changed. Update cache key, compute new value.
Size = Stat->getSize();
MTime = Stat->getLastModificationTime();
- fillCacheFromDisk(*FS, DC);
+ fillCacheFromDisk(*FS, DC, PathSpec);
}
};
@@ -113,7 +121,7 @@
std::vector<CompiledFragment>
getFragments(const Params &P, DiagnosticCallback DC) const override {
std::vector<CompiledFragment> Result;
- Cache.read(FS, DC, P.FreshTime, Result);
+ Cache.read(FS, DC, P.FreshTime, Fragment::SourceInfo::Absolute, Result);
return Result;
};
@@ -177,8 +185,10 @@
// Finally query each individual file.
// This will take a (per-file) lock for each file that actually exists.
std::vector<CompiledFragment> Result;
- for (FileConfigCache *Cache : Caches)
- Cache->read(FS, DC, P.FreshTime, Result);
+ for (FileConfigCache *Cache : Caches) {
+ Cache->read(FS, DC, P.FreshTime, Fragment::SourceInfo::Relative,
+ Result);
+ }
return Result;
};
Index: clang-tools-extra/clangd/ConfigFragment.h
===================================================================
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -91,6 +91,14 @@
/// The start of the original source for this fragment.
/// Only valid if SourceManager is set.
llvm::SMLoc Location;
+ /// Absolute path to directory containing the fragment. Doesn't contain the
+ /// trailing slash.
+ std::string FragmentDirectory;
+ /// Describes how paths specified in this fragment should be treated.
+ enum PathSpecification {
+ Absolute,
+ Relative,
+ } PathSpec = Absolute;
};
SourceInfo Source;
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -70,6 +70,7 @@
CompiledFragmentImpl &Out;
DiagnosticCallback Diagnostic;
llvm::SourceMgr *SourceMgr;
+ Fragment::SourceInfo &FragmentSource;
llvm::Optional<llvm::Regex> compileRegex(const Located<std::string> &Text) {
std::string Anchored = "^(" + *Text + ")$";
@@ -145,11 +146,19 @@
}
if (!PathMatch->empty()) {
Out.Conditions.push_back(
- [PathMatch(std::move(PathMatch))](const Params &P) {
+ [PathMatch(std::move(PathMatch)), PathSpec(FragmentSource.PathSpec),
+ FragmentDir(FragmentSource.FragmentDirectory)](const Params &P) {
if (P.Path.empty())
return false;
+ llvm::StringRef Path = P.Path;
+ // In relative mode ignore paths rooted outside FragmentDirectory
+ // and strip the prefix for relative matching.
+ if (PathSpec == Fragment::SourceInfo::Relative &&
+ !Path.consume_front(FragmentDir)) {
+ return false;
+ }
return llvm::any_of(*PathMatch, [&](const llvm::Regex &RE) {
- return RE.match(P.Path);
+ return RE.match(Path);
});
});
}
@@ -161,11 +170,21 @@
}
if (!PathExclude->empty()) {
Out.Conditions.push_back(
- [PathExclude(std::move(PathExclude))](const Params &P) {
+ [PathExclude(std::move(PathExclude)),
+ PathSpec(FragmentSource.PathSpec),
+ FragmentDir(FragmentSource.FragmentDirectory)](const Params &P) {
if (P.Path.empty())
return false;
+ llvm::StringRef Path = P.Path;
+ // In relative mode ignore paths rooted outside FragmentDirectory
+ // and strip the prefix for relative matching.
+ if (PathSpec == Fragment::SourceInfo::Relative) {
+ if (!Path.consume_front(FragmentDir))
+ return true;
+ Path.consume_front("/");
+ }
return llvm::none_of(*PathExclude, [&](const llvm::Regex &RE) {
- return RE.match(P.Path);
+ return RE.match(Path);
});
});
}
@@ -255,7 +274,8 @@
vlog("Config fragment: compiling {0}:{1} -> {2}", ConfigFile, LineCol.first,
Result.get());
- FragmentCompiler{*Result, D, Source.Manager.get()}.compile(std::move(*this));
+ FragmentCompiler{*Result, D, Source.Manager.get(), Source}.compile(
+ std::move(*this));
// Return as cheaply-copyable wrapper.
return [Result(std::move(Result))](const Params &P, Config &C) {
return (*Result)(P, C);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
