Author: Kadir Cetinkaya Date: 2022-03-21T17:27:05+01:00 New Revision: 164a10dcf205e2b069f1f9a00361cd2252a2bcad
URL: https://github.com/llvm/llvm-project/commit/164a10dcf205e2b069f1f9a00361cd2252a2bcad DIFF: https://github.com/llvm/llvm-project/commit/164a10dcf205e2b069f1f9a00361cd2252a2bcad.diff LOG: [clangd] Test against path insensitivity Differential Revision: https://reviews.llvm.org/D121286 Added: Modified: clang-tools-extra/clangd/HeaderSourceSwitch.cpp clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp clang-tools-extra/clangd/unittests/TestFS.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/HeaderSourceSwitch.cpp b/clang-tools-extra/clangd/HeaderSourceSwitch.cpp index ed86c2eb0d12d..b5f8bc98a515f 100644 --- a/clang-tools-extra/clangd/HeaderSourceSwitch.cpp +++ b/clang-tools-extra/clangd/HeaderSourceSwitch.cpp @@ -11,6 +11,7 @@ #include "SourceCode.h" #include "index/SymbolCollector.h" #include "support/Logger.h" +#include "support/Path.h" #include "clang/AST/Decl.h" namespace clang { @@ -82,7 +83,7 @@ llvm::Optional<Path> getCorrespondingHeaderOrSource(PathRef OriginalFile, llvm::StringMap<int> Candidates; // Target path => score. auto AwardTarget = [&](const char *TargetURI) { if (auto TargetPath = URI::resolve(TargetURI, OriginalFile)) { - if (*TargetPath != OriginalFile) // exclude the original file. + if (!pathEqual(*TargetPath, OriginalFile)) // exclude the original file. ++Candidates[*TargetPath]; } else { elog("Failed to resolve URI {0}: {1}", TargetURI, TargetPath.takeError()); diff --git a/clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp b/clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp index bebf59ed822ca..b2d7a7ee77240 100644 --- a/clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp +++ b/clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp @@ -12,7 +12,9 @@ #include "TestFS.h" #include "TestTU.h" #include "index/MemIndex.h" +#include "support/Path.h" #include "llvm/ADT/None.h" +#include "llvm/Testing/Support/SupportHelpers.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -271,6 +273,43 @@ TEST(HeaderSourceSwitchTest, ClangdServerIntegration) { *llvm::cantFail(runSwitchHeaderSource(Server, CppPath))); } +TEST(HeaderSourceSwitchTest, CaseSensitivity) { + TestTU TU = TestTU::withCode("void foo() {}"); + // Define more symbols in the header than the source file to trick heuristics + // into picking the header as source file, if the matching for header file + // path fails. + TU.HeaderCode = R"cpp( + inline void bar1() {} + inline void bar2() {} + void foo();)cpp"; + // Give main file and header diff erent base names to make sure file system + // heuristics don't work. + TU.Filename = "Source.cpp"; + TU.HeaderFilename = "Header.h"; + + auto Index = TU.index(); + TU.Code = std::move(TU.HeaderCode); + TU.HeaderCode.clear(); + auto AST = TU.build(); + + // Provide a diff erent-cased filename in the query than what we have in the + // index, check if we can still find the source file, which defines less + // symbols than the header. + auto HeaderAbsPath = testPath("HEADER.H"); + // We expect the heuristics to pick: + // - header on case sensitive file systems, because the HeaderAbsPath doesn't + // match what we've seen through index. + // - source on case insensitive file systems, as the HeaderAbsPath would match + // the filename in index. +#ifdef CLANGD_PATH_CASE_INSENSITIVE + EXPECT_THAT(getCorrespondingHeaderOrSource(HeaderAbsPath, AST, Index.get()), + llvm::ValueIs(testing::StrCaseEq(testPath(TU.Filename)))); +#else + EXPECT_THAT(getCorrespondingHeaderOrSource(HeaderAbsPath, AST, Index.get()), + llvm::ValueIs(testing::StrCaseEq(testPath(TU.HeaderFilename)))); +#endif +} + } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/TestFS.cpp b/clang-tools-extra/clangd/unittests/TestFS.cpp index 5fd86122e78b9..80745e488bfa0 100644 --- a/clang-tools-extra/clangd/unittests/TestFS.cpp +++ b/clang-tools-extra/clangd/unittests/TestFS.cpp @@ -18,6 +18,18 @@ namespace clang { namespace clangd { +namespace { + +// Tries to strip \p Prefix from beginning of \p Path. Returns true on success. +// If \p Prefix doesn't match, leaves \p Path untouched and returns false. +bool pathConsumeFront(PathRef &Path, PathRef Prefix) { + if (!pathStartsWith(Prefix, Path)) + return false; + Path = Path.drop_front(Prefix.size()); + return true; +} +} // namespace + llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> buildTestFS(llvm::StringMap<std::string> const &Files, llvm::StringMap<time_t> const &Timestamps) { @@ -99,7 +111,7 @@ class TestScheme : public URIScheme { llvm::Expected<std::string> getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body, llvm::StringRef HintPath) const override { - if (!HintPath.empty() && !HintPath.startswith(testRoot())) + if (!HintPath.empty() && !pathStartsWith(testRoot(), HintPath)) return error("Hint path is not empty and doesn't start with {0}: {1}", testRoot(), HintPath); if (!Body.consume_front("/")) @@ -111,12 +123,11 @@ class TestScheme : public URIScheme { llvm::Expected<URI> uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override { - llvm::StringRef Body = AbsolutePath; - if (!Body.consume_front(testRoot())) + if (!pathConsumeFront(AbsolutePath, testRoot())) return error("{0} does not start with {1}", AbsolutePath, testRoot()); return URI(Scheme, /*Authority=*/"", - llvm::sys::path::convert_to_slash(Body)); + llvm::sys::path::convert_to_slash(AbsolutePath)); } }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits