maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[clangd] Use Dirty Filesystem for cross file rename. 

Refactor cross file rename to use a Filesystem instead of a function for getting buffer contents of open files.

Depends on D94554

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D95043
This commit is contained in:
Nathan James 2021-03-10 13:41:27 +00:00
parent 25951c5ab8
commit 7044f1d875
No known key found for this signature in database
GPG Key ID: CC007AFCDA90AA5F
7 changed files with 68 additions and 98 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
clang-tools-extra/clangd/ClangdServer.cpp
View File

@ -490,9 +490,10 @@ void ClangdServer::prepareRename(PathRef File, Position Pos,
return CB(InpAST.takeError());
// prepareRename is latency-sensitive: we don't query the index, as we
// only need main-file references
auto Results = clangd::rename(
{Pos, NewName.getValueOr("__clangd_rename_dummy"), InpAST->AST, File,
/*Index=*/nullptr, RenameOpts});
auto Results =
clangd::rename({Pos, NewName.getValueOr("__clangd_rename_dummy"),
InpAST->AST, File, /*FS=*/nullptr,
/*Index=*/nullptr, RenameOpts});
if (!Results) {
// LSP says to return null on failure, but that will result in a generic
// failure message. If we send an LSP error response, clients can surface
@ -507,25 +508,16 @@ void ClangdServer::prepareRename(PathRef File, Position Pos,
void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
const RenameOptions &Opts,
Callback<RenameResult> CB) {
// A snapshot of all file dirty buffers.
llvm::StringMap<std::string> Snapshot = WorkScheduler->getAllFileContents();
auto Action = [File = File.str(), NewName = NewName.str(), Pos, Opts,
CB = std::move(CB), Snapshot = std::move(Snapshot),
CB = std::move(CB),
this](llvm::Expected<InputsAndAST> InpAST) mutable {
// Tracks number of files edited per invocation.
static constexpr trace::Metric RenameFiles("rename_files",
trace::Metric::Distribution);
if (!InpAST)
return CB(InpAST.takeError());
auto GetDirtyBuffer =
[&Snapshot](PathRef AbsPath) -> llvm::Optional<std::string> {
auto It = Snapshot.find(AbsPath);
if (It == Snapshot.end())
return llvm::None;
return It->second;
};
auto R = clangd::rename(
{Pos, NewName, InpAST->AST, File, Index, Opts, GetDirtyBuffer});
auto R = clangd::rename({Pos, NewName, InpAST->AST, File,
DirtyFS->view(llvm::None), Index, Opts});
if (!R)
return CB(R.takeError());

1
clang-tools-extra/clangd/ClangdServer.h
View File

@ -394,7 +394,6 @@ private:
// Store of the current versions of the open documents.
// Only written from the main thread (despite being threadsafe).
// FIXME: TUScheduler also keeps these, unify?
DraftStore DraftMgr;
std::unique_ptr<ThreadsafeFS> DirtyFS;

7
clang-tools-extra/clangd/TUScheduler.cpp
View File

@ -1519,13 +1519,6 @@ void TUScheduler::remove(PathRef File) {
// preamble of several open headers.
}
llvm::StringMap<std::string> TUScheduler::getAllFileContents() const {
llvm::StringMap<std::string> Results;
for (auto &It : Files)
Results.try_emplace(It.getKey(), It.getValue()->Contents);
return Results;
}
void TUScheduler::run(llvm::StringRef Name, llvm::StringRef Path,
llvm::unique_function<void()> Action) {
runWithSemaphore(Name, Path, std::move(Action), Barrier);

3
clang-tools-extra/clangd/TUScheduler.h
View File

@ -235,9 +235,6 @@ public:
/// if requested with WantDiags::Auto or WantDiags::Yes.
void remove(PathRef File);
/// Returns a snapshot of all file buffer contents, per last update().
llvm::StringMap<std::string> getAllFileContents() const;
/// Schedule an async task with no dependencies.
/// Path may be empty (it is used only to set the Context).
void run(llvm::StringRef Name, llvm::StringRef Path,

42
clang-tools-extra/clangd/refactor/Rename.cpp
View File

@ -586,10 +586,10 @@ findOccurrencesOutsideFile(const NamedDecl &RenameDecl,
// index (background index) is relatively stale. We choose the dirty buffers
// as the file content we rename on, and fallback to file content on disk if
// there is no dirty buffer.
llvm::Expected<FileEdits> renameOutsideFile(
const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
llvm::StringRef NewName, const SymbolIndex &Index, size_t MaxLimitFiles,
llvm::function_ref<llvm::Expected<std::string>(PathRef)> GetFileContent) {
llvm::Expected<FileEdits>
renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
llvm::StringRef NewName, const SymbolIndex &Index,
size_t MaxLimitFiles, llvm::vfs::FileSystem &FS) {
trace::Span Tracer("RenameOutsideFile");
auto AffectedFiles = findOccurrencesOutsideFile(RenameDecl, MainFilePath,
Index, MaxLimitFiles);
@ -599,13 +599,16 @@ llvm::Expected<FileEdits> renameOutsideFile(
for (auto &FileAndOccurrences : *AffectedFiles) {
llvm::StringRef FilePath = FileAndOccurrences.first();
auto AffectedFileCode = GetFileContent(FilePath);
if (!AffectedFileCode) {
elog("Fail to read file content: {0}", AffectedFileCode.takeError());
auto ExpBuffer = FS.getBufferForFile(FilePath);
if (!ExpBuffer) {
elog("Fail to read file content: Fail to open file {0}: {1}", FilePath,
ExpBuffer.getError().message());
continue;
}
auto AffectedFileCode = (*ExpBuffer)->getBuffer();
auto RenameRanges =
adjustRenameRanges(*AffectedFileCode, RenameDecl.getNameAsString(),
adjustRenameRanges(AffectedFileCode, RenameDecl.getNameAsString(),
std::move(FileAndOccurrences.second),
RenameDecl.getASTContext().getLangOpts());
if (!RenameRanges) {
@ -617,7 +620,7 @@ llvm::Expected<FileEdits> renameOutsideFile(
FilePath);
}
auto RenameEdit =
buildRenameEdit(FilePath, *AffectedFileCode, *RenameRanges, NewName);
buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewName);
if (!RenameEdit)
return error("failed to rename in file {0}: {1}", FilePath,
RenameEdit.takeError());
@ -668,28 +671,13 @@ void findNearMiss(
} // namespace
llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
assert(!RInputs.Index == !RInputs.FS &&
"Index and FS must either both be specified or both null.");
trace::Span Tracer("Rename flow");
const auto &Opts = RInputs.Opts;
ParsedAST &AST = RInputs.AST;
const SourceManager &SM = AST.getSourceManager();
llvm::StringRef MainFileCode = SM.getBufferData(SM.getMainFileID());
auto GetFileContent = [&RInputs,
&SM](PathRef AbsPath) -> llvm::Expected<std::string> {
llvm::Optional<std::string> DirtyBuffer;
if (RInputs.GetDirtyBuffer &&
(DirtyBuffer = RInputs.GetDirtyBuffer(AbsPath)))
return std::move(*DirtyBuffer);
auto Content =
SM.getFileManager().getVirtualFileSystem().getBufferForFile(AbsPath);
if (!Content)
return error("Fail to open file {0}: {1}", AbsPath,
Content.getError().message());
if (!*Content)
return error("Got no buffer for file {0}", AbsPath);
return (*Content)->getBuffer().str();
};
// Try to find the tokens adjacent to the cursor position.
auto Loc = sourceLocationInMainFile(SM, RInputs.Pos);
if (!Loc)
@ -765,7 +753,7 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
RenameDecl, RInputs.MainFilePath, RInputs.NewName, *RInputs.Index,
Opts.LimitFiles == 0 ? std::numeric_limits<size_t>::max()
: Opts.LimitFiles,
GetFileContent);
*RInputs.FS);
if (!OtherFilesEdits)
return OtherFilesEdits.takeError();
Result.GlobalChanges = *OtherFilesEdits;

15
clang-tools-extra/clangd/refactor/Rename.h
View File

@ -21,11 +21,6 @@ namespace clangd {
class ParsedAST;
class SymbolIndex;
/// Gets dirty buffer for a given file \p AbsPath.
/// Returns None if there is no dirty buffer for the given file.
using DirtyBufferGetter =
llvm::function_ref<llvm::Optional<std::string>(PathRef AbsPath)>;
struct RenameOptions {
/// The maximum number of affected files (0 means no limit), only meaningful
/// when AllowCrossFile = true.
@ -42,14 +37,14 @@ struct RenameInputs {
ParsedAST &AST;
llvm::StringRef MainFilePath;
// The filesystem to query when performing cross file renames.
// If this is set, Index must also be set, likewise if this is nullptr, Index
// must also be nullptr.
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = nullptr;
const SymbolIndex *Index = nullptr;
RenameOptions Opts = {};
// When set, used by the rename to get file content for all rename-related
// files.
// If there is no corresponding dirty buffer, we will use the file content
// from disk.
DirtyBufferGetter GetDirtyBuffer = nullptr;
};
struct RenameResult {

76
clang-tools-extra/clangd/unittests/RenameTests.cpp
View File

@ -33,6 +33,19 @@ using testing::SizeIs;
using testing::UnorderedElementsAre;
using testing::UnorderedElementsAreArray;
llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem>
createOverlay(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> Base,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> Overlay) {
auto OFS =
llvm::makeIntrusiveRefCnt<llvm::vfs::OverlayFileSystem>(std::move(Base));
OFS->pushOverlay(std::move(Overlay));
return OFS;
}
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> getVFSFromAST(ParsedAST &AST) {
return &AST.getSourceManager().getFileManager().getVirtualFileSystem();
}
// Convert a Range to a Ref.
Ref refWithRange(const clangd::Range &Range, const std::string &URI) {
Ref Result;
@ -815,7 +828,8 @@ TEST(RenameTest, WithinFileRename) {
auto Index = TU.index();
for (const auto &RenamePos : Code.points()) {
auto RenameResult =
rename({RenamePos, NewName, AST, testPath(TU.Filename), Index.get()});
rename({RenamePos, NewName, AST, testPath(TU.Filename),
getVFSFromAST(AST), Index.get()});
ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
ASSERT_EQ(1u, RenameResult->GlobalChanges.size());
EXPECT_EQ(
@ -1101,13 +1115,21 @@ TEST(RenameTest, IndexMergeMainFile) {
auto AST = TU.build();
auto Main = testPath("main.cc");
auto InMemFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
InMemFS->addFile(testPath("main.cc"), 0,
llvm::MemoryBuffer::getMemBuffer(Code.code()));
InMemFS->addFile(testPath("other.cc"), 0,
llvm::MemoryBuffer::getMemBuffer(Code.code()));
auto Rename = [&](const SymbolIndex *Idx) {
auto GetDirtyBuffer = [&](PathRef Path) -> llvm::Optional<std::string> {
return Code.code().str(); // Every file has the same content.
};
RenameInputs Inputs{Code.point(), "xPrime", AST, Main,
Idx, RenameOptions(), GetDirtyBuffer};
RenameInputs Inputs{Code.point(),
"xPrime",
AST,
Main,
Idx ? createOverlay(getVFSFromAST(AST), InMemFS)
: nullptr,
Idx,
RenameOptions()};
auto Results = rename(Inputs);
EXPECT_TRUE(bool(Results)) << llvm::toString(Results.takeError());
return std::move(*Results);
@ -1237,25 +1259,19 @@ TEST(CrossFileRenameTests, DirtyBuffer) {
Annotations MainCode("class [[Fo^o]] {};");
auto MainFilePath = testPath("main.cc");
// Dirty buffer for foo.cc.
auto GetDirtyBuffer = [&](PathRef Path) -> llvm::Optional<std::string> {
if (Path == FooPath)
return FooDirtyBuffer.code().str();
return llvm::None;
};
llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemFS =
new llvm::vfs::InMemoryFileSystem;
InMemFS->addFile(FooPath, 0,
llvm::MemoryBuffer::getMemBuffer(FooDirtyBuffer.code()));
// Run rename on Foo, there is a dirty buffer for foo.cc, rename should
// respect the dirty buffer.
TestTU TU = TestTU::withCode(MainCode.code());
auto AST = TU.build();
llvm::StringRef NewName = "newName";
auto Results = rename({MainCode.point(),
NewName,
AST,
MainFilePath,
Index.get(),
{},
GetDirtyBuffer});
auto Results =
rename({MainCode.point(), NewName, AST, MainFilePath,
createOverlay(getVFSFromAST(AST), InMemFS), Index.get()});
ASSERT_TRUE(bool(Results)) << Results.takeError();
EXPECT_THAT(
applyEdits(std::move(Results->GlobalChanges)),
@ -1270,13 +1286,8 @@ TEST(CrossFileRenameTests, DirtyBuffer) {
// Set a file "bar.cc" on disk.
TU.AdditionalFiles["bar.cc"] = std::string(BarCode.code());
AST = TU.build();
Results = rename({MainCode.point(),
NewName,
AST,
MainFilePath,
Index.get(),
{},
GetDirtyBuffer});
Results = rename({MainCode.point(), NewName, AST, MainFilePath,
createOverlay(getVFSFromAST(AST), InMemFS), Index.get()});
ASSERT_TRUE(bool(Results)) << Results.takeError();
EXPECT_THAT(
applyEdits(std::move(Results->GlobalChanges)),
@ -1312,13 +1323,8 @@ TEST(CrossFileRenameTests, DirtyBuffer) {
size_t estimateMemoryUsage() const override { return 0; }
} PIndex;
Results = rename({MainCode.point(),
NewName,
AST,
MainFilePath,
&PIndex,
{},
GetDirtyBuffer});
Results = rename({MainCode.point(), NewName, AST, MainFilePath,
createOverlay(getVFSFromAST(AST), InMemFS), &PIndex});
EXPECT_FALSE(Results);
EXPECT_THAT(llvm::toString(Results.takeError()),
testing::HasSubstr("too many occurrences"));
@ -1368,8 +1374,8 @@ TEST(CrossFileRenameTests, DeduplicateRefsFromIndex) {
Ref ReturnedRef;
} DIndex(XRefInBarCC);
llvm::StringRef NewName = "newName";
auto Results =
rename({MainCode.point(), NewName, AST, MainFilePath, &DIndex});
auto Results = rename({MainCode.point(), NewName, AST, MainFilePath,
getVFSFromAST(AST), &DIndex});
ASSERT_TRUE(bool(Results)) << Results.takeError();
EXPECT_THAT(
applyEdits(std::move(Results->GlobalChanges)),