Szelethus added inline comments.
================
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:674
+ const LocationContext *LC = Context.getLocationContext();
+ while ((LC = LC->getParent())) {
+
----------------
Szelethus wrote:
> NoQ wrote:
> > george.karpenkov wrote:
> > > nit: could we have `while (LC)` followed by `LC = LC->getParent()` ? Do
> > > you intentionally skip the first location context?
> > I guess the predicate we're checking is trivially true for the current
> > location context.
> Completely missed this inline, sorry!
>
> As @NoQ said, since this checker only fires after a constructor call, the
> first location context will surely be that, and I'm only checking whether the
> current ctor was called by another.
I'm actually not sure how you imagined that. The loop condition is `LC`, but if
I assign it to its parent right after that, I won't be sure that it's non-null.
The reason why I placed it there is that early exits could restart the loop at
"random" places.
================
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:681-689
+ Optional<nonloc::LazyCompoundVal> OtherObject =
+ getObjectVal(OtherCtor, Context);
+ if (!OtherObject)
+ continue;
+
+ // If the CurrentObject is a field of OtherObject, it will be analyzed
+ // during the analysis of OtherObject.
----------------
NoQ wrote:
> It seems safer to look at `CXXConstructExpr::getConstructionKind()`.
>
> Taking a `LazyCompoundVal` and converting it back to a region is definitely a
> bad idea because the region within `LazyCompoundVal` is completely immaterial
> and carries no meaning for the value represented by this `SVal`; it's not
> necessarily the region you're looking for.
Looking at the singleton test case, I think I need to get that region somehow,
and I'm not too sure what you meant under using
`CXXConstructExpr::getConstructionKind()`. One thing I could do, is similar to
how `getObjectVal` works:
```
Loc Tmp = Context.getSValBuilder().getCXXThis(OtherCtor->getParent(),
Context.getStackFrame());
auto OtherThis = Context.getState()->getSVal(Tmp).castAs<loc::MemRegionVal>();
OtherThis.getRegion(); // Hooray!
```
I have tested it, and it works wonders. Is this a "safe-to-use" region?
https://reviews.llvm.org/D48436
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
