ronantakizawa/codereview-bench huggingface dataset:
Repository: tokio-rs/tokio
File: tokio/src/io/interest.rs (Rust)

Code before:
#![cfg_attr(not(feature = "net"), allow(dead_code, unreachable_pub))]

use crate::io::ready::Ready;

use std::fmt;
use std::ops;

/// Readiness event interest.
///
/// Specifies the readiness events the caller is interested in when awaiting on
/// I/O resource readiness states.
#[cfg_attr(docsrs, doc(cfg(feature = "net")))]
#[derive(Clone, Copy, Eq, PartialEq)]
pub struct Interest {
    mio: Option<mio::Interest>,
    has_error_interest: bool,
}

impl Interest {
    const fn new(mio: mio::Interest) -> Self {
        Self {
            mio: Some(mio),
            has_error_interest: false,
        }
    }

    // The non-FreeBSD definitions in this block are active only when
    // building documentation.
    cfg_aio! {
        /// Interest for POSIX AIO.
        #[cfg(target_os = "freebsd")]
        pub const AIO: Interest = Interest::new(mio::Interest::AIO);

        /// Interest for POSIX AIO.
        #[cfg(not(target_os = "freebsd"))]
        pub const AIO: Interest = Interest::new(mio::Interest::READABLE);

        /// Interest for POSIX AIO lio_listio events.
        #[cfg(target_os = "freebsd")]

Review comment (suggestion):
Could you change this to a `usize` and define the bits yourself, similar to how `Ready` does it: https://github.com/tokio-rs/tokio/blob/master/tokio/src/io/ready.rs.

I would prefer not growing the struct size and avoid branches in the `is_*` checks.

Code after:
#![cfg_attr(not(feature = "net"), allow(dead_code, unreachable_pub))]

use crate::io::ready::Ready;

use std::fmt;
use std::ops;

// These must be unique.
// same as mio
const READABLE: usize = 0b0001;
const WRITABLE: usize = 0b0010;
// The following are not available on all platforms.
#[cfg(target_os = "freebsd")]
const AIO: usize = 0b0100;
#[cfg(target_os = "freebsd")]
const LIO: usize = 0b1000;
#[cfg(any(target_os = "linux", target_os = "android"))]
const PRIORITY: usize = 0b0001_0000;
// error is available on all platforms, but behavior is platform-specific
// mio does not have this interest
const ERROR: usize = 0b0010_0000;

/// Readiness event interest.
///
/// Specifies the readiness events the caller is interested in when awaiting on
/// I/O resource readiness states.
#[cfg_attr(docsrs, doc(cfg(feature = "net")))]
#[derive(Clone, Copy, Eq, PartialEq)]
pub struct Interest(usize);

impl Interest {
    // The non-FreeBSD definitions in this block are active only when
    // building documentation.
    cfg_aio! {
        /// Interest for POSIX AIO.
        #[cfg(target_os = "freebsd")]
        pub const AIO: Interest = Interest(AIO);

        /// Interest for POSIX AIO.
Repository: tokio-rs/tokio
File: tokio/src/runtime/id.rs (Rust)

Code before:
/// - Runtime IDs are unique relative to other *currently running* runtimes.
///   When a runtime completes, the same ID may be used for another runtime.
/// - Runtime IDs are *not* sequential, and do not indicate the order in which
///   runtimes are started or any other data.
/// - The runtime ID of the currently running task can be obtained from the
///   Handle.
///
/// # Examples
///
/// ```
/// use tokio::runtime::Handle;
///
/// #[tokio::main(flavor = "multi_thread", worker_threads = 4)]
/// async fn main() {
///   println!("Current runtime id: {}", Handle::current().id());
/// }
/// ```
///
/// **Note**: This is an [unstable API][unstable]. The public API of this type
/// may break in 1.x releases. See [the documentation on unstable
/// features][unstable] for details.
///
/// [unstable]: crate#unstable-features
#[cfg_attr(not(tokio_unstable), allow(unreachable_pub))]
#[derive(Clone, Copy, Debug, Hash, Eq, PartialEq)]
pub struct Id(u64);

impl Id {
    pub(crate) fn from_u64(val: u64) -> Self {
        Id(val)
    }
}

impl fmt::Display for Id {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        self.0.fmt(f)
    }
}


Review comment (suggestion):
You could make the id into a `NonZeroU64` since we make sure to not use zero as an id.

Code after:
///
/// - Runtime IDs are unique relative to other *currently running* runtimes.
///   When a runtime completes, the same ID may be used for another runtime.
/// - Runtime IDs are *not* sequential, and do not indicate the order in which
///   runtimes are started or any other data.
/// - The runtime ID of the currently running task can be obtained from the
///   Handle.
///
/// # Examples
///
/// ```
/// use tokio::runtime::Handle;
///
/// #[tokio::main(flavor = "multi_thread", worker_threads = 4)]
/// async fn main() {
///   println!("Current runtime id: {}", Handle::current().id());
/// }
/// ```
///
/// **Note**: This is an [unstable API][unstable]. The public API of this type
/// may break in 1.x releases. See [the documentation on unstable
/// features][unstable] for details.
///
/// [unstable]: crate#unstable-features
#[cfg_attr(not(tokio_unstable), allow(unreachable_pub))]
#[derive(Clone, Copy, Debug, Hash, Eq, PartialEq)]
pub struct Id(NonZeroU64);

impl From<NonZeroU64> for Id {
    fn from(value: NonZeroU64) -> Self {
        Id(value)
    }
}

impl fmt::Display for Id {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        self.0.fmt(f)
    }
}
Repository: tokio-rs/tokio
File: tokio/src/runtime/task/list.rs (Rust)

Code before:
// mixed up runtimes happen to have the same id.

cfg_has_atomic_u64! {
    use std::sync::atomic::{AtomicU64, Ordering};

    static NEXT_OWNED_TASKS_ID: AtomicU64 = AtomicU64::new(1);

    fn get_next_id() -> NonZeroU64 {
        loop {
            let id = NEXT_OWNED_TASKS_ID.fetch_add(1, Ordering::Relaxed);
            if let Some(id) = NonZeroU64::new(id) {
                return id;
            }
        }
    }
}

cfg_not_has_atomic_u64! {
    use std::sync::atomic::{AtomicU32, Ordering};

    static NEXT_OWNED_TASKS_ID: AtomicU32 = AtomicU32::new(1);

    fn get_next_id() -> NonZeroU64 {
        loop {
            let id = NEXT_OWNED_TASKS_ID.fetch_add(1, Ordering::Relaxed);
            if let Some(id) = NonZeroU64::new(id) {
                return id;
            }
        }
    }
}

pub(crate) struct OwnedTasks<S: 'static> {
    inner: Mutex<CountedOwnedTasksInner<S>>,
    id: NonZeroU64,
}
struct CountedOwnedTasksInner<S: 'static> {
    list: CountedLinkedList<Task<S>, <Task<S> as Link>::Target>,
    closed: bool,
}
pub(crate) struct LocalOwnedTasks<S: 'static> {
    inner: UnsafeCell<OwnedTasksInner<S>>,
    id: NonZeroU64,
    _not_send_or_sync: PhantomData<*const ()>,
}
struct OwnedTasksInner<S: 'static> {
    list: LinkedList<Task<S>, <Task<S> as Link>::Target>,
    closed: bool,
}

impl<S: 'static> OwnedTasks<S> {

Review comment (suggestion):
You still need the cast to u64.
```suggestion
            if let Some(id) = NonZeroU64::new(u64::from(id)) {
```

Code after:
// mixed up runtimes happen to have the same id.

cfg_has_atomic_u64! {
    use std::sync::atomic::{AtomicU64, Ordering};

    static NEXT_OWNED_TASKS_ID: AtomicU64 = AtomicU64::new(1);

    fn get_next_id() -> NonZeroU64 {
        loop {
            let id = NEXT_OWNED_TASKS_ID.fetch_add(1, Ordering::Relaxed);
            if let Some(id) = NonZeroU64::new(id) {
                return id;
            }
        }
    }
}

cfg_not_has_atomic_u64! {
    use std::sync::atomic::{AtomicU32, Ordering};

    static NEXT_OWNED_TASKS_ID: AtomicU32 = AtomicU32::new(1);

    fn get_next_id() -> NonZeroU64 {
        loop {
            let id = NEXT_OWNED_TASKS_ID.fetch_add(1, Ordering::Relaxed);
            if let Some(id) = NonZeroU64::new(u64::from(id)) {
                return id;
            }
        }
    }
}

pub(crate) struct OwnedTasks<S: 'static> {
    inner: Mutex<CountedOwnedTasksInner<S>>,
    id: NonZeroU64,
}
struct CountedOwnedTasksInner<S: 'static> {
    list: CountedLinkedList<Task<S>, <Task<S> as Link>::Target>,
    closed: bool,
}
pub(crate) struct LocalOwnedTasks<S: 'static> {
    inner: UnsafeCell<OwnedTasksInner<S>>,
    id: NonZeroU64,
    _not_send_or_sync: PhantomData<*const ()>,
}
struct OwnedTasksInner<S: 'static> {
    list: LinkedList<Task<S>, <Task<S> as Link>::Target>,
    closed: bool,
}

