[PATCH] D84186: [clang][cli] Convert Analyzer option string based options to new option parsing system
Duncan P. N. Exon Smith via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 16 10:38:31 PST 2020
dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.
LGTM.
================
Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:261-263
/// The inlining stack depth limit.
- // Cap the stack depth at 4 calls (5 stack frames, base + 4 calls).
- unsigned InlineMaxStackDepth = 5;
+ unsigned InlineMaxStackDepth;
----------------
jansvoboda11 wrote:
> dexonsmith wrote:
> > Hmm, I wonder if this is entirely better. It's not obvious at all, looking at the code here, whether `InlineMaxStackDepth` can be used without getting initialized at all.
> >
> > I agree we should have the default value in two places -- I think removing assignment to 5 is the right thing to do -- but I'm not sure leaving this entirely uninitialized is a good thing. WDYT of initializing it to 0?
> I think leaving this uninitialized communicates the fact that it will be initialized somewhere else.
>
> If I saw `unsigned InlineMacStackDepth = 0;` and then observed it changing to `5` without passing `-analyzer-inline-max-stack-depth=5`, I'd be surprised.
Okay, that's fair.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84186/new/
https://reviews.llvm.org/D84186
More information about the cfe-commits
mailing list