[PATCH] D81920: [clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: clang-tools-extra/clangd/support/FSProvider.h:37
+  /// This is an overload instead of an optional to make implicit string ->
+  /// StringRef conversion possible.
+  virtual llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
----------------
Re how to fix the GCC warning: I haven't fully understood this code yet (because it's hard to understand! :)) but I think what you're trying to do here is something like
```
class FileSystemProvider {
public:
    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
    getFileSystem(std::optional<PathRef> CWD) const {
        if (CWD.has_value()) {
            return this->doGetDefaultFileSystem();  // virtual call
        } else {
            return this->doGetFileSystem(CWD);  // virtual call
        }
    }
private:
    virtual llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
    doGetDefaultFileSystem() const = 0;
    virtual llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
    doGetFileSystem(PathRef) const;
};
```
And then in `class RealFileSystemProvider`, you want to override `doGetDefaultFileSystem()` while leaving `doGetFileSystem` untouched. Does that basically reflect your intent here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81920