impl<S: 'static> OwnedTasks<S> {
Repository: tokio-rs/tokio
File: tokio/src/time/interval.rs (Rust)

Code before:
    ///
    ///     time::sleep(Duration::from_millis(50)).await;
    ///
    ///     let deadline = Instant::now() + Duration::from_millis(30);
    ///     interval.reset_at(deadline);
    ///
    ///     interval.tick().await;
    ///     interval.tick().await;
    ///
    ///     // approximately 180ms have elapsed.
    /// }
    /// ```
    pub fn reset_at(&mut self, deadline: Instant) {
        self.delay.as_mut().reset(deadline);
    }

    /// Returns the [`MissedTickBehavior`] strategy currently being used.
    pub fn missed_tick_behavior(&self) -> MissedTickBehavior {
        self.missed_tick_behavior
    }

    /// Sets the [`MissedTickBehavior`] strategy that should be used.
    pub fn set_missed_tick_behavior(&mut self, behavior: MissedTickBehavior) {
        self.missed_tick_behavior = behavior;
    }

    /// Returns the period of the interval.
    pub fn period(&self) -> Duration {
        self.period
    }
}


Review comment (suggestion):
I think it would be good to update all of the other methods (including `reset`) with text along the lines of "this is equivalent to calling `reset_at(...)`." 

Code after:
    /// # Examples
    ///
    /// ```
    /// use tokio::time::{self, Instant};
    ///
    /// use std::time::Duration;
    ///
    /// #[tokio::main]
    /// async fn main() {
    ///     let mut interval = time::interval(Duration::from_millis(100));
    ///     interval.tick().await;
    ///
    ///     time::sleep(Duration::from_millis(50)).await;
    ///
    ///     let deadline = Instant::now() + Duration::from_millis(30);
    ///     interval.reset_at(deadline);
    ///
    ///     interval.tick().await;
    ///     interval.tick().await;
    ///
    ///     // approximately 180ms have elapsed.
    /// }
    /// ```
    pub fn reset_at(&mut self, deadline: Instant) {
        self.delay.as_mut().reset(deadline);
    }

    /// Returns the [`MissedTickBehavior`] strategy currently being used.
    pub fn missed_tick_behavior(&self) -> MissedTickBehavior {
        self.missed_tick_behavior
    }

    /// Sets the [`MissedTickBehavior`] strategy that should be used.
    pub fn set_missed_tick_behavior(&mut self, behavior: MissedTickBehavior) {
        self.missed_tick_behavior = behavior;
    }

    /// Returns the period of the interval.
    pub fn period(&self) -> Duration {
        self.period
    }
}
Repository: tokio-rs/tokio
File: tokio/src/sync/mutex.rs (Rust)

Code before:
            s,
            #[cfg(all(tokio_unstable, feature = "tracing"))]
            resource_span,
        }
    }

    /// Creates a new lock in an unlocked state ready for use.
    ///
    /// # Examples
    ///
    /// ```
    /// use tokio::sync::Mutex;
    ///
    /// static LOCK: Mutex<i32> = Mutex::const_new(5);
    /// ```
    ///
    /// NOTE that this function is available if your rust compiler
    /// is newer than 1.63 and does not specify `--cfg tokio_no_const_mutex_new`
    /// or if you enabled feature `parking_lot`.
    #[cfg(all(
        any(feature = "parking_lot", not(tokio_no_const_mutex_new)),
        not(all(loom, test))
    ))]
    #[cfg_attr(
        docsrs,
        doc(cfg(any(feature = "parking_lot", not(tokio_no_const_mutex_new))))
    )]
    pub const fn const_new(t: T) -> Self
    where
        T: Sized,
    {
        Self {
            c: UnsafeCell::new(t),
            s: semaphore::Semaphore::const_new(1),
            #[cfg(all(tokio_unstable, feature = "tracing"))]
            resource_span: tracing::Span::none(),
        }
    }

    /// Locks this mutex, causing the current task to yield until the lock has
    /// been acquired.  When the lock has been acquired, function returns a
    /// [`MutexGuard`].
    ///
    /// # Cancel safety
    ///
    /// This method uses a queue to fairly distribute locks in the order they
    /// were requested. Cancelling a call to `lock` makes you lose your place in
    /// the queue.
    ///
    /// # Examples
    ///

Review comment (suggestion):
tokio_no_const_mutex_new is not a stable public API, and the user likely to actually use it are only our CI and old nightly users. So I do not want it to appear in the docs.

Code after:
            s,
            #[cfg(all(tokio_unstable, feature = "tracing"))]
            resource_span,
        }
    }

    /// Creates a new lock in an unlocked state ready for use.
    ///
    /// # Examples
    ///
    /// ```
    /// use tokio::sync::Mutex;
    ///
    /// static LOCK: Mutex<i32> = Mutex::const_new(5);
    /// ```
    #[cfg(not(all(loom, test)))]
    pub const fn const_new(t: T) -> Self
    where
        T: Sized,
    {
        Self {
            c: UnsafeCell::new(t),
            s: semaphore::Semaphore::const_new(1),
            #[cfg(all(tokio_unstable, feature = "tracing"))]
            resource_span: tracing::Span::none(),
        }
    }

    /// Locks this mutex, causing the current task to yield until the lock has
    /// been acquired.  When the lock has been acquired, function returns a
    /// [`MutexGuard`].
    ///
    /// # Cancel safety
    ///
    /// This method uses a queue to fairly distribute locks in the order they
    /// were requested. Cancelling a call to `lock` makes you lose your place in
    /// the queue.
    ///
    /// # Examples
    ///
    /// ```
    /// use tokio::sync::Mutex;
    ///
    /// #[tokio::main]
    /// async fn main() {
    ///     let mutex = Mutex::new(1);
    ///
    ///     let mut n = mutex.lock().await;
    ///     *n = 2;
    /// }
    /// ```
Repository: tokio-rs/tokio
File: tokio/src/process/mod.rs (Rust)

