[PATCH] D106681: [analyzer][NFC] Move a block from `getBindingForElement` to separate functions

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

So, you return `None` instead of returning `UndefinedVal`.
All in all, it looks good, aside from a couple stuff inline.



================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1636
+
+  // Array should be immutable.
+  const VarDecl *VD = VR->getDecl();
----------------
Please preserve the original comment.


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1650
+  // TODO: Support multidimensional array.
+  if (isa<ConstantArrayType>(CAT->getElementType())) // is multidimensional
+    return None;
----------------
What if that is a typedef?


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1686-1689
+  // Array's declaration should have an initializer.
+  const Expr *Init = VD->getAnyInitializer();
+  if (!Init)
+    return None;
----------------
This should have been checked at L1642 to preserve the original behavior.
As of now, the code might return `UndefinedVal` at L1683 or `Unknown` at L1659, which is probably different than the original behavior - unless you prove otherwise.


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1714
+  // Return a constant value, if it is presented.
+  // FIXME: Support other SVals.
+  const Expr *E = ILE->getInit(Offset);
----------------
What other values could appear in this context other than constants?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106681