baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:541-542
+ BR.markInteresting(It1);
+ if (const auto &LCV1 = It1.getAs<nonloc::LazyCompoundVal>()) {
+ BR.markInteresting(LCV1->getRegion());
+ }
----------------
baloghadamsoftware wrote:
> baloghadamsoftware wrote:
> > baloghadamsoftware wrote:
> > > Szelethus wrote:
> > > > NoQ wrote:
> > > > > baloghadamsoftware wrote:
> > > > > > NoQ wrote:
> > > > > > > I'm opposed to this code for the same reason that i'm opposed to
> > > > > > > it in the debug checker. Parent region is an undocumented
> > > > > > > implementation detail of `RegionStore`. It is supposed to be
> > > > > > > immaterial to the user. You should not rely on its exact value.
> > > > > > >
> > > > > > > @baloghadamsoftware Can we eliminate all such code from iterator
> > > > > > > checkers and instead identify all iterators by regions in which
> > > > > > > they're stored? Does my improved C++ support help with this
> > > > > > > anyhow whenever it kicks in?
> > > > > > How to find the region where it is stored? I am open to find better
> > > > > > solutions, but it was the only one I could find so far. If we
> > > > > > ignore `LazyCompoundVal` then everything falls apart, we can remove
> > > > > > all the iterator-related checkers.
> > > > > It's the region from which you loaded it. If you obtained it as
> > > > > `Call.getArgSVal()` then it's the parameter region for the call. If
> > > > > you obtained it as `Call.getReturnValue()` then it's the target
> > > > > region can be obtained by looking at the //construction context// for
> > > > > the call.
> > > > `LazyCompoundVal` and friends seem to be a constantly emerging headache
> > > > for almost everyone. For how long I've spent in the analyzer, and have
> > > > religiously studied conversations and your workbook about it, I still
> > > > feel anxious whenever it comes up.
> > > >
> > > > It would be great to have this documented in the code sometime.
> > > Do you mean `CallEvent::getParameterLocation()` for arguments? What is
> > > the //construction context// for the call? How can it be obtained?
> > I do not know exactly how many place `LazyCompoundVal` appears, but one
> > place for sure is in `MaterializeTemporaryExpr::getSubExpr()`. What to use
> > there instead?
> I also get it in the `Val` parameter of `checkBind()`.
Now I spent a whole day in vain. You probably mean
`ExprEngine::getObjectUnderConstruction()`, (which takes
`ConstructionContextItem` as its argument) but it turned out that there are no
objects under construction in `checkPostCall()`. (Stack dump says
`constructing_objects` as `null`.) It seems that the //only working solution//
is the current one. I am not opposed to find better working solutions, but we
cannot spend months to completely rewrite parts of the analyzer for such a
simple patch. And note tags are definitely needed for iterator checkers.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75677/new/
https://reviews.llvm.org/D75677
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
