[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:713
+  StringRef Name;
+  if (const auto *DRE = dyn_cast<DeclRefExpr>(ContE->IgnoreParenCasts())) {
+    Name = DRE->getDecl()->getName();
----------------
baloghadamsoftware wrote:
> NoQ wrote:
> > NoQ wrote:
> > > baloghadamsoftware wrote:
> > > > baloghadamsoftware wrote:
> > > > > NoQ wrote:
> > > > > > Hmm, i think you should instead look at `ContReg`, i.e. whether it's a non-anonymous `VarRegion` or a `FieldRegion` or something like that (in other patches as well). It would work more often and it'll transparently handle references.
> > > > > Unfortunately it is a `SymRegion` so it does not work :-( (Even using `getMostDerivedRegion()` does not help.)
> > > > You mean the first checking form `SymbolicRegion`, then get its symbol, check for `SymbolRegionValue`, then get its `TypedValueRegion`, check for `DeclRegion` and use its `Decl`? This sound waaay more complicated and less readable. I am not sure which are the side cases: is it always `SymbolicRegion`? Is the `Symbol` of `SymbolicRegion` always a `SymbolRegionValue`? Is ithe `TypedValueRegion` (the return value of its `getRegion()`) always a `DeclRegion`?
> > > > Unfortunately it is a `SymRegion`
> > > 
> > > Emm, that's rarely the case. Only if it's a reference passed into a top-level function as a parameter.
> > (or to another unknown location) (please learn what `SymbolicRegion` is, it is very important for your work)
> > 
> > I guess you should do both then, because when the analyzer is able to resolve the reference it's better in this case to point out what is the actual container that's being modified.
> Is it OK now?
Looks good, can we also have tests for this case? I.e., anything where the container isn't passed by reference to an unknown location.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73720/new/

https://reviews.llvm.org/D73720