[PATCH] D93978: [clangd] DefineOutline doesn't require implementation file being saved

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D93978#2479440 <https://reviews.llvm.org/D93978#2479440>, @sammccall wrote:

> Ah, thanks for working on this!
>
> A few thoughts:
>
> - when we're pseudoparsing the file we're going to modify as we do here, using the new content is strictly better, no downside :-)
> - the old method here of accessing the content through the FS attached to the AST is a hack
> - however I think DraftStore is the wrong abstraction here and VFS is the right one.
>   - each tweak shouldn't have to deal with falling back from DraftStore to VFS (and rename also has an implementation of this!)
>   - C++ embedders don't have a DraftStore lying around
>   - bug: this code won't find a named but unsaved implementation file (since getCorrespondingHeaderOrSource uses VFS)
> - (also Tweak::Selection isn't a pure abstraction, we can jam the FS in there instead of passing it around as a separate param everywhere)
>
> So I think the right way to expose this to tweaks is to add a `vfs::FileSystem *` member to `Tweak::Selection`, that allows accessing the current FS (as opposed to the one used for building).
> The simplest implementation uses OverlayVFS + InMemoryFS + TUScheduler::getAllFileContents(). This isn't as expensive as it sounds as we need only set the FS pointer for `apply()`.
> (We can also implement a FS that copies more lazily, though I suspect it's not worth it)

I'm getting a little stuck here. I've made a OverlayFS which has the a TFS::view as the base, and an InMemoryFS containing the contents of the TUScheduler. I Passed that in the Selection and it all worked.
However once I changed the `getSourceFile` function to also use that same FS, it would find the file ok, but then clangd would crash(with no visible trace) when it tried to open the file.

  auto Buffer = FS.getBufferForFile(*CCFile); // Here

>From what I can getCorrespondingSourceOrHeader is querying the InMemoryFS and maybe those calls to FS::exists are somehow corrupting something.
Whatever it is, it all seems a bit messed up to me.

I'm gonna try with a debug build (and possibly asan) to see If I could possibly get anything more.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93978/new/

https://reviews.llvm.org/D93978