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