vsavchenko added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:297-307
+bool clang::ento::RangeSet::isUnsigned() const {
+ return begin()->From().isUnsigned();
+}
+
+uint32_t clang::ento::RangeSet::getBitWidth() const {
+ return begin()->From().getBitWidth();
+}
----------------
We should add `assert(!isEmpty());` in all three
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:645-676
+ // CastRangeSize is an amount of all possible values of cast type.
+ // Example: `char` has 256 values; `short` has 65536 values.
+ // But in fact we use `amount of values` - 1, because
+ // we can't keep `amount of values of UINT64` inside uint64_t.
+ // E.g. 256 is an amount of all possible values of `char` and we can't keep
+ // it inside `char`.
+ // And it's OK, it's enough to do correct calculations.
----------------
Maybe we can extract this into something like `truncateTo` and the next `if` to
`convertTo` private methods?
================
Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:117-118
+ template <typename T>
+ static constexpr APSIntType APSIntTy = APSIntType(sizeof(T) * 8,
+ !is_signed_v<T>);
+
----------------
Great, that works too!
================
Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:120
+
+ template <typename T> const llvm::APSInt &from(T X) {
+ static llvm::APSInt Int = APSIntTy<T>.getZeroValue();
----------------
Default to `BaseType`?
================
Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:126
- Range from(const RawRange &Init) {
+ template <typename T> Range from(const RawRangeT<T> &Init) {
return Range(from(Init.first), from(Init.second));
----------------
Default to `BaseType`?
================
Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:130
- RangeSet from(const RawRangeSet &Init) {
+ template <typename T>
+ RangeSet from(RawRangeSetT<T> Init, APSIntType Ty = APSIntTy<BaseType>) {
----------------
Default to `BaseType`?
================
Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:131
+ template <typename T>
+ RangeSet from(RawRangeSetT<T> Init, APSIntType Ty = APSIntTy<BaseType>) {
RangeSet RangeSet = F.getEmptySet();
----------------
Unused parameter?
================
Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:721-799
+TYPED_TEST(RangeSetCastToNoopTest, RangeSetCastToNoopTest) {
+ // Just to reduce the verbosity.
+ using F = typename TypeParam::FromType; // From
+ using T = typename TypeParam::ToType; // To
+
+ using TV = TestValues<T>;
+ constexpr auto MIN = TV::MIN;
----------------
If loop and promotion share the same test case, why should we split them into
two groups?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103094/new/
https://reviews.llvm.org/D103094
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