Code before:
    use super::ChildDropGuard;

    use futures::future::FutureExt;
    use std::future::Future;
    use std::io;
    use std::pin::Pin;
    use std::task::{Context, Poll};

    struct Mock {
        num_kills: usize,
        num_polls: usize,
        poll_result: Poll<Result<(), ()>>,
    }

    impl Mock {
        fn new() -> Self {
            Self::with_result(Poll::Pending)
        }

        fn with_result(result: Poll<Result<(), ()>>) -> Self {
            Self {
                num_kills: 0,
                num_polls: 0,
                poll_result: result,
            }
        }
    }

    impl Kill for Mock {
        fn kill(&mut self) -> io::Result<()> {
            self.num_kills += 1;
            Ok(())
        }
    }

    impl Future for Mock {
        type Output = Result<(), ()>;

        fn poll(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<Self::Output> {
            let inner = Pin::get_mut(self);
            inner.num_polls += 1;
            inner.poll_result
        }
    }

    #[test]
    fn kills_on_drop_if_specified() {
        let mut mock = Mock::new();

        {
            let guard = ChildDropGuard {

Review comment (suggestion):
These methods do not appear as "only available on windows" in the documentation. Also, it would be nice if `from_std` appears first in the documentation.

![image](https://github.com/tokio-rs/tokio/assets/928074/4788d3de-396b-4491-a939-926783717594)


Code after:
        ($type:ty) => {
            impl $type {
                /// Convert into [`OwnedHandle`].
                pub fn into_owned_handle(self) -> io::Result<OwnedHandle> {
                    todo!("For doc generation only")
                }
            }

            impl AsRawHandle for $type {
                fn as_raw_handle(&self) -> RawHandle {
                    todo!("For doc generation only")
                }
            }

            impl AsHandle for $type {
                fn as_handle(&self) -> BorrowedHandle<'_> {
                    todo!("For doc generation only")
                }
            }
        };
    }

    impl_traits!(ChildStdin);
    impl_traits!(ChildStdout);
    impl_traits!(ChildStderr);
}

#[cfg(all(test, not(loom)))]
mod test {
    use super::kill::Kill;
    use super::ChildDropGuard;

    use futures::future::FutureExt;
    use std::future::Future;
    use std::io;
    use std::pin::Pin;
    use std::task::{Context, Poll};

    struct Mock {
        num_kills: usize,
        num_polls: usize,
        poll_result: Poll<Result<(), ()>>,
    }

    impl Mock {
        fn new() -> Self {
            Self::with_result(Poll::Pending)
        }

        fn with_result(result: Poll<Result<(), ()>>) -> Self {
            Self {
Repository: tokio-rs/tokio
File: tokio/src/process/mod.rs (Rust)

Code before:
                    self.inner.into_owned_fd()
                }
            }

            impl AsRawFd for $type {
                fn as_raw_fd(&self) -> RawFd {
                    self.inner.as_raw_fd()
                }
            }

            impl AsFd for $type {
                fn as_fd(&self) -> BorrowedFd<'_> {
                    unsafe { BorrowedFd::borrow_raw(self.as_raw_fd()) }
                }
            }
        };
    }

    impl_traits!(ChildStdin);
    impl_traits!(ChildStdout);
    impl_traits!(ChildStderr);
}

#[cfg(windows)]
#[cfg_attr(docsrs, doc(cfg(windows)))]
mod windows {
    use super::*;
    use crate::os::windows::io::{AsHandle, BorrowedHandle, OwnedHandle};

    macro_rules! impl_traits {
        ($type:ty) => {
            impl $type {
                /// Convert into [`OwnedHandle`].
                pub fn into_owned_handle(self) -> io::Result<OwnedHandle> {
                    self.inner.into_owned_handle()
                }
            }

            impl AsRawHandle for $type {
                fn as_raw_handle(&self) -> RawHandle {
                    self.inner.as_raw_handle()
                }
            }

            impl AsHandle for $type {
                fn as_handle(&self) -> BorrowedHandle<'_> {
                    unsafe { BorrowedHandle::borrow_raw(self.as_raw_handle()) }
                }
            }
        };
    }

Review comment (suggestion):
We need something along these lines for the documentation to be rendered correctly.
```suggestion
#[cfg(any(windows, docsrs))]
#[cfg_attr(docsrs, doc(cfg(windows)))]
mod windows {
```

Code after:
                    self.inner.into_owned_fd()
                }
            }

            impl AsRawFd for $type {
                fn as_raw_fd(&self) -> RawFd {
                    self.inner.as_raw_fd()
                }
            }

            impl AsFd for $type {
                fn as_fd(&self) -> BorrowedFd<'_> {
                    unsafe { BorrowedFd::borrow_raw(self.as_raw_fd()) }
                }
            }
        };
    }

    impl_traits!(ChildStdin);
    impl_traits!(ChildStdout);
    impl_traits!(ChildStderr);
}

