Carlos =?utf-8?q?Gálvez?= <carlos.gal...@zenseact.com>, Carlos =?utf-8?q?Gálvez?= <carlos.gal...@zenseact.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/128...@github.com>
steakhal wrote: > I highly discourage implicit slowdowns > - If I understand correctly, you mean that this patch could lead to people > manually traversing the AST instead of using the MatchFinder, thus making > specific checks very slow. Correct? I'm not quite sure how to act upon this, > do you have a specific suggestion? I would hope deviating from the "standard > pattern" would be caught during code review. Otherwise, this would come out > as a bug report fairly soon, as it's easy to profile. So I don't see it as > high risk. and > @steakhal was against the checks implicitly triggering the "full analysis". If I'm not mistaken, currently the PR does not propose an alternative "full analysis" mode that we would silently switch to. So, this makes my concern void. What I wanted to highlight was that we should definitely not silently switch to doing something a lot more expensive. If we wanted to do something like that, we should have a clear way delivering log message or diagnostic to the user that we are falling back which will likely be a lot slower than it could be and this switch was caused by XYZ check. This would make it actionable from the user's perspective. But like I said, it's irrelevant, as we don't have this silent switch. That said, it would be probably nice to mention in the docs that enabling the `SystemHeaders` would likely incur some/significant performance penalty. I hope I clarified the misunderstanding that my comment may have caused. Sorry about that. https://github.com/llvm/llvm-project/pull/128150 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
