ASDenysPetrov added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:200-204 + } else { + // [ First ]------> + // [ Second ]---> + // MIN^ + // The First range is entirely inside the Second one. ---------------- steakhal wrote: > Might be `First->To() == Second->To()`. In this case the comment is not > completely accurate. I'll update. ================ Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:81 const llvm::APSInt &from(BaseType X) { - llvm::APSInt Dummy = Base; - Dummy = X; - return BVF.getValue(Dummy); + static llvm::APSInt Base{sizeof(BaseType) * 8, + std::is_unsigned<BaseType>::value}; ---------------- steakhal wrote: > ASDenysPetrov wrote: > > steakhal wrote: > > > ASDenysPetrov wrote: > > > > steakhal wrote: > > > > > Shouldn't you use `sizeof(BaseType) * CHAR_BIT` instead? > > > > Agree. It's better to avoid magic numbers. I'll fix. > > > It's not only that but just imagine testing a clang on a special hardware > > > where they have let's say 9 bit bytes, for parity or something similar > > > stuff. > > > The test would suddenly break. > > > Although I suspect there would be many more things to break TBH xD > > I am always skeptical about using`CHAR_BIT`, beacuse it represents bit > > number in `char`. And what if it would be 16 for instance (aka 2 bytes). > > But my intention is to get an amount of bits for a particular type. And I > > want something to represent a number of bits in a byte as a fundamental > > unit, but not something that depends on a `char` size on a particular > > platform. > > I would better introduce something like `constexpr size_t BITS_IN_BYTE = > > 8;`. > [[ https://eel.is/c++draft/basic.memobj#footnote-22 | basic.memobj 6.7.1/22 > ]]: > >The number of bits in a byte is reported by the macro `CHAR_BIT` in the > >header `<climits>`. These legacy names... :) I'll update. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99797/new/ https://reviews.llvm.org/D99797 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits