[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