[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency
Denys Petrov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 22 09:24:25 PDT 2021
ASDenysPetrov added a comment.
@martong
Thanks for your inlines. I'll update the patch.
================
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:
> > > 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;`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99797/new/
https://reviews.llvm.org/D99797
More information about the cfe-commits
mailing list