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

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked an inline comment as done.
baloghadamsoftware 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();
----------------
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?


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

https://reviews.llvm.org/D73720