[clang-tools-extra] [clangd] [Modules] Add VFS to ASTUnit::LoadFromASTFile (PR #113879)

Chuanqi Xu via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 28 01:03:16 PDT 2024


https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/113879

@kadircet mentioned in https://github.com/llvm/llvm-project/commit/448d8fa880be5cae0f63c3b248f07f647013a5a4#diff-fb3ba8a781117ff04736f951a274812cb7ad1678f9d71d4d91870b711ab45da0L285 that:

> this is definitely a functional change, clangd is used in environments that solely relies on VFS, and doesn't depend on ASTUnit at all.

> right now this is both introducing a dependency on ASTUnit, and making all the logical IO physical instead. can you instead use the regular compiler utilities in clangd, and get the astreader from CompilerInstance directly, which is VFS-aware, and doesn't depend on ASTUnit ?

But I like ASTUnit since I feel it is a better abstract here. For compiler instance, we need to provide the compiler invocation. But for the purpose here. we're just trying to open the BMI file to verify if the source files are out of date or not. We don't need compiler invocation here completely. So I prefer ASTUnit here especially clangd already depends on clangFrontend already so I feel like I didn't introduce a new dependency.

Then I added the `VFS` argument to `ASTUnit` to address the Virtual/Physical FS problem.

>From 3df5e9275a63ee9c51c4e9e9a77383a93be020a4 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Mon, 28 Oct 2024 15:54:37 +0800
Subject: [PATCH] [clangd] [Modules] Add VFS to ASTUnit::LoadFromASTFile

---
 clang-tools-extra/clangd/ModulesBuilder.cpp | 26 +++++++++++++--------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp
index 1eeff468ef1236..25f9ab6631fca7 100644
--- a/clang-tools-extra/clangd/ModulesBuilder.cpp
+++ b/clang-tools-extra/clangd/ModulesBuilder.cpp
@@ -127,10 +127,10 @@ struct ModuleFile {
   std::string ModuleFilePath;
 };
 
-bool IsModuleFileUpToDate(
-    PathRef ModuleFilePath,
-    const PrerequisiteModules &RequisiteModules) {
-IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
+bool IsModuleFileUpToDate(PathRef ModuleFilePath,
+                          const PrerequisiteModules &RequisiteModules,
+                          llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
+  IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
       CompilerInstance::createDiagnostics(new DiagnosticOptions());
 
   auto HSOpts = std::make_shared<HeaderSearchOptions>();
@@ -141,7 +141,11 @@ IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
   PCHContainerOperations PCHOperations;
   std::unique_ptr<ASTUnit> Unit = ASTUnit::LoadFromASTFile(
       ModuleFilePath.str(), PCHOperations.getRawReader(), ASTUnit::LoadASTOnly,
-      Diags, FileSystemOptions(), std::move(HSOpts));
+      Diags, FileSystemOptions(), std::move(HSOpts),
+      /*LangOpts=*/nullptr, /*OnlyLocalDecls=*/false,
+      /*CaptureDiagnostics=*/CaptureDiagsKind::None,
+      /*AllowASTWithCompilerErrors=*/false, /*UserFilesAreVolatile=*/false,
+      VFS);
 
   if (!Unit)
     return false;
@@ -167,10 +171,12 @@ IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
 
 bool IsModuleFilesUpToDate(
     llvm::SmallVector<PathRef> ModuleFilePaths,
-    const PrerequisiteModules &RequisiteModules) {
-  return llvm::all_of(ModuleFilePaths, [&RequisiteModules](auto ModuleFilePath) {
-    return IsModuleFileUpToDate(ModuleFilePath, RequisiteModules);
-  });
+    const PrerequisiteModules &RequisiteModules,
+    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
+  return llvm::all_of(
+      ModuleFilePaths, [&RequisiteModules, VFS](auto ModuleFilePath) {
+        return IsModuleFileUpToDate(ModuleFilePath, RequisiteModules, VFS);
+      });
 }
 
 // StandalonePrerequisiteModules - stands for PrerequisiteModules for which all
@@ -347,7 +353,7 @@ bool StandalonePrerequisiteModules::canReuse(
   SmallVector<StringRef> BMIPaths;
   for (auto &MF : RequiredModules)
     BMIPaths.push_back(MF.ModuleFilePath);
-  return IsModuleFilesUpToDate(BMIPaths, *this);
+  return IsModuleFilesUpToDate(BMIPaths, *this, VFS);
 }
 
 } // namespace clangd



More information about the cfe-commits mailing list