sammccall updated this revision to Diff 320352.
sammccall added a comment.
remove debug printfs
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95759/new/
https://reviews.llvm.org/D95759
Files:
clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/support/CMakeLists.txt
clang-tools-extra/clangd/support/Path.cpp
clang-tools-extra/clangd/support/Path.h
clang-tools-extra/clangd/unittests/RenameTests.cpp
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1067,6 +1067,52 @@
}
}
+MATCHER_P(newText, T, "") { return arg.newText == T; }
+
+TEST(RenameTest, IndexMergeMainFile) {
+ Annotations Code("int ^x();");
+ TestTU TU = TestTU::withCode(Code.code());
+ TU.Filename = "main.cc";
+ auto AST = TU.build();
+
+ auto Main = testPath("main.cc");
+
+ auto Rename = [&](const SymbolIndex *Idx) {
+ auto GetDirtyBuffer = [&](PathRef Path) -> llvm::Optional<std::string> {
+ return Code.code().str(); // Every file has the same content.
+ };
+ RenameOptions Opts;
+ Opts.AllowCrossFile = true;
+ RenameInputs Inputs{Code.point(), "xPrime", AST, Main,
+ Idx, Opts, GetDirtyBuffer};
+ auto Results = rename(Inputs);
+ EXPECT_TRUE(bool(Results)) << llvm::toString(Results.takeError());
+ return std::move(*Results);
+ };
+
+ // We do not expect to see duplicated edits from AST vs index.
+ auto Results = Rename(TU.index().get());
+ EXPECT_THAT(Results.GlobalChanges.keys(), ElementsAre(Main));
+ EXPECT_THAT(Results.GlobalChanges[Main].asTextEdits(),
+ ElementsAre(newText("xPrime")));
+
+ // Sanity check: we do expect to see index results!
+ TU.Filename = "other.cc";
+ Results = Rename(TU.index().get());
+ EXPECT_THAT(Results.GlobalChanges.keys(),
+ UnorderedElementsAre(Main, testPath("other.cc")));
+
+#if defined(_WIN32) || defined(__APPLE__)
+ // On case-insensitive systems, no duplicates if AST vs index case differs.
+ // https://github.com/clangd/clangd/issues/665
+ TU.Filename = "MAIN.CC";
+ Results = Rename(TU.index().get());
+ EXPECT_THAT(Results.GlobalChanges.keys(), ElementsAre(Main));
+ EXPECT_THAT(Results.GlobalChanges[Main].asTextEdits(),
+ ElementsAre(newText("xPrime")));
+#endif
+}
+
TEST(RenameTest, MainFileReferencesOnly) {
// filter out references not from main file.
llvm::StringRef Test =
@@ -1576,43 +1622,43 @@
llvm::StringRef IndexedCode;
llvm::StringRef DraftCode;
} Tests[] = {
- {
- // both line and column are changed, not a near miss.
- R"cpp(
+ {
+ // both line and column are changed, not a near miss.
+ R"cpp(
int [[x]] = 0;
)cpp",
- R"cpp(
+ R"cpp(
// insert a line.
double x = 0;
)cpp",
- },
- {
- // subset.
- R"cpp(
+ },
+ {
+ // subset.
+ R"cpp(
int [[x]] = 0;
)cpp",
- R"cpp(
+ R"cpp(
int [[x]] = 0;
{int x = 0; }
)cpp",
- },
- {
- // shift columns.
- R"cpp(int [[x]] = 0; void foo(int x);)cpp",
- R"cpp(double [[x]] = 0; void foo(double x);)cpp",
- },
- {
- // shift lines.
- R"cpp(
+ },
+ {
+ // shift columns.
+ R"cpp(int [[x]] = 0; void foo(int x);)cpp",
+ R"cpp(double [[x]] = 0; void foo(double x);)cpp",
+ },
+ {
+ // shift lines.
+ R"cpp(
int [[x]] = 0;
void foo(int x);
)cpp",
- R"cpp(
+ R"cpp(
// insert a line.
int [[x]] = 0;
void foo(int x);
)cpp",
- },
+ },
};
LangOptions LangOpts;
LangOpts.CPlusPlus = true;
@@ -1635,69 +1681,62 @@
struct {
llvm::StringRef IndexedCode;
llvm::StringRef LexedCode;
- } Tests[] = {
- {
- // no lexed ranges.
- "[[]]",
- "",
- },
- {
- // both line and column are changed, not a near miss.
- R"([[]])",
- R"(
+ } Tests[] = {{
+ // no lexed ranges.
+ "[[]]",
+ "",
+ },
+ {
+ // both line and column are changed, not a near miss.
+ R"([[]])",
+ R"(
[[]]
)",
- },
- {
- // subset.
- "[[]]",
- "^[[]] [[]]"
- },
- {
- // shift columns.
- "[[]] [[]]",
- " ^[[]] ^[[]] [[]]"
- },
- {
- R"(
+ },
+ {// subset.
+ "[[]]", "^[[]] [[]]"},
+ {// shift columns.
+ "[[]] [[]]", " ^[[]] ^[[]] [[]]"},
+ {
+ R"(
[[]]
[[]] [[]]
)",
- R"(
+ R"(
// insert a line
^[[]]
^[[]] ^[[]]
)",
- },
- {
- R"(
+ },
+ {
+ R"(
[[]]
[[]] [[]]
)",
- R"(
+ R"(
// insert a line
^[[]]
^[[]] ^[[]] // column is shifted.
)",
- },
- {
- R"(
+ },
+ {
+ R"(
[[]]
[[]] [[]]
)",
- R"(
+ R"(
// insert a line
[[]]
[[]] [[]] // not mapped (both line and column are changed).
)",
- },
- {
- R"(
+ },
+ {
+ R"(
[[]]
[[]]
@@ -1706,7 +1745,7 @@
}
)",
- R"(
+ R"(
// insert a new line
^[[]]
^[[]]
@@ -1715,22 +1754,21 @@
^[[]]
[[]] // additional range
)",
- },
- {
- // non-distinct result (two best results), not a near miss
- R"(
+ },
+ {
+ // non-distinct result (two best results), not a near miss
+ R"(
[[]]
[[]]
[[]]
)",
- R"(
+ R"(
[[]]
[[]]
[[]]
[[]]
)",
- }
- };
+ }};
for (const auto &T : Tests) {
SCOPED_TRACE(T.IndexedCode);
auto Lexed = Annotations(T.LexedCode);
Index: clang-tools-extra/clangd/support/Path.h
===================================================================
--- clang-tools-extra/clangd/support/Path.h
+++ clang-tools-extra/clangd/support/Path.h
@@ -22,6 +22,12 @@
/// signatures.
using PathRef = llvm::StringRef;
+// For platforms where paths are case-insensitive (but case-preserving),
+// we need to do case-insensitive comparisons and use lowercase keys.
+// FIXME: Make Path a real class with desired semantics instead.
+std::string maybeCaseFoldPath(PathRef Path);
+bool pathEqual(PathRef, PathRef);
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/support/Path.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/support/Path.cpp
@@ -0,0 +1,30 @@
+//===--- Path.cpp -------------------------------------------*- C++-*------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "support/Path.h"
+namespace clang {
+namespace clangd {
+
+std::string maybeCaseFoldPath(PathRef Path) {
+#if defined(_WIN32) || defined(__APPLE__)
+ return Path.lower();
+#else
+ return std::string(Path);
+#endif
+}
+
+bool pathEqual(PathRef A, PathRef B) {
+#if defined(_WIN32) || defined(__APPLE__)
+ return A.equals_lower(B);
+#else
+ return A == B;
+#endif
+}
+
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/support/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/support/CMakeLists.txt
+++ clang-tools-extra/clangd/support/CMakeLists.txt
@@ -23,6 +23,7 @@
Logger.cpp
Markup.cpp
MemoryTree.cpp
+ Path.cpp
Shutdown.cpp
Threading.cpp
ThreadsafeFS.cpp
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -68,7 +68,7 @@
if (OtherFile)
return;
if (auto RefFilePath = filePath(R.Location, /*HintFilePath=*/MainFile)) {
- if (*RefFilePath != MainFile)
+ if (!pathEqual(*RefFilePath, MainFile))
OtherFile = *RefFilePath;
}
});
@@ -474,7 +474,7 @@
if ((R.Kind & RefKind::Spelled) == RefKind::Unknown)
return;
if (auto RefFilePath = filePath(R.Location, /*HintFilePath=*/MainFile)) {
- if (*RefFilePath != MainFile)
+ if (!pathEqual(*RefFilePath, MainFile))
AffectedFiles[*RefFilePath].push_back(toRange(R.Location));
}
});
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===================================================================
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -395,20 +395,6 @@
return None;
}
-// For platforms where paths are case-insensitive (but case-preserving),
-// we need to do case-insensitive comparisons and use lowercase keys.
-// FIXME: Make Path a real class with desired semantics instead.
-// This class is not the only place this problem exists.
-// FIXME: Mac filesystems default to case-insensitive, but may be sensitive.
-
-static std::string maybeCaseFoldPath(PathRef Path) {
-#if defined(_WIN32) || defined(__APPLE__)
- return Path.lower();
-#else
- return std::string(Path);
-#endif
-}
-
std::vector<DirectoryBasedGlobalCompilationDatabase::DirectoryCache *>
DirectoryBasedGlobalCompilationDatabase::getDirectoryCaches(
llvm::ArrayRef<llvm::StringRef> Dirs) const {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
