martong 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:
> 
+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.
```


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1696
+              const llvm::APSInt &Idx = CI->getValue();
+              const uint64_t I = static_cast<uint64_t>(Idx.getExtValue());
+              // Use `getZExtValue` because array extent can not be negative.
----------------
aaron.ballman wrote:
> 
This `static_cast` seems to be dangerous to me, it might overflow. Can't we 
compare `Idx` directly to `Extent`? I see that `Idx` is an `APSint` and 
`Extent` is an `APInt`, but  I think we should be able to handle the comparison 
on the APInt level b/c that takes care of the signedness. And the overflow 
situation should be handled as well properly with `APInt`, given from it's name 
"arbitrary precision int". In this sense I don't see why do we need `I` at all.


================
Comment at: clang/test/Analysis/initialization.c:1
-// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze 
-analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-config 
eagerly-assume=false  
-analyzer-checker=core.uninitialized.Assign,debug.ExprInspection -verify %s
 
----------------
I don't see how this change is related.  How could the tests work before having 
the `uninitialized.Assign` enabled before?


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