[PATCH] D111542: [analyzer] Retrieve VLA extent from its redeclaration.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@steakhal 
Thank you for your comments.



================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1663
     // We can trust a const value or a value of a global initializer in main().
-    const VarDecl *VD = VR->getDecl();
+    const VarDecl *VD = VR->getDecl()->getMostRecentDecl();
     if (VD->getType().isConstQualified() ||
----------------
steakhal wrote:
> I think you supposed to use the `getCanonicalDecl()` instead.
Using `getCanonicalDecl` does not fix the issue. I've checked.


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1673
+            const QualType T = VD->getType();
+            if (const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(T)) {
+              // Support one-dimensional array.
----------------
steakhal wrote:
> What does the function return if this branch is not taken?
> Shouldn't you handle `IncompleteArrayType` as well?
> What does the function return if this branch is not taken?
In case of a //false// branch it goes down the code to `getBindingForFieldOrElementCommon`.
> Shouldn't you handle `IncompleteArrayType` as well?
Only `ConstantArrayType` has `getSize()` method. And `getMostRecentDecl` give us an `ConstantArrayType` even if it is declared as `IncompleteArrayType`, but `getCanonicalDecl` does not.


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1718
     }
   }
 
----------------
steakhal wrote:
> I would favor not adding another level of indentation xD
It annoys me as well. I could use `goto` but I suppose you won't like it even more.  :-) 


================
Comment at: clang/test/Analysis/initialization.c:101
+
+const int glob_arr3[]; // VLA
+const int glob_arr3[4] = {1, 2, 3};
----------------
steakhal wrote:
> Why is this a VLA? I thought it's just an `IncompleteArrayType`.
> Could you please make sure that this is actually a VLA?
I mixed up in terminilogy. I'll fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111542