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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to