[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:448-449
 
   Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl->getParent(),
                                                     Context.getStackFrame());
 
----------------
Szelethus wrote:
> NoQ wrote:
> > This totally needs `assert(CtorDecl == Context.getStackFrame()->getDecl())`. Otherwise we're in big trouble because we'll be looking into a this-region that doesn't exist on this stack frame.
> > 
> > On second thought, though, i guess we should put this assertion into the constructor of `CXXThisRegion`. I'll do this.
> > 
> > Also there's an overload of `getCXXThis` that accepts the method itself, no need to get parent.
> Ummmm that wouldn't be very nice, because...
Yeah, i guess i'll have to think a bit deeper about this. I really want to prevent invalid `CXXThisRegion`s from appearing, but it might be not that simple.


================
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:456-483
 static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor,
                                       CheckerContext &Context) {
 
-  Optional<nonloc::LazyCompoundVal> CurrentObject = getObjectVal(Ctor, Context);
-  if (!CurrentObject)
+  const TypedValueRegion *CurrRegion = getConstructedRegion(Ctor, Context);
+  if (!CurrRegion)
     return false;
 
----------------
Szelethus wrote:
> ...`willBeAnalyzerLater()` relies on this, and it uses all sorts of constructor decls to check whether `Context.getLocationContext()->getDecl()` would be a subregion of another object. Are you sure that this is incorrect?
I mean not the this-region of the object, but the `CXXThisRegion` itself, in which this-region is stored. It is definitely not aliased across stack frames.


Repository:
  rC Clang

https://reviews.llvm.org/D51300