george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed.
looks good apart from minor nits ================ Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:744 + const Expr *InitWithAdjustments, const Expr *Result = nullptr, + const SubRegion **OutRegionWithAdjustments = nullptr); ---------------- In general I would say that pair is still preferable, but I agree that with pre-c++17 codebase it gets too verbose. ================ Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:423 // correctly in case (Result == Init). - State = State->BindExpr(Result, LC, Reg); + if (Result->isGLValue()) + State = State->BindExpr(Result, LC, Reg); ---------------- could we have braces here? Once we have "else" I would say we should have those. ================ Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2543 // Handle regular struct fields / member variables. - state = createTemporaryRegionIfNeeded(state, LCtx, BaseExpr); - SVal baseExprVal = state->getSVal(BaseExpr, LCtx); + const SubRegion *MR; + state = createTemporaryRegionIfNeeded(state, LCtx, BaseExpr, ---------------- Let's initialize that to nullptr https://reviews.llvm.org/D50855 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits