balazske added inline comments.
================ Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:403 /// frame. May fail; returns null on failure. - const VarRegion *getParameterLocation(unsigned Index, - unsigned BlockCount) const; + const TypedValueRegion *getParameterLocation(unsigned Index, + unsigned BlockCount) const; ---------------- I would expect that this returns a `ParamRegion` (from the name of the function). (The implementation shows that the type can just be changed.) ================ 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: > 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? ================ Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:745 // Can only reasonably pretty-print DeclRegions. + if (const auto *DR = dyn_cast<DeclRegion>(R)) { ---------------- This comment is not fully true any more. ================ Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:255 // correspond to the stack frame's function declaration. assert(VR->getStackFrame() == SFC); ---------------- Is it correct to create `VR` only to have this assert? ================ Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:357 V = *OptV; - else - break; + else break; State = addObjectUnderConstruction(State, {CE, Idx}, LCtx, V); ---------------- This `break` is at wrong place. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79704/new/ https://reviews.llvm.org/D79704 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits