kadircet created this revision.
Herald added subscribers: usaxena95, arphaman.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.
Path{Match,Exclude} and MountPoint were checking paths case-sensitively
on all platforms, as with other features, this was causing problems on
windows. Since users can have capital drive letters on config files, but
editors might lower-case them.
This patch addresses that issue by:
- Normalizing all the paths being passed to ConfigProvider in ClangdServer.
- Storing MountPoint and regexes used for pathmatching in case-folded manner.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D96690
Files:
clang-tools-extra/clangd/ClangdServer.cpp
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
@@ -99,6 +99,19 @@
Frag.If.PathMatch.emplace_back("ba*r");
EXPECT_FALSE(compileAndApply());
EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+
+#if defined(_WIN32) || defined(__APPLE__)
+ // Only matches case-insensitively.
+ Frag = {};
+ Frag.If.PathMatch.emplace_back("B*R");
+ EXPECT_TRUE(compileAndApply());
+ EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+
+ Frag = {};
+ Frag.If.PathExclude.emplace_back("B*R");
+ EXPECT_FALSE(compileAndApply());
+ EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+#endif
}
TEST_F(ConfigCompileTests, CompileCommands) {
@@ -372,14 +385,14 @@
compileAndApply();
ASSERT_THAT(Diags.Diagnostics, IsEmpty());
ASSERT_TRUE(Conf.Index.External);
- EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+ EXPECT_TRUE(pathEqual(Conf.Index.External->MountPoint, FooPath));
// None defaults to ".".
Frag = GetFrag(FooPath, llvm::None);
compileAndApply();
ASSERT_THAT(Diags.Diagnostics, IsEmpty());
ASSERT_TRUE(Conf.Index.External);
- EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+ EXPECT_TRUE(pathEqual(Conf.Index.External->MountPoint, FooPath));
// Without a file, external index is empty.
Parm.Path = "";
@@ -405,7 +418,21 @@
compileAndApply();
ASSERT_THAT(Diags.Diagnostics, IsEmpty());
ASSERT_TRUE(Conf.Index.External);
- EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+ EXPECT_TRUE(pathEqual(Conf.Index.External->MountPoint, FooPath));
+
+#if defined(_WIN32) || defined(__APPLE__)
+ // Only matches case-insensitively.
+ BazPath = testPath("fOo/baz.h", llvm::sys::path::Style::posix);
+ BazPath = llvm::sys::path::convert_to_slash(BazPath);
+ Parm.Path = BazPath;
+
+ FooPath = testPath("FOO/", llvm::sys::path::Style::posix);
+ Frag = GetFrag("", FooPath.c_str());
+ compileAndApply();
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ ASSERT_TRUE(Conf.Index.External);
+ EXPECT_TRUE(pathEqual(Conf.Index.External->MountPoint, FooPath));
+#endif
}
} // namespace
} // namespace config
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -31,6 +31,7 @@
#include "Features.inc"
#include "TidyProvider.h"
#include "support/Logger.h"
+#include "support/Path.h"
#include "support/Trace.h"
#include "llvm/ADT/None.h"
#include "llvm/ADT/Optional.h"
@@ -101,9 +102,11 @@
// Normalized Fragment::SourceInfo::Directory.
std::string FragmentDirectory;
- llvm::Optional<llvm::Regex> compileRegex(const Located<std::string> &Text) {
+ llvm::Optional<llvm::Regex>
+ compileRegex(const Located<std::string> &Text,
+ llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags) {
std::string Anchored = "^(" + *Text + ")$";
- llvm::Regex Result(Anchored);
+ llvm::Regex Result(Anchored, Flags);
std::string RegexError;
if (!Result.isValid(RegexError)) {
diag(Error, "Invalid regex " + Anchored + ": " + RegexError, Text.Range);
@@ -184,6 +187,7 @@
FragmentDirectory = llvm::sys::path::convert_to_slash(F.Source.Directory);
if (FragmentDirectory.back() != '/')
FragmentDirectory += '/';
+ FragmentDirectory = maybeCaseFoldPath(FragmentDirectory);
}
compile(std::move(F.If));
compile(std::move(F.CompileFlags));
@@ -195,9 +199,17 @@
if (F.HasUnrecognizedCondition)
Out.Conditions.push_back([&](const Params &) { return false; });
+#if defined(_WIN32) || defined(__APPLE__)
+ // When matching paths via regex on Windows/Apple they should be treated
+ // case insensitively.
+ llvm::Regex::RegexFlags Flags = llvm::Regex::IgnoreCase;
+#else
+ llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
+#endif
+
auto PathMatch = std::make_unique<std::vector<llvm::Regex>>();
for (auto &Entry : F.PathMatch) {
- if (auto RE = compileRegex(Entry))
+ if (auto RE = compileRegex(Entry, Flags))
PathMatch->push_back(std::move(*RE));
}
if (!PathMatch->empty()) {
@@ -218,7 +230,7 @@
auto PathExclude = std::make_unique<std::vector<llvm::Regex>>();
for (auto &Entry : F.PathExclude) {
- if (auto RE = compileRegex(Entry))
+ if (auto RE = compileRegex(Entry, Flags))
PathExclude->push_back(std::move(*RE));
}
if (!PathExclude->empty()) {
@@ -347,7 +359,7 @@
llvm::sys::path::Style::posix);
if (!AbsPath)
return;
- Spec.MountPoint = std::move(*AbsPath);
+ Spec.MountPoint = maybeCaseFoldPath(std::move(*AbsPath));
Out.Apply.push_back([Spec(std::move(Spec))](const Params &P, Config &C) {
if (!P.Path.startswith(Spec.MountPoint))
return;
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -30,6 +30,7 @@
#include "support/Logger.h"
#include "support/Markup.h"
#include "support/MemoryTree.h"
+#include "support/Path.h"
#include "support/ThreadsafeFS.h"
#include "support/Trace.h"
#include "clang/Format/Format.h"
@@ -213,6 +214,7 @@
if (!File.empty()) {
assert(llvm::sys::path::is_absolute(File));
llvm::sys::path::native(File, PosixPath, llvm::sys::path::Style::posix);
+ PosixPath = maybeCaseFoldPath(PosixPath);
Params.Path = PosixPath.str();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
