njames93 updated this revision to Diff 315235.
njames93 marked an inline comment as done.
njames93 added a comment.
Address most comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93978/new/
https://reviews.llvm.org/D93978
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/refactor/Tweak.cpp
clang-tools-extra/clangd/refactor/Tweak.h
clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
@@ -74,7 +74,8 @@
SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.first,
Range.second, [&](SelectionTree ST) {
Tweak::Selection S(Index, AST, Range.first,
- Range.second, std::move(ST));
+ Range.second, std::move(ST),
+ nullptr);
if (auto T = prepareTweak(TweakID, S)) {
Result = (*T)->apply(S);
return true;
Index: clang-tools-extra/clangd/tool/Check.cpp
===================================================================
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -202,7 +202,8 @@
vlog(" {0} {1}", Pos, Tok.text(AST->getSourceManager()));
auto Tree = SelectionTree::createRight(AST->getASTContext(),
AST->getTokens(), Start, End);
- Tweak::Selection Selection(&Index, *AST, Start, End, std::move(Tree));
+ Tweak::Selection Selection(&Index, *AST, Start, End, std::move(Tree),
+ nullptr);
for (const auto &T : prepareTweaks(Selection, Opts.TweakFilter)) {
auto Result = T->apply(Selection);
if (!Result) {
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -36,6 +36,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/VirtualFileSystem.h"
#include <cstddef>
#include <string>
@@ -63,10 +64,9 @@
}
llvm::Optional<Path> getSourceFile(llvm::StringRef FileName,
- const Tweak::Selection &Sel) {
- if (auto Source = getCorrespondingHeaderOrSource(
- FileName,
- &Sel.AST->getSourceManager().getFileManager().getVirtualFileSystem()))
+ const Tweak::Selection &Sel,
+ llvm::vfs::FileSystem *FS) {
+ if (auto Source = getCorrespondingHeaderOrSource(FileName, FS))
return *Source;
return getCorrespondingHeaderOrSource(FileName, *Sel.AST, Sel.Index);
}
@@ -403,13 +403,14 @@
if (!MainFileName)
return error("Couldn't get absolute path for main file.");
- auto CCFile = getSourceFile(*MainFileName, Sel);
+ auto *FS = Sel.FS;
+ assert(FS && "FS Must be set in apply");
+
+ auto CCFile = getSourceFile(*MainFileName, Sel, FS);
+
if (!CCFile)
return error("Couldn't find a suitable implementation file.");
-
- auto &FS =
- Sel.AST->getSourceManager().getFileManager().getVirtualFileSystem();
- auto Buffer = FS.getBufferForFile(*CCFile);
+ auto Buffer = FS->getBufferForFile(*CCFile);
// FIXME: Maybe we should consider creating the implementation file if it
// doesn't exist?
if (!Buffer)
Index: clang-tools-extra/clangd/refactor/Tweak.h
===================================================================
--- clang-tools-extra/clangd/refactor/Tweak.h
+++ clang-tools-extra/clangd/refactor/Tweak.h
@@ -30,6 +30,7 @@
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/VirtualFileSystem.h"
#include <string>
namespace clang {
@@ -48,7 +49,8 @@
/// Input to prepare and apply tweaks.
struct Selection {
Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin,
- unsigned RangeEnd, SelectionTree ASTSelection);
+ unsigned RangeEnd, SelectionTree ASTSelection,
+ llvm::vfs::FileSystem *VFS);
/// The text of the active document.
llvm::StringRef Code;
/// The Index for handling codebase related queries.
@@ -64,6 +66,11 @@
unsigned SelectionEnd;
/// The AST nodes that were selected.
SelectionTree ASTSelection;
+ /// File system used to access source code (for cross-file tweaks).
+ /// This can be used to overlay the "dirty" contents of files open in the
+ /// editor, which (in case of headers) may not match the saved contents used
+ /// for building the AST.
+ llvm::vfs::FileSystem *FS = nullptr;
// FIXME: provide a way to get sources and ASTs for other files.
};
Index: clang-tools-extra/clangd/refactor/Tweak.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Tweak.cpp
+++ clang-tools-extra/clangd/refactor/Tweak.cpp
@@ -47,9 +47,12 @@
Tweak::Selection::Selection(const SymbolIndex *Index, ParsedAST &AST,
unsigned RangeBegin, unsigned RangeEnd,
- SelectionTree ASTSelection)
+ SelectionTree ASTSelection,
+ llvm::vfs::FileSystem *FS)
: Index(Index), AST(&AST), SelectionBegin(RangeBegin),
- SelectionEnd(RangeEnd), ASTSelection(std::move(ASTSelection)) {
+ SelectionEnd(RangeEnd), ASTSelection(std::move(ASTSelection)),
+ FS(FS ? FS
+ : &AST.getSourceManager().getFileManager().getVirtualFileSystem()) {
auto &SM = AST.getSourceManager();
Code = SM.getBufferData(SM.getMainFileID());
Cursor = SM.getComposedLoc(SM.getMainFileID(), RangeBegin);
Index: clang-tools-extra/clangd/TUScheduler.h
===================================================================
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -236,9 +236,15 @@
/// if requested with WantDiags::Auto or WantDiags::Yes.
void remove(PathRef File);
+ /// FIXME: Remove this infavour of the VFS approach.
/// Returns a snapshot of all file buffer contents, per last update().
llvm::StringMap<std::string> getAllFileContents() const;
+ /// Returns a Snapshot of all file buffer contents in a Filesystem overlayed
+ /// ontop of \p Base.
+ std::unique_ptr<llvm::vfs::OverlayFileSystem> overlayFileContents(
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> Base) 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,
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1456,5 +1456,23 @@
MT.detail(Elem.first()).child("ast").addUsage(Elem.second.UsedBytesAST);
}
}
+
+std::unique_ptr<llvm::vfs::OverlayFileSystem> TUScheduler::overlayFileContents(
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> Base) const {
+ auto OverlayFS =
+ std::make_unique<llvm::vfs::OverlayFileSystem>(std::move(Base));
+
+ // FIXME: Copying all files is needlessly expensive when in most cases we only
+ // want to read typically 1 of the files. It may be worth looking into lazily
+ // copying the contents.
+ auto *InMemoryFS = new llvm::vfs::InMemoryFileSystem;
+ for (const auto &File : Files)
+ InMemoryFS->addFile(
+ File.first(), 0,
+ llvm::MemoryBuffer::getMemBufferCopy(File.getValue()->Contents));
+
+ OverlayFS->pushOverlay(InMemoryFS);
+ return OverlayFS;
+}
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -32,6 +32,7 @@
#include "support/MemoryTree.h"
#include "support/ThreadsafeFS.h"
#include "support/Trace.h"
+#include "clang/Basic/LLVM.h"
#include "clang/Format/Format.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h"
@@ -49,6 +50,7 @@
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
#include <chrono>
@@ -429,7 +431,8 @@
// May generate several candidate selections, due to SelectionTree ambiguity.
// vector of pointers because GCC doesn't like non-copyable Selection.
static llvm::Expected<std::vector<std::unique_ptr<Tweak::Selection>>>
-tweakSelection(const Range &Sel, const InputsAndAST &AST) {
+tweakSelection(const Range &Sel, const InputsAndAST &AST,
+ llvm::vfs::FileSystem *FS) {
auto Begin = positionToOffset(AST.Inputs.Contents, Sel.start);
if (!Begin)
return Begin.takeError();
@@ -441,7 +444,7 @@
AST.AST.getASTContext(), AST.AST.getTokens(), *Begin, *End,
[&](SelectionTree T) {
Result.push_back(std::make_unique<Tweak::Selection>(
- AST.Inputs.Index, AST.AST, *Begin, *End, std::move(T)));
+ AST.Inputs.Index, AST.AST, *Begin, *End, std::move(T), FS));
return false;
});
assert(!Result.empty() && "Expected at least one SelectionTree");
@@ -454,12 +457,13 @@
// Tracks number of times a tweak has been offered.
static constexpr trace::Metric TweakAvailable(
"tweak_available", trace::Metric::Counter, "tweak_id");
- auto Action = [File = File.str(), Sel, CB = std::move(CB),
+ auto Action = [File = File.str(), Sel, CB = std::move(CB), &TFS = this->TFS,
Filter =
std::move(Filter)](Expected<InputsAndAST> InpAST) mutable {
if (!InpAST)
return CB(InpAST.takeError());
- auto Selections = tweakSelection(Sel, *InpAST);
+ auto FS = TFS.view(llvm::None);
+ auto Selections = tweakSelection(Sel, *InpAST, FS.get());
if (!Selections)
return CB(Selections.takeError());
std::vector<TweakRef> Res;
@@ -495,15 +499,17 @@
auto Action = [File = File.str(), Sel, TweakID = TweakID.str(),
CB = std::move(CB),
this](Expected<InputsAndAST> InpAST) mutable {
+ IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS(
+ WorkScheduler.overlayFileContents(TFS.view(llvm::None)));
if (!InpAST)
return CB(InpAST.takeError());
- auto Selections = tweakSelection(Sel, *InpAST);
+ auto Selections = tweakSelection(Sel, *InpAST, FS.get());
if (!Selections)
return CB(Selections.takeError());
llvm::Optional<llvm::Expected<Tweak::Effect>> Effect;
// Try each selection, take the first one that prepare()s.
// If they all fail, Effect will hold get the last error.
- for (const auto &Selection : *Selections) {
+ for (auto &Selection : *Selections) {
auto T = prepareTweak(TweakID, *Selection);
if (T) {
Effect = (*T)->apply(*Selection);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
