[PATCH] D104285: [analyzer] Retrieve a value from list initialization of constant array declaration in a global scope.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1692-1694
+            const bool IsOneDimensionalArray =
+                !isa<ConstantArrayType>(CAT->getElementType());
+            if (IsOneDimensionalArray) {
----------------
aaron.ballman wrote:
> ASDenysPetrov wrote:
> > martong wrote:
> > > aaron.ballman wrote:
> > > > 
> > > +1 for Aaron's suggestion, but then it would be really helpful to have an explanatory comment. E.g.:
> > > ```
> > > if (!isa<ConstantArrayType>(CAT->getElementType())) { // This is a one dimensional array.
> > > ```
> > I think that self-descriptive code is better than comments nearby. And it does not affect anything in terms of performance.
> FWIW, I found the condensed form more readable (I don't have to wonder what's going to care about that variable later in the function with lots of nesting).
I see what you mean. That's fair in terms of variables caring. I think it's just the other hand of the approach. I don't have strong preferences here, it's just my personal flavor because it doesn't need to introspect the expression to undersand what it means. But I'll make an update using your proposition.


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

https://reviews.llvm.org/D104285