[PATCH] D50855: [analyzer] pr37578: Fix lvalue/rvalue problem in field-of-temporary adjustments.
George Karpenkov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 21 10:55:11 PDT 2018
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
More information about the cfe-commits
mailing list