NoQ added inline comments.
================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:620-624
+ "AggressiveEraseModeling",
+ "Enables exploration of the past-the-end branch for the "
+ "return value of the erase() method of containers.",
+ "false",
+ Released>
----------------
Szelethus wrote:
> Hmm. The description isn't really user-friendly, imo, but the option doesn't
> sound too user-facing either. I don't think this is an option to be tinkered
> with. I think we should make this hidden.
>
> Also, even for developers, I find this a bitconfusing at first. Do we not
> enable that by default? Why not? What do we have to gain/lose?
What is the rule that the user needs to follow in order to avoid the warning?
"Always check the return value of `erase()` before use"? If so, a good
description would be along the lines of "Warn when the return value of
`erase()` is not checked before use; compare that value to end() in order to
suppress the warning" (or something more specific).
================
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>
]>,
----------------
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//"
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
