Skip to content

Commit

Permalink
Merge pull request #115 from Alexhuszagh/unsafe_iter
Browse files Browse the repository at this point in the history
Ensure BytesIter is an unsafe trait.
  • Loading branch information
Alexhuszagh authored Sep 9, 2024
2 parents 24a1d6a + a367315 commit f3d3cc1
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 7 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/Cifuzz.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: CIFuzz
on: [pull_request]
on: [pull_request, workflow_dispatch]
jobs:
Fuzzing:
runs-on: ubuntu-latest
Expand Down
8 changes: 7 additions & 1 deletion lexical-util/src/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@ pub use crate::skip::{AsBytes, Bytes};
/// A default implementation is provided for slice iterators.
/// This trait **should never** return `null` from `as_ptr`, or be
/// implemented for non-contiguous data.
pub trait BytesIter<'a>: Iterator<Item = &'a u8> {
///
/// # Safety
/// The safe methods are sound as long as the caller ensures that
/// the methods for `read_32`, `read_64`, etc. check the bounds
/// of the underlying contiguous buffer and is only called on
/// contiguous buffers.
pub unsafe trait BytesIter<'a>: Iterator<Item = &'a u8> {
/// Determine if each yielded value is adjacent in memory.
const IS_CONTIGUOUS: bool;

Expand Down
2 changes: 1 addition & 1 deletion lexical-util/src/noskip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ pub struct BytesIterator<'a: 'b, 'b, const __: u128> {
byte: &'b mut Bytes<'a, __>,
}

impl<'a: 'b, 'b, const __: u128> BytesIter<'a> for BytesIterator<'a, 'b, __> {
unsafe impl<'a: 'b, 'b, const __: u128> BytesIter<'a> for BytesIterator<'a, 'b, __> {
const IS_CONTIGUOUS: bool = Bytes::<'a, __>::IS_CONTIGUOUS;

#[inline(always)]
Expand Down
8 changes: 4 additions & 4 deletions lexical-util/src/skip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ impl<'a, const FORMAT: u128> Bytes<'a, FORMAT> {
/// Try to read a the next four bytes as a u32.
/// This advances the internal state of the iterator.
#[inline(always)]
unsafe fn read_u32(&self) -> Option<u32> {
fn read_u32(&self) -> Option<u32> {
if Self::IS_CONTIGUOUS && self.as_slice().len() >= mem::size_of::<u32>() {
// SAFETY: safe since we've guaranteed the buffer is greater than
// the number of elements read. u32 is valid for all bit patterns
Expand All @@ -454,7 +454,7 @@ impl<'a, const FORMAT: u128> Bytes<'a, FORMAT> {
/// Try to read a the next four bytes as a u32.
/// This advances the internal state of the iterator.
#[inline(always)]
unsafe fn read_u64(&self) -> Option<u64> {
fn read_u64(&self) -> Option<u64> {
if Self::IS_CONTIGUOUS && self.as_slice().len() >= mem::size_of::<u64>() {
// SAFETY: safe since we've guaranteed the buffer is greater than
// the number of elements read. u64 is valid for all bit patterns
Expand Down Expand Up @@ -721,7 +721,7 @@ macro_rules! skip_iterator_byteiter_base {
/// Create impl ByteIter block for skip iterator.
macro_rules! skip_iterator_byteiter_impl {
($iterator:ident, $mask:ident, $i:ident, $l:ident, $t:ident, $c:ident) => {
impl<'a: 'b, 'b, const FORMAT: u128> BytesIter<'a> for $iterator<'a, 'b, FORMAT> {
unsafe impl<'a: 'b, 'b, const FORMAT: u128> BytesIter<'a> for $iterator<'a, 'b, FORMAT> {
skip_iterator_byteiter_base!(FORMAT, $mask);

/// Peek the next value of the iterator, without consuming it.
Expand Down Expand Up @@ -821,7 +821,7 @@ impl<'a: 'b, 'b, const FORMAT: u128> SpecialBytesIterator<'a, 'b, FORMAT> {
is_digit_separator!(FORMAT);
}

impl<'a: 'b, 'b, const FORMAT: u128> BytesIter<'a> for SpecialBytesIterator<'a, 'b, FORMAT> {
unsafe impl<'a: 'b, 'b, const FORMAT: u128> BytesIter<'a> for SpecialBytesIterator<'a, 'b, FORMAT> {
skip_iterator_byteiter_base!(FORMAT, SPECIAL_DIGIT_SEPARATOR);

/// Peek the next value of the iterator, without consuming it.
Expand Down

0 comments on commit f3d3cc1

Please sign in to comment.