baloghadamsoftware marked an inline comment as done.
baloghadamsoftware 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:
> 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:)
So you both are saying that I should tell the user that we cannot help, the
community does not support that he catches this bug in early phase with the
analyzer. He must catch it in the testing phase for higher costs. This is not
the best way to build confidence in the analyzer.
In my perception options are used to serve one users need without harming
others. I never understand why there is so much resistance against introducing
an option that does not change the default behavior at all.
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
