alamb commented on code in PR #9137:
URL: https://github.com/apache/arrow-rs/pull/9137#discussion_r2679461520


##########
arrow-buffer/src/buffer/mutable.rs:
##########
@@ -623,6 +623,139 @@ impl MutableBuffer {
         buffer
     }
 
+    /// Advances the buffer by `additional` bits without initializing the new 
bytes.
+    ///
+    /// # Safety
+    /// Callers must ensure that all newly added bits are written before the 
buffer is read.
+    #[inline]
+    unsafe fn advance_uninit(&mut self, additional: usize) {
+        let new_len = self.len + additional;
+        let new_len_bytes = bit_util::ceil(new_len, 8);
+        if new_len_bytes > self.len() {
+            self.reserve(new_len_bytes - self.len());
+            // SAFETY: caller will initialize all newly exposed bytes
+            unsafe { self.set_len(new_len_bytes) };
+        }
+        self.len = new_len;

Review Comment:
   Some thoughts reading this: It is pretty similar to `set_len` except: 
`set_len` doesn't change capacity and is in terms of bytes
   
   Some readability suggestions: 
   1. Make it work with bytes (not bits) and move the byte len calculation to 
`extend_bool_trusted_iter`
   2. Potentially just use `set_len` (maybe adding the reserve calculation)
   
   Since this is not a public API I don't think it is a big deal and/or we can 
do this a follow on PR



##########
arrow-buffer/src/buffer/mutable.rs:
##########
@@ -623,6 +623,139 @@ impl MutableBuffer {
         buffer
     }
 
+    /// Advances the buffer by `additional` bits without initializing the new 
bytes.
+    ///
+    /// # Safety
+    /// Callers must ensure that all newly added bits are written before the 
buffer is read.
+    #[inline]
+    unsafe fn advance_uninit(&mut self, additional: usize) {
+        let new_len = self.len + additional;
+        let new_len_bytes = bit_util::ceil(new_len, 8);
+        if new_len_bytes > self.len() {
+            self.reserve(new_len_bytes - self.len());
+            // SAFETY: caller will initialize all newly exposed bytes
+            unsafe { self.set_len(new_len_bytes) };
+        }
+        self.len = new_len;

Review Comment:
   Some thoughts reading this: It is pretty similar to `set_len` except: 
`set_len` doesn't change capacity and is in terms of bytes
   
   Some readability suggestions: 
   1. Make it work with bytes (not bits) and move the byte len calculation to 
`extend_bool_trusted_iter`
   2. Potentially just use `set_len` (maybe adding the reserve calculation)
   
   Since this is not a public API I don't think it is a big deal and/or we can 
do this a follow on PR



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to