Commit 93ee3410 authored by Shawn Wu's avatar Shawn Wu Committed by Facebook GitHub Bot

EliasFano: fix an edge case in the conditional check during encoding.

Summary:
Context: we don't yet support uint8_t/uint16_t as SkipValueType due to integral promotion errors. Once we do (something like D22249713, D24571536), consider the below test case

```
using ValueType = uint64_t;
using SkipValueType = uint8_t;
Encode<ValueType, SkipValueType, ...>({0, std::numeric_limits<ValueType>::max() -1});
```

This would check fail at
```
CHECK_LT(upperBound >> numLowerBits, std::numeric_limits<SkipValueType>::max());
```

(255 vs 255)

In fact, I think CHECK_LE is a more correct check.

Also made another check fail to print nicer results for uint8 values.

Reviewed By: ot, philippv

Differential Revision: D24605502

fbshipit-source-id: e422ed1f959ff8ef63dfe99149340637a3741b2a
parent ae7f09e3
......@@ -256,9 +256,9 @@ struct EliasFanoEncoderV2<
// *** Validity checks.
// Shift by numLowerBits must be valid.
CHECK_LT(numLowerBits, 8 * sizeof(Value));
CHECK_LT(static_cast<int>(numLowerBits), 8 * sizeof(Value));
CHECK_LT(size, std::numeric_limits<SkipValueType>::max());
CHECK_LT(
CHECK_LE(
upperBound >> numLowerBits, std::numeric_limits<SkipValueType>::max());
return fromInternalSizes(numLowerBits, upper, size);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment