[clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory

[kadircet]

Summary

We've faced a couple of problems when the returned FS didn't have the
proper working directory. New signature makes the API safer against such
problems.

[sammccall]

Looks like an improvement but I don't like the (".") in various places.

Maybe make the param optional<StringRef> and don't cd if it's none?
I wouldn't give it a default arg though, the idea is to force a choice.

clang-tools-extra/clangd/ClangdServer.cpp

187

Not sure what to do in this patch, but ultimately I don't think we must/should pass an FS of any form into this callback.
(Doesn't make sense to assume getting options needs the FS, embedders can use the context to know what version of the FS to use if needed)

[kadircet]

• Change signature to llvm::Optional<StringRef> to accomodate call sites that don't want to cd

[kadircet]

• Provide two overloads to make implicit string -> StringRef conversion possible.

[kadircet]

clang-tools-extra/clangd/ClangdServer.cpp

187

sounds good, will delete in a follow-up patch.

[sammccall]

clang-tools-extra/clangd/ClangdServer.cpp

323

nit: I think we should have /*CWD=*/ on the None (and elsewhere)

402

seems weird that we provide a working directory sometimes but not other times when getting format style.
I think this is because not providing one is suspicious, it's not actually needed, but sometimes we have one around?

Maybe just make getFormatStyleForFile take a FSProvider instead?

555

we could plumb in threadsafefs here too...

clang-tools-extra/clangd/Preamble.cpp

424

I don't see a corresponding removed CD - is this a bugfix?

clang-tools-extra/clangd/support/FSProvider.cpp

81

Nit: RealFileSystemProvider isn't the current class.

What about elog("VFS: failed to set CWD to {0}: {1}", CWD, Err.message())?

clang-tools-extra/clangd/support/FSProvider.h

32

/// Initial working directory is arbitrary.

34

"Similar to above one" seems a bit awkward, maybe "As above"?

curret -> current

38

did you want this to be virtual so that a truly virtual FS e.g. where the working directory is just a variable can be constructed with it directly?

[kadircet]

clang-tools-extra/clangd/ClangdServer.cpp

555

this function is also used by tweaks (DefineOutline), and they don't have access to FSProvider. I would rather do that plumbing on a separate patch.

clang-tools-extra/clangd/Preamble.cpp

424

not really, CWD doesn't matter for preamble scanning as we only go over current file contents that are set explicitly.

It will actually be dropped by https://reviews.llvm.org/D81719, changing it to pass None here to not confuse.

[kadircet]

• Address comments

[uabelho]

clang-tools-extra/clangd/support/FSProvider.h

38

I noticed that gcc (7.4) warns on this line:

/data/repo/master/clang-tools-extra/clangd/support/FSProvider.h:39:3: warning: 'virtual llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> clang::clangd::FileSystemProvider::getFileSystem(clang::clangd::PathRef) const' was hidden [-Woverloaded-virtual]
   getFileSystem(PathRef CWD) const;
   ^~~~~~~~~~~~~

45

Similar warning here as well:

/data/repo/master/clang-tools-extra/clangd/support/FSProvider.h:45:7: warning:   by 'virtual llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> clang::clangd::RealFileSystemProvider::getFileSystem(llvm::NoneType) const' [-Woverloaded-virtual]
       getFileSystem(llvm::NoneType) const override;
       ^~~~~~~~~~~~~

[uabelho]

clang-tools-extra/clangd/Preamble.cpp

241

A warning here too

23:51:14 /repo/bbiswjenk/fem023-eiffel003/workspace/llvm/sdk_1_20_ki_dev_test/clang-tools-extra/clangd/Preamble.cpp:234:5: warning:   by 'virtual llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> clang::clangd::{anonymous}::scanPreamble(llvm::StringRef, const clang::tooling::CompileCommand&)::EmptyFS::view(llvm::NoneType) const' [-Woverloaded-virtual]
23:51:14      view(llvm::NoneType) const override {
23:51:14      ^~~~

[kadircet]

thanks for the info @uabelho!

this looks like a dormant warning though, as StringRef is not implicitly convertible to NoneType (and vice-versa) hence anyone trying to make use of the hidden overload would get a hard compile error anyways.
Moreover this class is mostly accessed through a base pointer, hence name hiding in derived classes isn't really an issue (for most of the production code).

Also the warning itself seems to be noisy https://gcc.gnu.org/bugzilla/show_bug.cgi?id=20423. Interesting this seems to be only enabled for clang and nothing else, I wonder how it is decided.
Unfortunately history doesn't tell much https://github.com/llvm/llvm-project/blame/master/clang/CMakeLists.txt#L396.

There are 5 derived classes (3 of them are in tests), so just putting a using declaration to un-hide the overload seems too disruptive.
Again renaming the endpoints (and possibly changing the signature) just to suppress this warning also doesn't seem so nice.

I would rather like to turn-off this warning for at least gcc, assuming this is not specific to that version. Can you check if you see the warning with different versions of gcc?

[uabelho]

In D81920#2109901, @kadircet wrote:

Can you check if you see the warning with different versions of gcc?

Originally I got them with 7.4.0 and now I tried with 9.3.0 and got them there as well.

[bjope]

I think the warnings can be hidden by something like this (I'm no expert in this area though, but it seems like this technique has been used a number of times before in llvm, so hopefully it applies here as well):

diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index ca0a76db78f4..1970541bc56a 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -230,6 +230,7 @@ llvm::Expected<ScannedPreamble>
 scanPreamble(llvm::StringRef Contents, const tooling::CompileCommand &Cmd) {
   class EmptyFS : public ThreadsafeFS {
   public:
+    using ThreadsafeFS::view;
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
     view(llvm::NoneType) const override {
       return new llvm::vfs::InMemoryFileSystem;
diff --git a/clang-tools-extra/clangd/support/ThreadsafeFS.h b/clang-tools-extra/clangd/support/ThreadsafeFS.h
index aa6825fb3999..eb9016bad201 100644
--- a/clang-tools-extra/clangd/support/ThreadsafeFS.h
+++ b/clang-tools-extra/clangd/support/ThreadsafeFS.h
@@ -42,6 +42,7 @@ public:
 
 class RealThreadsafeFS : public ThreadsafeFS {
 public:
+  using ThreadsafeFS::view;
   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
       view(llvm::NoneType) const override;
 };

at least I get no warnings when building clangd (haven't checked the unittests, but I don't care about warnings in those right now).

[• Quuxplusone]

clang-tools-extra/clangd/support/FSProvider.h

36

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?

[sammccall]

And then in class RealFileSystemProvider, you want to override doGetDefaultFileSystem() while leaving doGetFileSystem untouched. Does that basically reflect your intent here?

Yes, except with fewer confusing names :-)

The name used now (there was a rename patch following this) is ThreadsafeFS::view - it took us a couple of years to work out what this abstraction really was.

The purpose of using the same name for the two public overloads is that they do the same thing. The purpose of using the same name for the public no-CWD version and the version to be overridden is that they *are* the same thing. These are more important reasons than the idea that non-existent non-polymorphic callers will miss an overload.

("More important" subjectively, but code always ends up stamped with the taste of the people working on it...)

[sammccall]

Oops, I misread and forgot one point. The public interface can't (only) take optional<StringRef> because that's not implicitly convertible from a std::string which is what we usually want to pass. Thus the silly overloads, which we were going to fix with a more careful overload set approximating optional<StringRef> once we hit a case where we didn't know statically whether we had a working directory or not.

(Didn't push for it to be in this patch because getting overload sets right is hard)

clang-tools-extra/clangd/ClangdServer.cpp

 
 void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
                                llvm::StringRef Version,
                                WantDiagnostics WantDiags, bool ForceRebuild) {
   ParseOptions Opts;
   Opts.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults();
   // FIXME: call tidy options builder on the worker thread, it can do IO.
   if (GetClangTidyOptions)
-    Opts.ClangTidyOpts = GetClangTidyOptions(*FSProvider.getFileSystem(), File);
+    Opts.ClangTidyOpts =
+        GetClangTidyOptions(*FSProvider.getFileSystem(llvm::None), File);
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
 
   // Compile command is set asynchronously during update, as it can be slow.
   ParseInputs Inputs;
   Inputs.FSProvider = &FSProvider;
   Inputs.Contents = std::string(Contents);
   Inputs.Version = Version.str();
   Inputs.ForceRebuild = ForceRebuild;

 }
 
 llvm::Expected<std::vector<TextEdit>>
 ClangdServer::formatOnType(llvm::StringRef Code, PathRef File, Position Pos,
                            StringRef TriggerText) {
   llvm::Expected<size_t> CursorPos = positionToOffset(Code, Pos);
   if (!CursorPos)
     return CursorPos.takeError();
-  auto FS = FSProvider.getFileSystem();
   auto Style = format::getStyle(format::DefaultFormatStyle, File,
-                                format::DefaultFallbackStyle, Code, FS.get());
+                                format::DefaultFallbackStyle, Code,
+                                FSProvider.getFileSystem(llvm::None).get());
   if (!Style)
     return Style.takeError();
 
   std::vector<TextEdit> Result;
   for (const tooling::Replacement &R :
        formatIncremental(Code, *CursorPos, TriggerText, *Style))
     Result.push_back(replacementToEdit(Code, R));
   return Result;
auto Action = [File = File.str(), NewName = NewName.str(), Pos, Opts,
     auto Edits = clangd::rename(
         {Pos, NewName, InpAST->AST, File, Index, Opts, GetDirtyBuffer});
     if (!Edits)
       return CB(Edits.takeError());
 
     if (Opts.WantFormat) {
       auto Style = getFormatStyleForFile(
           File, InpAST->Inputs.Contents,
-          InpAST->Inputs.FSProvider->getFileSystem().get());
+          InpAST->Inputs.FSProvider
+              ->getFileSystem(InpAST->Inputs.CompileCommand.Directory)
+              .get());
       llvm::Error Err = llvm::Error::success();
       for (auto &E : *Edits)
         Err =
             llvm::joinErrors(reformatEdit(E.getValue(), Style), std::move(Err));
 
       if (Err)
         return CB(std::move(Err));
     }

 }
 
 void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
                               Callback<Tweak::Effect> CB) {
   // Tracks number of times a tweak has been applied.
   static constexpr trace::Metric TweakAttempt(
       "tweak_attempt", trace::Metric::Counter, "tweak_id");
   TweakAttempt.record(1, TweakID);
-  auto Action =
-      [File = File.str(), Sel, TweakID = TweakID.str(), CB = std::move(CB),
-       FS = FSProvider.getFileSystem()](Expected<InputsAndAST> InpAST) mutable {
-        if (!InpAST)
-          return CB(InpAST.takeError());
-        auto Selections = tweakSelection(Sel, *InpAST);
-        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) {
-          auto T = prepareTweak(TweakID, *Selection);
-          if (T) {
-            Effect = (*T)->apply(*Selection);
-            break;
-          }
-          Effect = T.takeError();
-        }
-        assert(Effect.hasValue() && "Expected at least one selection");
-        if (*Effect) {
-          // Tweaks don't apply clang-format, do that centrally here.
-          for (auto &It : (*Effect)->ApplyEdits) {
-            Edit &E = It.second;
-            format::FormatStyle Style =
-                getFormatStyleForFile(File, E.InitialCode, FS.get());
-            if (llvm::Error Err = reformatEdit(E, Style))
-              elog("Failed to format {0}: {1}", It.first(), std::move(Err));
-          }
-        }
-        return CB(std::move(*Effect));
-      };
+  auto Action = [File = File.str(), Sel, TweakID = TweakID.str(),
+                 CB = std::move(CB),
+                 this](Expected<InputsAndAST> InpAST) mutable {
+    auto FS = FSProvider.getFileSystem(InpAST->Inputs.CompileCommand.Directory);
   WorkScheduler.runWithAST("ApplyTweak", File, std::move(Action));
 }
 
 void ClangdServer::dumpAST(PathRef File,
                            llvm::unique_function<void(std::string)> Callback) {
   auto Action = [Callback = std::move(Callback)](
                     llvm::Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST) {

 void ClangdServer::switchSourceHeader(
     PathRef Path, Callback<llvm::Optional<clangd::Path>> CB) {
   // We want to return the result as fast as possible, strategy is:
   //  1) use the file-only heuristic, it requires some IO but it is much
   //     faster than building AST, but it only works when .h/.cc files are in
   //     the same directory.
   //  2) if 1) fails, we use the AST&Index approach, it is slower but supports
   //     different code layout.
   if (auto CorrespondingFile = getCorrespondingHeaderOrSource(
-          std::string(Path), FSProvider.getFileSystem()))
+          std::string(Path), FSProvider.getFileSystem(llvm::None)))
     return CB(std::move(CorrespondingFile));
   auto Action = [Path = Path.str(), CB = std::move(CB),
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
     CB(getCorrespondingHeaderOrSource(Path, InpAST->AST, Index));
   };
   WorkScheduler.runWithAST("SwitchHeaderSource", Path, std::move(Action));
 }
 
 llvm::Expected<tooling::Replacements>
 ClangdServer::formatCode(llvm::StringRef Code, PathRef File,
                          llvm::ArrayRef<tooling::Range> Ranges) {
   // Call clang-format.
-  format::FormatStyle Style =
-      getFormatStyleForFile(File, Code, FSProvider.getFileSystem().get());
+  format::FormatStyle Style = getFormatStyleForFile(
+      File, Code, FSProvider.getFileSystem(llvm::None).get());
   tooling::Replacements IncludeReplaces =
       format::sortIncludes(Style, Code, Ranges, File);
   auto Changed = tooling::applyAllReplacements(Code, IncludeReplaces);
   if (!Changed)
     return Changed.takeError();
 
   return IncludeReplaces.merge(format::reformat(
       Style, *Changed,

 }
 
 void ClangdServer::findHover(PathRef File, Position Pos,
                              Callback<llvm::Optional<HoverInfo>> CB) {
   auto Action = [File = File.str(), Pos, CB = std::move(CB),
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
-    format::FormatStyle Style =
-        getFormatStyleForFile(File, InpAST->Inputs.Contents,
-                              InpAST->Inputs.FSProvider->getFileSystem().get());
+    format::FormatStyle Style = getFormatStyleForFile(
+        File, InpAST->Inputs.Contents,
+        InpAST->Inputs.FSProvider
+            ->getFileSystem(InpAST->Inputs.CompileCommand.Directory)
+            .get());
     CB(clangd::getHover(InpAST->AST, Pos, std::move(Style), Index));
   };
 
   WorkScheduler.runWithAST("Hover", File, std::move(Action),
                            TUScheduler::InvalidateOnUpdate);
 }
 
 void ClangdServer::typeHierarchy(PathRef File, Position Pos, int Resolve,

clang-tools-extra/clangd/CodeComplete.cpp

bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
   PreambleBounds PreambleRegion =
       ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0);
   bool CompletingInPreamble = PreambleRegion.Size > Input.Offset;
   if (Input.Patch)
     Input.Patch->apply(*CI);
   // NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise
   // the remapped buffers do not get freed.
   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS =
-      Input.ParseInput.FSProvider->getFileSystem();
+      Input.ParseInput.FSProvider->getFileSystem(
+          Input.ParseInput.CompileCommand.Directory);
   if (Input.Preamble.StatCache)
     VFS = Input.Preamble.StatCache->getConsumingFS(std::move(VFS));
-  if (VFS->setCurrentWorkingDirectory(
-          Input.ParseInput.CompileCommand.Directory))
-    elog("Couldn't set working directory during code completion");
   auto Clang = prepareCompilerInstance(
       std::move(CI), !CompletingInPreamble ? &Input.Preamble.Preamble : nullptr,
       std::move(ContentsBuffer), std::move(VFS), IgnoreDiags);
   Clang->getPreprocessorOpts().SingleFileParseMode = CompletingInPreamble;
   Clang->setCodeCompletionConsumer(Consumer.release());
 
   SyntaxOnlyAction Action;
   if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) {
CodeCompleteResult run(const SemaCompleteInput &SemaCCInput) && {
     //   - partial identifier and context. We need these for the index query.
     CodeCompleteResult Output;
     auto RecorderOwner = std::make_unique<CompletionRecorder>(Opts, [&]() {
       assert(Recorder && "Recorder is not set");
       CCContextKind = Recorder->CCContext.getKind();
       IsUsingDeclaration = Recorder->CCContext.isUsingDeclaration();
       auto Style = getFormatStyleForFile(
           SemaCCInput.FileName, SemaCCInput.ParseInput.Contents,
-          SemaCCInput.ParseInput.FSProvider->getFileSystem().get());
+          SemaCCInput.ParseInput.FSProvider
+              ->getFileSystem(SemaCCInput.ParseInput.CompileCommand.Directory)
+              .get());
       const auto NextToken = Lexer::findNextToken(
           Recorder->CCSema->getPreprocessor().getCodeCompletionLoc(),
           Recorder->CCSema->getSourceManager(), Recorder->CCSema->LangOpts);
       if (NextToken)
         NextTokenKind = NextToken->getKind();
       // If preprocessor was run, inclusions from preprocessor callback should
       // already be added to Includes.
       Inserter.emplace(
if (!Offset) {
     return CodeCompleteResult();
   }
   auto Flow = CodeCompleteFlow(
       FileName, Preamble ? Preamble->Includes : IncludeStructure(),
       SpecFuzzyFind, Opts);
   return (!Preamble || Opts.RunParser == CodeCompleteOptions::NeverParse)
              ? std::move(Flow).runWithoutSema(
                    ParseInput.Contents, *Offset,
-                   ParseInput.FSProvider->getFileSystem())
+                   ParseInput.FSProvider->getFileSystem(
+                       ParseInput.CompileCommand.Directory))
              : std::move(Flow).run({FileName, *Offset, *Preamble,
                                     // We want to serve code completions with
                                     // low latency, so don't bother patching.
                                     /*PreamblePatch=*/llvm::None, ParseInput});
 }
 
 SignatureHelp signatureHelp(PathRef FileName, Position Pos,
                             const PreambleData &Preamble,

clang-tools-extra/clangd/Compiler.cpp

 //===--- Compiler.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 "Compiler.h"
 #include "support/Logger.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Serialization/PCHContainerOperations.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
 
 namespace clang {
 namespace clangd {
 
 void IgnoreDiagnostics::log(DiagnosticsEngine::Level DiagLevel,
                             const clang::Diagnostic &Info) {

 
 std::unique_ptr<CompilerInvocation>
 buildCompilerInvocation(const ParseInputs &Inputs, clang::DiagnosticConsumer &D,
                         std::vector<std::string> *CC1Args) {
   std::vector<const char *> ArgStrs;
   for (const auto &S : Inputs.CompileCommand.CommandLine)
     ArgStrs.push_back(S.c_str());
 
-  auto VFS = Inputs.FSProvider->getFileSystem();
-  if (VFS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {
-    log("Couldn't set working directory when creating compiler invocation.");
-    // We proceed anyway, our lit-tests rely on results for non-existing working
-    // dirs.
-  }
-
+  auto VFS = Inputs.FSProvider->getFileSystem(Inputs.CompileCommand.Directory);
   llvm::IntrusiveRefCntPtr<DiagnosticsEngine> CommandLineDiagsEngine =
       CompilerInstance::createDiagnostics(new DiagnosticOptions, &D, false);
   std::unique_ptr<CompilerInvocation> CI = createInvocationFromCommandLine(
       ArgStrs, CommandLineDiagsEngine, std::move(VFS),
       /*ShouldRecoverOnErrors=*/true, CC1Args);
   if (!CI)
     return nullptr;
   // createInvocationFromCommandLine sets DisableFree.

clang-tools-extra/clangd/ParsedAST.cpp

 #include "clang/Serialization/ASTWriter.h"
 #include "clang/Serialization/PCHContainerOperations.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <memory>
 #include <vector>
 
 // Force the linker to link in Clang-tidy modules.
 // clangd doesn't support the static analyzer.
 #define CLANG_TIDY_DISABLE_STATIC_ANALYZER_CHECKS

 llvm::Optional<ParsedAST>
 ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
                  std::unique_ptr<clang::CompilerInvocation> CI,
                  llvm::ArrayRef<Diag> CompilerInvocationDiags,
                  std::shared_ptr<const PreambleData> Preamble) {
   trace::Span Tracer("BuildAST");
   SPAN_ATTACH(Tracer, "File", Filename);
 
-  auto VFS = Inputs.FSProvider->getFileSystem();
+  auto VFS = Inputs.FSProvider->getFileSystem(Inputs.CompileCommand.Directory);
   if (Preamble && Preamble->StatCache)
     VFS = Preamble->StatCache->getConsumingFS(std::move(VFS));
-  if (VFS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {
-    log("Couldn't set working directory when building the preamble.");
-    // We proceed anyway, our lit-tests rely on results for non-existing working
-    // dirs.
-  }
 
   assert(CI);
   // Command-line parsing sets DisableFree to true by default, but we don't want
   // to leak memory in clangd.
   CI->getFrontendOpts().DisableFree = false;
   const PrecompiledPreamble *PreamblePCH =
       Preamble ? &Preamble->Preamble : nullptr;
 

clang-tools-extra/clangd/Preamble.cpp

 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormatVariadic.h"
scanPreamble(llvm::StringRef Contents,
   // than vfs::FileSystem, that way we can just use ParseInputs without this
   // hack.
   auto GetFSProvider = [](llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS) {
     class VFSProvider : public FileSystemProvider {
     public:
       VFSProvider(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
           : VFS(std::move(FS)) {}
       llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-      getFileSystem() const override {
+      getFileSystem(llvm::NoneType) const override {
         return VFS;
       }
 
     private:
       const llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS;
     };
     return std::make_unique<VFSProvider>(std::move(FS));
   };
buildPreamble(PathRef FileName, CompilerInvocation CI,
   // the preamble and make it smaller.
   assert(!CI.getFrontendOpts().SkipFunctionBodies);
   CI.getFrontendOpts().SkipFunctionBodies = true;
   // We don't want to write comment locations into PCH. They are racy and slow
   // to read back. We rely on dynamic index for the comments instead.
   CI.getPreprocessorOpts().WriteCommentListToPCH = false;
 
   CppFilePreambleCallbacks SerializedDeclsCollector(FileName, PreambleCallback);
-  auto VFS = Inputs.FSProvider->getFileSystem();
-  if (VFS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {
-    log("Couldn't set working directory when building the preamble.");
-    // We proceed anyway, our lit-tests rely on results for non-existing working
-    // dirs.
-  }
-
+  auto VFS = Inputs.FSProvider->getFileSystem(Inputs.CompileCommand.Directory);
   llvm::SmallString<32> AbsFileName(FileName);
   VFS->makeAbsolute(AbsFileName);
   auto StatCache = std::make_unique<PreambleFileStatusCache>(AbsFileName);
   auto BuiltPreamble = PrecompiledPreamble::Build(
       CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine,
       StatCache->getProducingFS(VFS),
       std::make_shared<PCHContainerOperations>(), StoreInMemory,
       SerializedDeclsCollector);

 
 bool isPreambleCompatible(const PreambleData &Preamble,
                           const ParseInputs &Inputs, PathRef FileName,
                           const CompilerInvocation &CI) {
   auto ContentsBuffer =
       llvm::MemoryBuffer::getMemBuffer(Inputs.Contents, FileName);
   auto Bounds =
       ComputePreambleBounds(*CI.getLangOpts(), ContentsBuffer.get(), 0);
-  auto VFS = Inputs.FSProvider->getFileSystem();
-  VFS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory);
+  auto VFS = Inputs.FSProvider->getFileSystem(Inputs.CompileCommand.Directory);
   return compileCommandsAreEqual(Inputs.CompileCommand,
                                  Preamble.CompileCommand) &&
          Preamble.Preamble.CanReuse(CI, ContentsBuffer.get(), Bounds,
                                     VFS.get());
 }
 
 void escapeBackslashAndQuotes(llvm::StringRef Text, llvm::raw_ostream &OS) {
   for (char C : Text) {

 }
 
 PreamblePatch PreamblePatch::create(llvm::StringRef FileName,
                                     const ParseInputs &Modified,
                                     const PreambleData &Baseline) {
   trace::Span Tracer("CreatePreamblePatch");
   SPAN_ATTACH(Tracer, "File", FileName);
   assert(llvm::sys::path::is_absolute(FileName) && "relative FileName!");
-  auto VFS =
-      Baseline.StatCache->getConsumingFS(Modified.FSProvider->getFileSystem());
+  auto VFS = Baseline.StatCache->getConsumingFS(
+      Modified.FSProvider->getFileSystem(Modified.CompileCommand.Directory));
   // First scan preprocessor directives in Baseline and Modified. These will be
   // used to figure out newly added directives in Modified. Scanning can fail,
   // the code just bails out and creates an empty patch in such cases, as:
   // - If scanning for Baseline fails, no knowledge of existing includes hence
   //   patch will contain all the includes in Modified. Leading to rebuild of
   //   whole preamble, which is terribly slow.
   // - If scanning for Modified fails, cannot figure out newly added ones so
   //   there's nothing to do but generate an empty patch.

clang-tools-extra/clangd/index/Background.cpp

void BackgroundIndex::update(
   }
 }
 
 llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd) {
   trace::Span Tracer("BackgroundIndex");
   SPAN_ATTACH(Tracer, "file", Cmd.Filename);
   auto AbsolutePath = getAbsolutePath(Cmd);
 
-  auto FS = FSProvider.getFileSystem();
-  FS->setCurrentWorkingDirectory(Cmd.Directory);
+  auto FS = FSProvider.getFileSystem(Cmd.Directory);
   auto Buf = FS->getBufferForFile(AbsolutePath);
   if (!Buf)
     return llvm::errorCodeToError(Buf.getError());
   auto Hash = digest(Buf->get()->getBuffer());
 
   // Take a snapshot of the versions to avoid locking for each file in the TU.
   llvm::StringMap<ShardVersion> ShardVersionsSnapshot;
   {
for (auto &LS : Result) {
 
       IndexedSymbols.update(LS.AbsolutePath, std::move(SS), std::move(RS),
                             std::move(RelS), LS.CountReferences);
     }
   }
   Rebuilder.loadedShard(LoadedShards);
   Rebuilder.doneLoading();
 
-  auto FS = FSProvider.getFileSystem();
+  auto FS = FSProvider.getFileSystem(llvm::None);
   llvm::DenseSet<PathRef> TUsToIndex;
   // We'll accept data from stale shards, but ensure the files get reindexed
   // soon.
   for (auto &LS : Result) {
     if (!shardIsStale(LS, FS.get()))
       continue;
     PathRef TUForFile = LS.DependentTU;
     assert(!TUForFile.empty() && "File without a TU!");

clang-tools-extra/clangd/support/FSProvider.h

 //===--- FSProvider.h - VFS provider for ClangdServer ------------*- 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_FSPROVIDER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_FSPROVIDER_H
 
+#include "Path.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include <memory>
 
 namespace clang {
 namespace clangd {
 
 // Wrapper for vfs::FileSystem for use in multithreaded programs like clangd.
 // As FileSystem is not threadsafe, concurrent threads must each obtain one.
 class FileSystemProvider {
 public:
   virtual ~FileSystemProvider() = default;
   /// Called by ClangdServer to obtain a vfs::FileSystem to be used for parsing.
   /// Context::current() will be the context passed to the clang entrypoint,
   /// such as addDocument(), and will also be propagated to result callbacks.
   /// Embedders may use this to isolate filesystem accesses.
   virtual llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-  getFileSystem() const = 0;
+  getFileSystem(llvm::NoneType CWD) const = 0;
+
+  /// Similar to above one, except it will try to set curret working directory
+  /// to \p CWD.
+  /// This is an overload instead of an optional to make implicit string ->
+  /// StringRef conversion possible.
+  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
+  getFileSystem(PathRef CWD) const;
 };
 
 class RealFileSystemProvider : public FileSystemProvider {
 public:
   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-  getFileSystem() const override;
+      getFileSystem(llvm::NoneType) const override;
 };
 
 } // namespace clangd
 } // namespace clang
 
 #endif

clang-tools-extra/clangd/support/FSProvider.cpp

 //===--- FSProvider.cpp - VFS provider for ClangdServer -------------------===//
 //
 // 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/FSProvider.h"
+#include "Logger.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include <memory>
 
 namespace clang {
private:
 
   private:
     std::unique_ptr<File> Wrapped;
   };
 };
 } // namespace
 
 llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-clang::clangd::RealFileSystemProvider::getFileSystem() const {
+FileSystemProvider::getFileSystem(PathRef CWD) const {
+  auto FS = getFileSystem(llvm::None);
+  if (FS->setCurrentWorkingDirectory(CWD))
+    elog("Failed to set CWD in RealFileSystemProvider");
+  return FS;
+}
+
+llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
+clang::clangd::RealFileSystemProvider::getFileSystem(llvm::NoneType) const {
   // Avoid using memory-mapped files.
   // FIXME: Try to use a similar approach in Sema instead of relying on
   //        propagation of the 'isVolatile' flag through all layers.
   return new VolatileFileSystem(
       llvm::vfs::createPhysicalFileSystem().release());
 }
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/tool/ClangdMain.cpp

 #include "index/Serialization.h"
 #include "refactor/Rename.h"
 #include "support/Path.h"
 #include "support/Shutdown.h"
 #include "support/Trace.h"
 #include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
if (EnableClangTidy) {
     auto EmptyDefaults = tidy::ClangTidyOptions::getDefaults();
     EmptyDefaults.Checks.reset(); // So we can tell if checks were ever set.
     tidy::ClangTidyOptions OverrideClangTidyOptions;
     if (!ClangTidyChecks.empty())
       OverrideClangTidyOptions.Checks = ClangTidyChecks;
     ClangTidyOptProvider = std::make_unique<tidy::FileOptionsProvider>(
         tidy::ClangTidyGlobalOptions(),
         /* Default */ EmptyDefaults,
-        /* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem());
+        /* Override */ OverrideClangTidyOptions,
+        FSProvider.getFileSystem(llvm::None));
     Opts.GetClangTidyOptions = [&](llvm::vfs::FileSystem &,
                                    llvm::StringRef File) {
       // This function must be thread-safe and tidy option providers are not.
       tidy::ClangTidyOptions Opts;
       {
         std::lock_guard<std::mutex> Lock(ClangTidyOptMu);
         // FIXME: use the FS provided to the function.
         Opts = ClangTidyOptProvider->getOptions(File);

clang-tools-extra/clangd/unittests/ClangdTests.cpp

 #include "ClangdLSPServer.h"
 #include "ClangdServer.h"
 #include "CodeComplete.h"
 #include "GlobalCompilationDatabase.h"
 #include "Matchers.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "URI.h"
+#include "support/Path.h"
 #include "support/Threading.h"
 #include "clang/Config/config.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <algorithm>
)cpp";
 
   EXPECT_EQ(DumpParse1, DumpParse2);
   EXPECT_NE(DumpParse1, DumpParseDifferent);
 }
 
 TEST_F(ClangdVFSTest, PropagatesContexts) {
   static Key<int> Secret;
   struct FSProvider : public FileSystemProvider {
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem> getFileSystem() const override {
+    IntrusiveRefCntPtr<llvm::vfs::FileSystem>
+    getFileSystem(llvm::NoneType) const override {
       Got = Context::current().getExisting(Secret);
       return buildTestFS({});
     }
     mutable int Got;
   } FS;
   struct Callbacks : public ClangdServer::Callbacks {
     void onDiagnosticsReady(PathRef File, llvm::StringRef Version,
                             std::vector<Diag> Diagnostics) override {

 // Check that running code completion doesn't stat() a bunch of files from the
 // preamble again. (They should be using the preamble's stat-cache)
 TEST(ClangdTests, PreambleVFSStatCache) {
   class ListenStatsFSProvider : public FileSystemProvider {
   public:
     ListenStatsFSProvider(llvm::StringMap<unsigned> &CountStats)
         : CountStats(CountStats) {}
 
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem> getFileSystem() const override {
+    IntrusiveRefCntPtr<llvm::vfs::FileSystem>
+    getFileSystem(llvm::NoneType) const override {
       class ListenStatVFS : public llvm::vfs::ProxyFileSystem {
       public:
         ListenStatVFS(IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
                       llvm::StringMap<unsigned> &CountStats)
             : ProxyFileSystem(std::move(FS)), CountStats(CountStats) {}
 
         llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
         openFileForRead(const Twine &Path) override {

clang-tools-extra/clangd/unittests/FSTests.cpp

 namespace {
 
 TEST(FSTests, PreambleStatusCache) {
   llvm::StringMap<std::string> Files;
   Files["x"] = "";
   Files["y"] = "";
   Files["main"] = "";
   auto FS = buildTestFS(Files);
-  FS->setCurrentWorkingDirectory(testRoot());
 
   PreambleFileStatusCache StatCache(testPath("main"));
   auto ProduceFS = StatCache.getProducingFS(FS);
   EXPECT_TRUE(ProduceFS->openFileForRead("x"));
   EXPECT_TRUE(ProduceFS->status("y"));
   EXPECT_TRUE(ProduceFS->status("main"));
 
   EXPECT_TRUE(StatCache.lookup(testPath("x")).hasValue());

clang-tools-extra/clangd/unittests/HeadersTests.cpp

 #include "Compiler.h"
 #include "TestFS.h"
 #include "TestTU.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/PreprocessorOptions.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
 namespace {
std::unique_ptr<CompilerInstance> setupClang() {
 
     ParseInputs PI;
     PI.CompileCommand = *Cmd;
     PI.FSProvider = &FS;
     auto CI = buildCompilerInvocation(PI, IgnoreDiags);
     EXPECT_TRUE(static_cast<bool>(CI));
     // The diagnostic options must be set before creating a CompilerInstance.
     CI->getDiagnosticOpts().IgnoreWarnings = true;
-    auto VFS = FS.getFileSystem();
-    VFS->setCurrentWorkingDirectory(Cmd->Directory);
+    auto VFS = PI.FSProvider->getFileSystem(Cmd->Directory);
     auto Clang = prepareCompilerInstance(
         std::move(CI), /*Preamble=*/nullptr,
         llvm::MemoryBuffer::getMemBuffer(FS.Files[MainFile], MainFile),
         std::move(VFS), IgnoreDiags);
 
     EXPECT_FALSE(Clang->getFrontendOpts().Inputs.empty());
     return Clang;
   }

clang-tools-extra/clangd/unittests/PreambleTests.cpp

collectPatchedIncludes(llvm::StringRef ModifiedContents,
   auto CI = buildCompilerInvocation(PI, Diags);
   PP.apply(*CI);
   // Run preprocessor over the modified contents with patched Invocation. We
   // provide a preamble and trim contents to ensure only the implicit header
   // introduced by the patch is parsed and nothing else.
   // We don't run PP directly over the patch cotents to test production
   // behaviour.
   auto Bounds = Lexer::ComputePreamble(ModifiedContents, *CI->getLangOpts());
-  auto Clang =
-      prepareCompilerInstance(std::move(CI), &BaselinePreamble->Preamble,
-                              llvm::MemoryBuffer::getMemBufferCopy(
-                                  ModifiedContents.slice(0, Bounds.Size).str()),
-                              PI.FSProvider->getFileSystem(), Diags);
+  auto Clang = prepareCompilerInstance(
+      std::move(CI), &BaselinePreamble->Preamble,
+      PI.FSProvider->getFileSystem(PI.CompileCommand.Directory), Diags);
   PreprocessOnlyAction Action;
   if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) {
     ADD_FAILURE() << "failed begin source file";
     return {};
   }
   IncludeStructure Includes;
   Clang->getPreprocessor().addPPCallbacks(
       collectIncludeStructureCallback(Clang->getSourceManager(), &Includes));

clang-tools-extra/clangd/unittests/TestFS.h

 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTFS_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTFS_H
 #include "ClangdServer.h"
 #include "GlobalCompilationDatabase.h"
 #include "support/Path.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/VirtualFileSystem.h"
 
 namespace clang {
 namespace clangd {
 
 // Builds a VFS that provides access to the provided files, plus temporary
 // directories.
 llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
 buildTestFS(llvm::StringMap<std::string> const &Files,
             llvm::StringMap<time_t> const &Timestamps = {});
 
 // A VFS provider that returns TestFSes containing a provided set of files.
 class MockFSProvider : public FileSystemProvider {
 public:
-  IntrusiveRefCntPtr<llvm::vfs::FileSystem> getFileSystem() const override {
+  IntrusiveRefCntPtr<llvm::vfs::FileSystem> getFileSystem() const {
     return buildTestFS(Files, Timestamps);
   }
 
+  IntrusiveRefCntPtr<llvm::vfs::FileSystem>
+  getFileSystem(llvm::NoneType) const override {
+    return getFileSystem();
+  }
+
   // If relative paths are used, they are resolved with testPath().
   llvm::StringMap<std::string> Files;
   llvm::StringMap<time_t> Timestamps;
 };
 
 // A Compilation database that returns a fixed set of compile flags.
 class MockCompilationDatabase : public GlobalCompilationDatabase {
 public: