Szelethus added inline comments.

================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:647-653
     CmdLineOption<Boolean,
                   "AggressiveStdFindModeling",
                   "Enables exploration of the failure branch in std::find-like 
"
                   "functions.",
                   "false",
                   Released>
   ]>,
----------------
Szelethus wrote:
> NoQ wrote:
> > baloghadamsoftware wrote:
> > > Szelethus wrote:
> > > > Ah, okay, I see which one you refer to. We should totally make this 
> > > > non-user facing as well. 
> > > The option is not about state split! It is for choosing between the 
> > > (default) conservative approach and a more aggressive one. It is 
> > > absolutely for the user to set. Some users prefer less false positive for 
> > > the price of losing true positives. However, some other users prefer more 
> > > true positives for the price of additional false positives. This is why 
> > > we have checker options to be able to serve both groups.
> > > 
> > > This feature was explicitly requested by our users who first disabled 
> > > iterator checkers because of the too many false positives but later 
> > > re-enabled them because they run into a bug in a production system which 
> > > could have been prevented by enabling them. However, they run into 
> > > another bug that our checker did not find because of its conservative 
> > > behavior. They requested a more aggressive one but we must not do it for 
> > > everyone. The concept of the Analyzer is that we apply the conservative 
> > > approach by default and the aggressive can be enabled by analyzer and 
> > > checker options.
> > These options are unlikely to be on-by-default in vanilla clang because of 
> > having a fundamentally unfixable class of false positives associated with 
> > them. Honestly, i've never seen users who are actually ok with false 
> > positives. I've regularly seen users say that they are ok with false 
> > positives when they wanted their false negatives fixed, but it always 
> > turned out that they don't understand what they're asking for and they 
> > quickly changed their mind when they saw the result. But you guys basically 
> > vend your own tool to your own customers and bear your own responsibility 
> > and i therefore can't tell you what to do in your realm, and i'm ok with 
> > having options that allow you to disagree with my choices.
> > 
> > inb4, "//`-analyzer-config` is not user-facing!!!11//"
> >inb4, "-analyzer-config is not user-facing!!!11"
> I was starting to breath really heavy and type really fast :D
> 
> Jokes aside, I'll spill the beans, we don't have a really convenient way to 
> tweak these options through CodeChecker just yet, so for the time being, they 
> really aren't, but maybe one day.
> 
> With that said, I also agree with you regarding options that overapproximate 
> the analysis (greater code coverage with more false and true positives). User 
> facing options should be the ones where the nature of the bug itself if 
> subjective, like whether we want to check the initializedness of pointees. We 
> sure can regard them as unwanted, or we could regard them as totally fine, so 
> an option is a great compromise.
> 
> >[...] it always turned out that they don't understand what they're asking 
> >for [...]
> 
> This is why I think this shouldn't be user-facing. You need to be quite 
> knowledgeable about the analyzer, and IteratorChecker in particular to make 
> good judgement about enabling the option added in this patch. That doesn't 
> mean you shouldn't experiment with it, but I'd prefer to not expose it too 
> much.
> 
> I just don't see this feature being that desired by many if they knew the 
> costs, even if one user really wants it. We could however, in that case, say
> "Hey, there is this option you can enable that might help you hunt down more 
> bugs. It does change the actual analysis itself, and experience showed that 
> it might result in more false positives and could impact performance, so we 
> opted not to make it user-facing, but it is there, if you feel adventurous."
Granted, I might be demonizing the cons of these options too much, but the 
description doesn't help on understanding it that much either O:)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77150/new/

https://reviews.llvm.org/D77150



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to