https://github.com/ArcsinX commented:

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
- 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.

https://github.com/llvm/llvm-project/pull/187432
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to