[PATCH] D50855: [analyzer] pr37578: Fix lvalue/rvalue problem in field-of-temporary adjustments.

[George Karpenkov via Phabricator via cfe-commits]

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