[PATCH] D111654: [analyzer] Retrieve a value from list initialization of multi-dimensional array declaration.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1643-1654
+  const VarRegion *VR = dyn_cast<VarRegion>(R->getSuperRegion());
+  SmallVector<SVal, 2> SValOffsets;
+  SValOffsets.push_back(R->getIndex());
+  if (const ElementRegion *ER = dyn_cast<ElementRegion>(R->getSuperRegion())) {
+    const ElementRegion *LastER = nullptr;
+    do {
+      SValOffsets.push_back(ER->getIndex());
----------------
steakhal wrote:
> I dislike this part of code. It has side effects all over the place.
> Please use immediately called lambdas to describe your intentions in a more //pure// manner.
> 
> Besides that, when I see `ElementRegions`, I'm always scared. They sometimes represent reinterpret casts.
> Since you have dealt with so many casts and region store stuff in the past, I'm assuming you have thought about this here.
> Although, I'm still scared slightly, and I'd like to have evidence of this thought process. Likely some assertions or at least some comments? Could you @ASDenysPetrov elaborate on this?
> Please use immediately called lambdas.
I'll do.
> They sometimes represent reinterpret casts. ...  I'm assuming you have thought about this here.
Yes. This is my another revision in the stack. You are welcome: D110927


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1656
+
+  // TODO: Add a test case for this check.
+  if (!VR)
----------------
martong wrote:
> Please address this TODO.
I found the tests:
  - Analysis/MemRegion.cpp
  - Analysis/ctor.mm
  - Analysis/mpichecker.cpp
  - Analysis/string.c
They cover this check.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111654/new/

https://reviews.llvm.org/D111654