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();
----------------
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.


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