#[cfg(any(windows, docsrs))]
#[cfg_attr(docsrs, doc(cfg(windows)))]
mod windows {
    use super::*;
    use crate::os::windows::io::{AsHandle, AsRawHandle, BorrowedHandle, OwnedHandle, RawHandle};

    #[cfg(not(docsrs))]
    macro_rules! impl_traits {
        ($type:ty) => {
            impl $type {
                /// Convert into [`OwnedHandle`].
                pub fn into_owned_handle(self) -> io::Result<OwnedHandle> {
                    self.inner.into_owned_handle()
                }
            }

            impl AsRawHandle for $type {
                fn as_raw_handle(&self) -> RawHandle {
                    self.inner.as_raw_handle()
                }
            }

            impl AsHandle for $type {
                fn as_handle(&self) -> BorrowedHandle<'_> {
                    unsafe { BorrowedHandle::borrow_raw(self.as_raw_handle()) }
                }
            }
        };
Repository: tokio-rs/tokio
File: tokio/src/sync/once_cell.rs (Rust)

Code before:

    /// Creates a new `OnceCell` that contains the provided value.
    ///
    /// This is equivalent to `<OnceCell<T> as From<T>>::from(value)`.
    ///
    /// # Example
    ///
    /// ```
    /// use tokio::sync::OnceCell;
    ///
    /// static ONCE: OnceCell<u32> = OnceCell::from_value(1);
    ///
    /// async fn get_global_integer() -> &'static u32 {
    ///     ONCE.get_or_init(|| async {
    ///         1 + 1
    ///     }).await
    /// }
    ///
    /// #[tokio::main]
    /// async fn main() {
    ///     let result = get_global_integer().await;
    ///     assert_eq!(*result, 1);
    /// }
    /// ```
    #[cfg(not(all(loom, test)))]
    pub const fn from_value(value: T) -> Self {
        OnceCell {
            value_set: AtomicBool::new(true),
            value: UnsafeCell::new(MaybeUninit::new(value)),
            semaphore: Semaphore::const_new_closed(),
        }
    }

    /// Creates a new empty `OnceCell` instance.
    ///
    /// Equivalent to `OnceCell::new`, except that it can be used in static
    /// variables.
    ///
    /// # Example
    ///
    /// ```
    /// use tokio::sync::OnceCell;
    ///
    /// static ONCE: OnceCell<u32> = OnceCell::const_new();
    ///
    /// async fn get_global_integer() -> &'static u32 {
    ///     ONCE.get_or_init(|| async {
    ///         1 + 1
    ///     }).await
    /// }
    ///

Review comment (suggestion):
I think it makes more sense to have a `const_new_with` constructor instead.

Code after:

    /// Creates a new `OnceCell` that contains the provided value.
    ///
    /// # Example
    ///
    /// ```
    /// use tokio::sync::OnceCell;
    ///
    /// static ONCE: OnceCell<u32> = OnceCell::const_new_with(1);
    ///
    /// async fn get_global_integer() -> &'static u32 {
    ///     ONCE.get_or_init(|| async {
    ///         1 + 1
    ///     }).await
    /// }
    ///
    /// #[tokio::main]
    /// async fn main() {
    ///     let result = get_global_integer().await;
    ///     assert_eq!(*result, 1);
    /// }
    /// ```
    #[cfg(not(all(loom, test)))]
    pub const fn const_new_with(value: T) -> Self {
        OnceCell {
            value_set: AtomicBool::new(true),
            value: UnsafeCell::new(MaybeUninit::new(value)),
            semaphore: Semaphore::const_new_closed(),
        }
    }

    /// Creates a new empty `OnceCell` instance.
    ///
    /// Equivalent to `OnceCell::new`, except that it can be used in static
    /// variables.
    ///
    /// # Example
    ///
    /// ```
    /// use tokio::sync::OnceCell;
    ///
    /// static ONCE: OnceCell<u32> = OnceCell::const_new();
    ///
    /// async fn get_global_integer() -> &'static u32 {
    ///     ONCE.get_or_init(|| async {
    ///         1 + 1
    ///     }).await
    /// }
    ///
    /// #[tokio::main]
    /// async fn main() {