ASDenysPetrov added inline comments.
================
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};
----------------
ASDenysPetrov wrote:
> 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.
BTW, the first time this definition appeared only in C++17 in a footnote. The
committee is not yet confident enough about CHAR_BIT that it still resides
there :)
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
