baloghadamsoftware marked an inline comment as done. baloghadamsoftware added inline comments.
================ Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:1044 +class ParamWithoutVarRegion : public TypedValueRegion { + friend class MemRegionManager; ---------------- NoQ wrote: > baloghadamsoftware wrote: > > baloghadamsoftware wrote: > > > baloghadamsoftware wrote: > > > > NoQ wrote: > > > > > There should be only one way to express the parameter region. Let's > > > > > call this one simply `ParamRegion` or something like that, and > > > > > `assert(!isa<ParmVarDecl>(D))` in the constructor of `VarRegion`. > > > > Do you mean that we should use `ParamRegion` in every case, thus also > > > > when we have the definitioan for the function? I wonder whether it > > > > breaks too many things. > > > This will surely not work. The common handling of `ParamVarDecl` and > > > `VarDecl` is soo deeply rooted in the whole analyzer that separating them > > > means creation of a totally new analyzer engine from scratch. > > More specifically: whenever a function is inlined, its parameters are used > > as variables via `DeclRefExpr`s. A `DeclRefExpr` refers to a `Decl` which > > is a `ParamVarDecl` but that has reference neither for the `CallExpr` > > (since it is not related to the call, but to the `FunctionDecl` or > > `ObjCMethodDecl`) nore for its `Index` in the call. The former is the real > > problem that cannot be solved even on theoretical level: a function which > > is inlined cannot depend on the different `CallExpr`s where it is called. > > Even worse, if the function is analyzed top-level it has not `CallExpr` at > > all so using `ParamRegion` for its parameters is completely impossible. > > A `DeclRefExpr` refers to a `Decl` which is a `ParamVarDecl` but that has > > reference neither for the `CallExpr` (since it is not related to the call, > > but to the `FunctionDecl` or `ObjCMethodDecl`) > > The call site is available as part of the current location context. > > > nore for its Index in the call > > It's available as part of `ParmVarDecl`. > > > The former is the real problem that cannot be solved even on theoretical > > level: a function which is inlined cannot depend on the different > > `CallExpr`s where it is called > > It already depends on the `CallExpr`. `ParmVarDecl`-based `VarRegion`s are > different for different call sites even if `ParmVarDecl` is the same; > moreover, they reside in non-overlapping memory spaces. > > > Even worse, if the function is analyzed top-level it has not `CallExpr` at > > all so using `ParamRegion` for its parameters is completely impossible. > > Ok, let's make an exception for this case and either use the old `VarRegion` > (given that there's no redecl confusion in this case) or allow the `CallExpr` > to be null (it's still trivially easy to extract all the necessary > information). > > > The common handling of `ParamVarDecl` and `VarDecl` is soo deeply rooted in > > the whole analyzer that separating them means creation of a totally new > > analyzer engine from scratch. > > I don't see any indication of that yet. > or allow the `CallExpr` to be `null` How do we calculate the type then? Or should we store it explicitly? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77229/new/ https://reviews.llvm.org/D77229 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits