[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

Thu Aug 31 01:33:36 PDT 2023

ChuanqiXu added a comment.

In D153114#4630408 <https://reviews.llvm.org/D153114#4630408>, @nridge wrote:

> In D153114#4630318 <https://reviews.llvm.org/D153114#4630318>, @ChuanqiXu wrote:
>
>> However, I can't search the caller of `reparseOpenFilesIfNeeded` which semantics matches the behavior. The two callers of  `reparseOpenFilesIfNeeded` I found are `ClangdLSPServer::applyConfiguration()` and `ClangdLSPServer::onDocumentDidSave()` and neither of them matches description `files included in the  preamble have been modified`.
>>
>> So I want to ask what's the behavior when I update a header and where is the corresponding code. Thanks.
>
> I'm afraid `onDocumentDidSave()` is all we have for now. It detects changes to the header when editing the header in the client (when the header is saved). I don't believe we have a mechanism for detecting changes to the header made in other ways.

IIUC, when we open `a.cpp` and `b.h` (there is no relationship between them), and we edit and save `b.h`, then the AST Worker of `a.cpp` will receive a request to update `a.cpp`. Did I understand correct? I imaged there may be an early exit point in the path of `ASTWorker::update` or `PreambleThread::update` if we detects the preamble doesn't change actually. But I failed to find it.

> If/when we want to add such a mechanism, I think the way to do it is using didChangeWatchedFiles (there is some discussion there about why LSP recommends servers delegate file-watching to the client rather than implementing file-watching in the server).

Got it. And I am wondering the reason we didn't implement it may be that it is not so bad actually. Since a user generally won't open too many tabs. Do I understand right? And if it is the case, maybe we need to look at it in the future since it may be a concern with modules.

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

https://reviews.llvm.org/D153114