kadircet updated this revision to Diff 270333.
kadircet added a comment.

- Use Cmd.Directory when creating includefixer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81720

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp

Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -70,11 +70,12 @@
   // 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),
+      llvm::MemoryBuffer::getMemBufferCopy(
+          ModifiedContents.slice(0, Bounds.Size).str()),
+      Diags, PI.CompileCommand, PI.FSProvider, &BaselinePreamble->Preamble,
+      BaselinePreamble->StatCache.get());
   PreprocessOnlyAction Action;
   if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) {
     ADD_FAILURE() << "failed begin source file";
Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -52,12 +52,12 @@
     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 Clang = prepareCompilerInstance(
-        std::move(CI), /*Preamble=*/nullptr,
+        std::move(CI),
         llvm::MemoryBuffer::getMemBuffer(FS.Files[MainFile], MainFile),
-        std::move(VFS), IgnoreDiags);
+        IgnoreDiags, *Cmd, &FS,
+        /*Preamble=*/nullptr,
+        /*FSCache=*/nullptr);
 
     EXPECT_FALSE(Clang->getFrontendOpts().Inputs.empty());
     return Clang;
Index: clang-tools-extra/clangd/index/Background.cpp
===================================================================
--- clang-tools-extra/clangd/index/Background.cpp
+++ clang-tools-extra/clangd/index/Background.cpp
@@ -243,9 +243,7 @@
   trace::Span Tracer("BackgroundIndex");
   SPAN_ATTACH(Tracer, "file", Cmd.Filename);
   auto AbsolutePath = getAbsolutePath(Cmd);
-
   auto FS = FSProvider.getFileSystem();
-  FS->setCurrentWorkingDirectory(Cmd.Directory);
   auto Buf = FS->getBufferForFile(AbsolutePath);
   if (!Buf)
     return llvm::errorCodeToError(Buf.getError());
@@ -269,8 +267,10 @@
                                    "Couldn't build compiler invocation");
 
   auto Clang =
-      prepareCompilerInstance(std::move(CI), /*Preamble=*/nullptr,
-                              std::move(*Buf), std::move(FS), IgnoreDiags);
+      prepareCompilerInstance(std::move(CI), std::move(*Buf), IgnoreDiags,
+                              Inputs.CompileCommand, &FSProvider,
+                              /*Preamble=*/nullptr,
+                              /*FSCache=*/nullptr);
   if (!Clang)
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "Couldn't build compiler instance");
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -242,10 +242,10 @@
   auto PreambleContents =
       llvm::MemoryBuffer::getMemBufferCopy(Contents.substr(0, Bounds.Size));
   auto Clang = prepareCompilerInstance(
-      std::move(CI), nullptr, std::move(PreambleContents),
+      std::move(CI), std::move(PreambleContents), IgnoreDiags, Cmd,
       // Provide an empty FS to prevent preprocessor from performing IO. This
       // also implies missing resolved paths for includes.
-      EmptyFSProvider()->getFileSystem(), IgnoreDiags);
+      EmptyFSProvider().get(), nullptr, nullptr);
   if (Clang->getFrontendOpts().Inputs.empty())
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "compiler instance had no inputs");
Index: clang-tools-extra/clangd/ParsedAST.h
===================================================================
--- clang-tools-extra/clangd/ParsedAST.h
+++ clang-tools-extra/clangd/ParsedAST.h
@@ -51,7 +51,7 @@
   /// If \p Preamble is non-null it is reused during parsing.
   /// This function does not check if preamble is valid to reuse.
   static llvm::Optional<ParsedAST>
-  build(llvm::StringRef Filename, const ParseInputs &Inputs,
+  build(PathRef AbsFilePath, const ParseInputs &Inputs,
         std::unique_ptr<clang::CompilerInvocation> CI,
         llvm::ArrayRef<Diag> CompilerInvocationDiags,
         std::shared_ptr<const PreambleData> Preamble);
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -12,6 +12,7 @@
 #include "AST.h"
 #include "Compiler.h"
 #include "Diagnostics.h"
+#include "FS.h"
 #include "Headers.h"
 #include "IncludeFixer.h"
 #include "Preamble.h"
@@ -241,28 +242,23 @@
 }
 
 llvm::Optional<ParsedAST>
-ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
+ParsedAST::build(PathRef AbsFilePath, 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();
-  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.
-  }
+  SPAN_ATTACH(Tracer, "File", AbsFilePath);
 
   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;
+  const PrecompiledPreamble *PreamblePCH = nullptr;
+  const PreambleFileStatusCache *PreambleStatCache = nullptr;
+  if (Preamble) {
+    PreamblePCH = &Preamble->Preamble;
+    PreambleStatCache = Preamble->StatCache.get();
+  }
 
   // This is on-by-default in windows to allow parsing SDK headers, but it
   // breaks many features. Disable it for the main-file (not preamble).
@@ -272,13 +268,14 @@
 
   llvm::Optional<PreamblePatch> Patch;
   if (Preamble) {
-    Patch = PreamblePatch::create(Filename, Inputs, *Preamble);
+    Patch = PreamblePatch::create(AbsFilePath, Inputs, *Preamble);
     Patch->apply(*CI);
   }
   auto Clang = prepareCompilerInstance(
-      std::move(CI), PreamblePCH,
-      llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, Filename), VFS,
-      ASTDiags);
+      std::move(CI),
+      llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, AbsFilePath),
+      ASTDiags, Inputs.CompileCommand, Inputs.FSProvider, PreamblePCH,
+      PreambleStatCache);
   if (!Clang)
     return None;
 
@@ -301,7 +298,7 @@
   llvm::Optional<tidy::ClangTidyContext> CTContext;
   {
     trace::Span Tracer("ClangTidyInit");
-    dlog("ClangTidy configuration for file {0}: {1}", Filename,
+    dlog("ClangTidy configuration for file {0}: {1}", AbsFilePath,
          tidy::configurationAsText(Inputs.Opts.ClangTidyOpts));
     tidy::ClangTidyCheckFactories CTFactories;
     for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
@@ -310,7 +307,7 @@
         tidy::ClangTidyGlobalOptions(), Inputs.Opts.ClangTidyOpts));
     CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
     CTContext->setASTContext(&Clang->getASTContext());
