Charusso added a comment. In D66042#1624475 <https://reviews.llvm.org/D66042#1624475>, @Szelethus wrote:
> In D66042#1624469 <https://reviews.llvm.org/D66042#1624469>, @Charusso wrote: > > > In a long-term rewriting the Analyzer from scratch would be ideal. There is > > no problem with this patch, it will not cause any issues like that. May I > > would like to disable the apiModeling as well, with my patch it is one > > command. With your approach it would require to rewrite all of the existing > > apiModeling checkers after the core ones for no reason as we have better > > way: this patch. > > > Why would we need to rewrite apiModeling? Its hidden from users, so the issue > of enabling/disabling them is not a big topic. May the analyzer thinks that my `error()` function is inside some Apple product, which we model in some Apple way, but let us say I am at Microsoft, and I do not want to fix any Apple based product, but at least I want to hide those diagnostics. See that problem in https://reviews.llvm.org/D63915?id=207135#inline-570462. > Here is a problem with your patch: How would you go about silencing > diagnostics for `osx.cocoa.RetainCount`? I suppose > `-analyzer-silence-checker=osx.cocoa.RetainCount`. The problem however, that > the checker tag associated with it refers to `osx.cocoa.RetainCountBase` > under the hood, so you'll need to silence that instead. From that point on, > any other checker that suffers from the same issue is also silenced, that was > not the intent. > > I genuinely mean to cause no unnecessary headaches for you, but adding this > is a significant API change that we'll have to support going forward if it > gets into a release. If this is a quick fix to reduce the false positives on > LLVM, I'm all for it, but would prefer to see a solution without such a > commitment. If I am at Microsoft, I would say `-disable-checker osx` to prevent issues like above. Of course it would be better if we do not have to suppress 7.000 reports running the Analyzer on LLVM using just the ReturnVisitor's suppressing. So on a long-term rewrite from scratch would be ideal. For now it is a cool patch without any overhead. You have specified those checker dependencies very well, so if someone want to silence something then there is a cool place to starts with. Other than that the user's experimental features will remain experimental as it is still better than disabling the core and rush for Bugzilla how bad is the Analyzer. The goal here to have a way to hide meaningless reports which could produced by the core checkers as well. If we see some errors we rewrite it later, that means being experimental, that is how the usual developing process goes, that is why there is already a green mark: we are good to go. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66042/new/ https://reviews.llvm.org/D66042 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
