NoQ accepted this revision.
NoQ added a comment.

Looks great, thanks!



================
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:696
 void ento::registerUninitializedObjectChecker(CheckerManager &Mgr) {
   auto Chk = Mgr.registerChecker<UninitializedObjectChecker>();
   Chk->IsPedantic = Mgr.getAnalyzerOptions().getBooleanOption(
----------------
Szelethus wrote:
> NoQ wrote:
> > george.karpenkov wrote:
> > > registerChecker passes through arguments to the constructor. Could we use 
> > > that instead of mutable fields?
> > Really? Yay. I never noticed that. I think currently all checkers that need 
> > options use mutable fields for that purpose, and everybody thought it was 
> > intended to work this way.
> > 
> > I'm not super worried about fields being mutable because all checker 
> > callbacks are `const` (so you're still protected from mutating them in all 
> > of your code), but that'd definitely make the code look better.
> Well, I didn't really find that to be the case. Even if it was, 
> `getBooleanOption(StringRef, /*DefaultVal*/ bool, const ento::CheckerBase 
> *);` depends on the constructed checker, so I don't think I can pull this off.
> 
> There actually is a way to get boolean options without having to pass the 
> checker as an argument, but then it wouldn't be a checker specific option:
> `-analyzer-config Pedantic=true`
> instead of
> `-analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true`.
Yeah, well, i guess if you just use `this` as the last argument of 
`getBooleanOption()`, you'll still have `CheckName` unset during construction.

A slight refactoring might help, but it doesn't currently work "out of the box".


https://reviews.llvm.org/D48285



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

Reply via email to