AaronBallman wrote: > > 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.
I've added more reviewers for more opinions. If we don't reach a good conclusion in a somewhat timely manner, then I think it makes sense to get the clang area team involved to schedule a meeting to discuss in real time to see if we can get unstuck. Please correct me if I'm wrong, but it sounds like the situation we're in is that Clang has bugs with modules + PCH and those bugs are exposed to clangd users more readily and cause a bad user experience. So the goal of this patch is to temporarily disable PCH from clangd when modules are also used, but the concerns are that users are still paying the cost of PCH generation but not getting the benefit with their use. If that's roughly correct, I think there's more work required here. I would prefer that users don't pay the extra cost they're not getting benefit out of, but if that's too much of an implementation burden to support for a stopgap measure, I think users have no good way to determine what's going on and maybe we should have diagnostics for this scenario. In fact, given that this is an underlying issue in Clang, should Clang be diagnosing that combination of flags to make it clear that it's currently an unsupported configuration? https://github.com/llvm/llvm-project/pull/187432 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