-    CTContext->setCurrentFile(Filename);
+    CTContext->setCurrentFile(AbsFilePath);
     CTChecks = CTFactories.createChecks(CTContext.getPointer());
     ASTDiags.setLevelAdjuster([&CTContext](DiagnosticsEngine::Level DiagLevel,
                                            const clang::Diagnostic &Info) {
@@ -356,18 +353,18 @@
   // Add IncludeFixer which can recover diagnostics caused by missing includes
   // (e.g. incomplete type) and attach include insertion fixes to diagnostics.
   llvm::Optional<IncludeFixer> FixIncludes;
-  auto BuildDir = VFS->getCurrentWorkingDirectory();
-  if (Inputs.Opts.SuggestMissingIncludes && Inputs.Index &&
-      !BuildDir.getError()) {
-    auto Style = getFormatStyleForFile(Filename, Inputs.Contents, VFS.get());
+  // FIXME: Why not use CompileCommand.Directory instead?
+  if (Inputs.Opts.SuggestMissingIncludes && Inputs.Index) {
+    auto Style = getFormatStyleForFile(
+        AbsFilePath, Inputs.Contents, Inputs.FSProvider->getFileSystem().get());
     auto Inserter = std::make_shared<IncludeInserter>(
-        Filename, Inputs.Contents, Style, BuildDir.get(),
+        AbsFilePath, Inputs.Contents, Style, Inputs.CompileCommand.Directory,
         &Clang->getPreprocessor().getHeaderSearchInfo());
     if (Preamble) {
       for (const auto &Inc : Preamble->Includes.MainFileIncludes)
         Inserter->addExisting(Inc);
     }
-    FixIncludes.emplace(Filename, Inserter, *Inputs.Index,
+    FixIncludes.emplace(AbsFilePath, Inserter, *Inputs.Index,
                         /*IndexRequestLimit=*/5);
     ASTDiags.contributeFixes([&FixIncludes](DiagnosticsEngine::Level DiagLevl,
                                             const clang::Diagnostic &Info) {
Index: clang-tools-extra/clangd/Compiler.h
===================================================================
--- clang-tools-extra/clangd/Compiler.h
+++ clang-tools-extra/clangd/Compiler.h
@@ -16,6 +16,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILER_H
 
 #include "../clang-tidy/ClangTidyOptions.h"
+#include "FS.h"
 #include "GlobalCompilationDatabase.h"
 #include "index/Index.h"
 #include "support/FSProvider.h"
@@ -68,15 +69,17 @@
 ///   - Preamble is overriden to use PCH passed to this function. It means the
 ///     changes to the preamble headers or files included in the preamble are
 ///     not visible to this compiler instance.
-///   - llvm::vfs::FileSystem is used for all underlying file accesses. The
-///     actual vfs used by the compiler may be an overlay over the passed vfs.
+///   - \p FSProvider is used for all underlying file accesses. The actual vfs
+///     used by the compiler may be an overlay over the passed vfs.
 /// Returns null on errors. When non-null value is returned, it is expected to
 /// be consumed by FrontendAction::BeginSourceFile to properly destroy \p
 /// MainFile.
 std::unique_ptr<CompilerInstance> prepareCompilerInstance(
-    std::unique_ptr<clang::CompilerInvocation>, const PrecompiledPreamble *,
-    std::unique_ptr<llvm::MemoryBuffer> MainFile,
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem>, DiagnosticConsumer &);
+    std::unique_ptr<clang::CompilerInvocation> CI,
+    std::unique_ptr<llvm::MemoryBuffer> Buffer, DiagnosticConsumer &DiagsClient,
+    const tooling::CompileCommand &Cmd, const FileSystemProvider *FSProvider,
+    const PrecompiledPreamble *Preamble,
+    const PreambleFileStatusCache *FSCache);
 
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Compiler.cpp
===================================================================
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -7,10 +7,13 @@
 //===----------------------------------------------------------------------===//
 
 #include "Compiler.h"
+#include "FS.h"
+#include "support/FSProvider.h"
 #include "support/Logger.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Serialization/PCHContainerOperations.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
 
@@ -92,17 +95,22 @@
   return CI;
 }
 
-std::unique_ptr<CompilerInstance>
-prepareCompilerInstance(std::unique_ptr<clang::CompilerInvocation> CI,
-                        const PrecompiledPreamble *Preamble,
-                        std::unique_ptr<llvm::MemoryBuffer> Buffer,
-                        llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
-                        DiagnosticConsumer &DiagsClient) {
-  assert(VFS && "VFS is null");
+std::unique_ptr<CompilerInstance> prepareCompilerInstance(
+    std::unique_ptr<clang::CompilerInvocation> CI,
+    std::unique_ptr<llvm::MemoryBuffer> Buffer, DiagnosticConsumer &DiagsClient,
+    const tooling::CompileCommand &Cmd, const FileSystemProvider *FSProvider,
+    const PrecompiledPreamble *Preamble,
+    const PreambleFileStatusCache *FSCache) {
+  assert(FSProvider && "FSProvider is null");
   assert(!CI->getPreprocessorOpts().RetainRemappedFileBuffers &&
          "Setting RetainRemappedFileBuffers to true will cause a memory leak "
          "of ContentsBuffer");
 
+  auto VFS = FSProvider->getFileSystem();
+  if (VFS->setCurrentWorkingDirectory(Cmd.Directory))
+    elog("Failed to set working directory while preparing compiler instance");
+  if (FSCache)
+    VFS = FSCache->getConsumingFS(std::move(VFS));
   // NOTE: we use Buffer.get() when adding remapped files, so we have to make
   // sure it will be released if no error is emitted.
   if (Preamble) {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1112,16 +1112,11 @@
     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();
-  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);
+      std::move(CI), std::move(ContentsBuffer), IgnoreDiags,
+      Input.ParseInput.CompileCommand, Input.ParseInput.FSProvider,
+      !CompletingInPreamble ? &Input.Preamble.Preamble : nullptr,
+      Input.Preamble.StatCache.get());
   Clang->getPreprocessorOpts().SingleFileParseMode = CompletingInPreamble;
   Clang->setCodeCompletionConsumer(Consumer.release());
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to