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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to