ChuanqiXu9 wrote: > Hello. If I understand correctly, with this patch we have the following logic: > > * we build preamble > > * we build AST without preamble usage if the TU uses C++20 modules > > > This means that we loose preamble optimization approach and at every key > stroke (file modification from an editor) in the file body we will rebuild > the whole AST (including `#include`s section). > > Maybe one can expect that C++20 modules usage means that preamble mostly > consists of C++20 modules, thus AST rebuild can be fast, but in general it > looks like performance degradation. > > If we consider this as a trade-off between performance loss and diagnostics > (which do not affect functionality? We can still do go-to-definition), > personally I more lean towards having this diagnostics, because: > > * We can suppress some diagnostics using configuration > https://clangd.llvm.org/config#suppress >
Yeah, I think you understand what I want to do. And what we have gaps is: - The support status of mixing PCH and C++20 named modules in clang, which is the base of clangd works. - The current status of modules support in clangd For the first point, as the maintainer of modules in clang, I deeply concerns the usablity of mixing PCH and named modules in clang. We have a few cerntain issue reports. And there are more clangd crash reports, which I suspect is due to mixing PCH and clang named modules. Since all these reports said the project can be built with clang but not clangd. And from implementors point of view, I don't think we can fix this in clang in any time soon. And for the second point, the status of modules support in clangd is experimental and the major problem for users right now is usability instead of performance. And the concern of performance exists but may not be huge to me. As the design of named modules is friendly to tools like clangd naturally. As with named modules, different tab (TU) can share BMIs. But with the preamble PCH, it is not shared. So I think this is a not perfect but workable and simple solution. > * As you said, the problem is not initially in clangd, but in clang. And > it seems we should not fix this in clangd at any cost. I feel this is too ideal. And from my point of view, from clang and clangd's perspective, including technical point and human resources and community ecosystem, this is the best solution we can have right now. If we say "oh, we prefer best solution", that will only delay the support of modules in clangd without a DDL. BTW, I am the original authors for modules support in clangd and I'd like to maintain clangd's modules support for a while too. So you don't have too worry about the maintainability for a relatively long time. https://github.com/llvm/llvm-project/pull/187432 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
