akashchamp commented on PR #51310: URL: https://github.com/apache/arrow/pull/51310#issuecomment-5750661946
Yes, there are several hand-rolled NextRun loops that would collapse into a range-for. The clearest ones: cpp/src/arrow/compare.cc:685 (RangeDataEqualsImpl) loops while(true) on SetBitRunReader::NextRun, checks length == 0 to return, and sets result_ = false on the first mismatch, which becomes for (const auto run : IterateSetBitRuns(...)) with an early return; cpp/src/arrow/compute/kernels/pivot_internal.cc:134 searches for the first unset validity bit by accumulating null_pos += run.length, and with IterateBitRuns the PositionedBitRun already carries the position so it is a find-first with break; cpp/src/arrow/array/concatenate.cc:616, cpp/src/arrow/compute/kernels/scalar_if_else.cc:1136 and :1155, and cpp/src/arrow/compute/kernels/hash_aggregate_numeric.cc:652 all keep a manual position counter and an if (run.length == 0) break sentinel check around BitRunReader that IterateBitRuns makes unnecessary; cpp/src/arrow/util/list_util.cc:75 (MinViewOffset) has the same while(true) sh ape around SetBitRunReader. I have kept this PR to the new helpers plus tests so it stays easy to review and would convert those sites in a follow-up, but I am happy to do compare.cc here if you would rather see one real caller in this 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]
