ArcsinX wrote: > Here is not a complete list: > > #130280 #148370 #80570 > [clangd/clangd#2536](https://github.com/clangd/clangd/issues/2536) > [clangd/clangd#1981](https://github.com/clangd/clangd/issues/1981)
You said that a lot of users don't create an issue, but "just saying". My question was addressed to this statement. BTW, if this patch fixes these issues, maybe we can mention these first > Could you clarify the Policy details? Most of your arguments are about limited resources and solution simplicity, but we should focus on quality https://llvm.org/docs/DeveloperPolicy.html#quality Here we have performance regression in clangd (but we should not). So, we have a bug in clang, but hiding this bug in clangd with the patch which leads to performance degradation in some clangd scenarios (these scenarios had no problems before). I don't see in the policy that such a patch is acceptable if we have limited resources to can't provide a correct solution in a short period of time. > > Modules support feature was already introduced in clangd (by you) and > > together with this patch we introduce performance degradation in some > > scenarios (with this feature enabled) and these scenarios had no problems > > before. > > For the current support status of modules in clangd, I think it is not so > meaningful to talk about performance for modules user right now. It is > basically not working or works rarely. And also the performance loss should > be limited. On the one hand, building PCH is relatively fast. On the other > hand, in modules uses, the number of #include will be limited. I think we can accept performance problem if the preamble is not build at all (when TU contains modules imports/modules support is enabled). In this patch we build the preamble (updating index while building, collecting includes information, etc.) and drop it after that, this looks like something wrong. This also inacceptable to you with the same argument about resources. https://github.com/llvm/llvm-project/pull/187432 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
