[PATCH] D79704: [Analyzer] [NFC] Parameter Regions

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp:176-180
   if (const auto *DeclReg = Reg->getAs<DeclRegion>()) {
     if (isa<ParmVarDecl>(DeclReg->getDecl()))
       Reg = C.getState()->getSVal(SV.castAs<Loc>()).getAsRegion();
+  } else if (const auto *ParamReg = Reg->getAs<ParamRegion>()) {
+    Reg = C.getState()->getSVal(SV.castAs<Loc>()).getAsRegion();
----------------
baloghadamsoftware wrote:
> balazske wrote:
> > baloghadamsoftware wrote:
> > > Szelethus wrote:
> > > > This is interesting. I looked up `DeclRegion`, and it seems to be the region that is tied to a `ValueDecl`. `VarDecl` is a subtype of `ValueDecl`, and `ParmVarDecl` is a subtype of `VarDecl`, so wouldn't it make sense for `ParamRegion` to be a subtype of `VarRegion`?
> > > `DeclRegion` stores the `Decl`, `ParamRegion` retrieves it based on the `Index` it stores. There is no is-a relation between them.
> > During the lifetime of a `ParamRegion` is it possible that it will return different `Decl` objects?
> @NoQ?
Purpose of the question is that if the answer is "no", the decl could be stored at construction of a `ParamRegion` and it could be a subclass of `DeclRegion`. From the code I do not see that the belonging `Decl` can change, probably yes if the stack frame changes somehow. So the reason to have the `ParamRegion` seems to be to have the expression and index available, maybe use these for profiling (instead of a `Decl`). Still it could be made a subclass of `DeclRegion` that stores additionally the expr and index and profiling works with these values.


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

https://reviews.llvm.org/D79704