llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Chuanqi Xu (ChuanqiXu9) <details> <summary>Changes</summary> @<!-- -->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. --- Full diff: https://github.com/llvm/llvm-project/pull/113879.diff 1 Files Affected: - (modified) clang-tools-extra/clangd/ModulesBuilder.cpp (+16-10) ``````````diff 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 `````````` </details> https://github.com/llvm/llvm-project/pull/113879 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
