jhorstmann commented on a change in pull request #8571:
URL: https://github.com/apache/arrow/pull/8571#discussion_r518830136



##########
File path: rust/arrow/src/util/bit_chunk_iterator.rs
##########
@@ -55,48 +58,52 @@ impl<'a> BitChunks<'a> {
 pub struct BitChunkIterator<'a> {
     buffer: &'a Buffer,
     raw_data: *const u8,
-    offset: usize,
+    bit_offset: usize,
     chunk_len: usize,
     index: usize,
 }
 
 impl<'a> BitChunks<'a> {
+    /// Returns the number of remaining bits, guaranteed to be between 0 and 
63 (inclusive)
     #[inline]
     pub const fn remainder_len(&self) -> usize {
         self.remainder_len
     }
 
+    /// Returns the bitmask of remaining bits
     #[inline]
     pub fn remainder_bits(&self) -> u64 {
         let bit_len = self.remainder_len;
         if bit_len == 0 {
             0
         } else {
-            let byte_len = ceil(bit_len, 8);

Review comment:
       Exactly. If we have 7 remainder bits for example, if the offset starts 
at 0 it is enough to read `ceil(7, 8) = 1` byte, but if we start for example at 
offset 4 we need to combine bits from `ceil(4+7, 8) = 2` bytes. The further 
refactoring of the loop was then necessary because we may now need to combine 
bits from 9 bytes, and the previous code (which only shifted after the loop) 
would have overflowed an `u64`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to