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

Code before:
/// flag.
///
/// # Current thread runtime
///
/// The default test runtime is single-threaded. Each test gets a
/// separate current-thread runtime.
///
/// ```no_run
/// #[tokio::test]
/// async fn my_test() {
///     assert!(true);
/// }
/// ```
///
/// ## Usage
///
/// ### Using the multi-thread runtime
///
/// ```no_run
/// #[tokio::test(flavor = "multi_thread")]
/// async fn my_test() {
///     assert!(true);
/// }
/// ```
///
///Equivalent code not using `#[tokio::test]`
///
/// ```no_run
/// #[test]
/// fn my_test() {
///     tokio::runtime::Builder::new_multi_thread()
///         .enable_all()
///         .build()
///         .unwrap()
///         .block_on(async {
///             assert!(true);
///         })
/// }
/// ```
///
/// ### Using current thread runtime
///
/// ```no_run
/// #[tokio::test]
/// async fn my_test() {
///     assert!(true);
/// }
/// ```
///
///Equivalent code not using `#[tokio::test]`
///

Review comment (suggestion):
```suggestion
/// Equivalent code not using `#[tokio::test]`
```

Code after:
/// Note: The multi-threaded runtime requires the `rt-multi-thread` feature
/// flag.
///
/// # Current thread runtime
///
/// The default test runtime is single-threaded. Each test gets a
/// separate current-thread runtime.
///
/// ```no_run
/// #[tokio::test]
/// async fn my_test() {
///     assert!(true);
/// }
/// ```
///
/// ## Usage
///
/// ### Using the multi-thread runtime
///
/// ```no_run
/// #[tokio::test(flavor = "multi_thread")]
/// async fn my_test() {
///     assert!(true);
/// }
/// ```
///
/// Equivalent code not using `#[tokio::test]`
///
/// ```no_run
/// #[test]
/// fn my_test() {
///     tokio::runtime::Builder::new_multi_thread()
///         .enable_all()
///         .build()
///         .unwrap()
///         .block_on(async {
///             assert!(true);
///         })
/// }
/// ```
///
/// ### Using current thread runtime
///
/// ```no_run
/// #[tokio::test]
/// async fn my_test() {
///     assert!(true);
/// }
/// ```
///
/// Equivalent code not using `#[tokio::test]`
Repository: tokio-rs/tokio
File: tokio/src/task/local.rs (Rust)

Code before:
    pub(super) fn spawn_local_inner<F>(future: F, name: Option<&str>) -> JoinHandle<F::Output>
    where F: Future + 'static,
          F::Output: 'static
    {
        CURRENT.with(|maybe_cx| {
            let cx = maybe_cx
                .expect("`spawn_local` called from outside of a `task::LocalSet`");

            cx.spawn(future, name)
        })
    }
}

/// Initial queue capacity.
const INITIAL_CAPACITY: usize = 64;

/// Max number of tasks to poll per tick.
const MAX_TASKS_PER_TICK: usize = 61;

/// How often it check the remote queue first.
const REMOTE_FIRST_INTERVAL: u8 = 31;

#[derive(Debug)]
pub struct EnterGuard<'a> {
    _guard: &'a LocalSet,
}

impl LocalSet {
    /// Returns a new local task set.
    pub fn new() -> LocalSet {
        LocalSet {
            tick: Cell::new(0),
            context: Context {
                owned: LocalOwnedTasks::new(),
                queue: VecDequeCell::with_capacity(INITIAL_CAPACITY),
                shared: Arc::new(Shared {
                    queue: Mutex::new(Some(VecDeque::with_capacity(INITIAL_CAPACITY))),
                    waker: AtomicWaker::new(),
                }),
            },
            _not_send: PhantomData,
        }
    }

    /// Enter current LocalSet context
    pub fn enter(&self) -> EnterGuard<'_> {
        CURRENT.inner.with(|c| {
            c.set(&self.context as *const _ as *const ());
            EnterGuard { _guard: &self }
        })
    }

Review comment (suggestion):
This defines a public type, but the `local` module is not public. You should reexport it somewhere else. We should also consider if `LocalEnterGuard` is a better name.

Code after:
    where
        F: Future + 'static,
        F::Output: 'static,
    {
        spawn_local_inner(future, None)
    }


    #[track_caller]
    pub(super) fn spawn_local_inner<F>(future: F, name: Option<&str>) -> JoinHandle<F::Output>
    where F: Future + 'static,
          F::Output: 'static
    {
        CURRENT.with(|maybe_cx| {
            let ctx = clone_rc(maybe_cx);
            match ctx {
                None => panic!("`spawn_local` called from outside of a `task::LocalSet`"),
                Some(cx) => cx.spawn(future, name)
            }

        })
    }
}

/// Initial queue capacity.
const INITIAL_CAPACITY: usize = 64;

/// Max number of tasks to poll per tick.
const MAX_TASKS_PER_TICK: usize = 61;

/// How often it check the remote queue first.
const REMOTE_FIRST_INTERVAL: u8 = 31;

/// Context guard for LocalSet
pub struct LocalEnterGuard(Option<Rc<Context>>);

impl Drop for LocalEnterGuard {
    fn drop(&mut self) {
        CURRENT.with(|ctx| {
            ctx.replace(self.0.take());
        })
    }
}

impl fmt::Debug for LocalEnterGuard {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        f.debug_struct("LocalEnterGuard").finish()
    }
}

impl LocalSet {
Repository: tokio-rs/tokio
File: tokio/tests/rt_panic.rs (Rust)

Code before:
        } else {
            None
        }
    }
}

#[test]
fn test_current_handle_panic_caller() -> Result<(), Box<dyn Error>> {
    let panic_location_file = test_panic(|| {
        let _ = Handle::current();
    });

    // The panic location should be in this file
    assert_eq!(&panic_location_file.unwrap(), file!());

    Ok(())
}

#[test]
fn test_into_panic_panic_caller() -> Result<(), Box<dyn Error>> {
    let panic_location_file = test_panic(move || {
        let rt = basic();
        rt.block_on(async {
            let handle = tokio::spawn(async {
                tokio::time::sleep(std::time::Duration::from_millis(100)).await;
            });

            handle.abort();

            let err = handle.await.unwrap_err();
            assert!(!&err.is_panic());

            let _ = err.into_panic();
        });
    });

    // The panic location should be in this file
    assert_eq!(&panic_location_file.unwrap(), file!());

    Ok(())
}

#[test]
fn test_builder_worker_threads_panic_caller() -> Result<(), Box<dyn Error>> {
    let panic_location_file = test_panic(|| {
        let _ = Builder::new_multi_thread().worker_threads(0).build();
    });

    // The panic location should be in this file
    assert_eq!(&panic_location_file.unwrap(), file!());


Review comment (bug):
Use `futures::future::pending()` instead of a timer where the test fails if the timer elapses.

Code after:
        if result.is_err() {
            panic_file.lock().clone()
        } else {
            None
        }
    }
}

#[test]
fn current_handle_panic_caller() -> Result<(), Box<dyn Error>> {
    let panic_location_file = test_panic(|| {
        let _ = Handle::current();
    });

    // The panic location should be in this file
    assert_eq!(&panic_location_file.unwrap(), file!());

    Ok(())
}

#[test]
fn into_panic_panic_caller() -> Result<(), Box<dyn Error>> {
    let panic_location_file = test_panic(move || {
        let rt = basic();
        rt.block_on(async {
            let handle = tokio::spawn(future::pending::<()>());

            handle.abort();

            let err = handle.await.unwrap_err();
            assert!(!&err.is_panic());

            let _ = err.into_panic();
        });
    });

    // The panic location should be in this file
    assert_eq!(&panic_location_file.unwrap(), file!());

    Ok(())
}

#[test]
fn builder_worker_threads_panic_caller() -> Result<(), Box<dyn Error>> {
    let panic_location_file = test_panic(|| {
        let _ = Builder::new_multi_thread().worker_threads(0).build();
    });

    // The panic location should be in this file
    assert_eq!(&panic_location_file.unwrap(), file!());
Repository: tokio-rs/tokio
File: tokio/tests/rt_panic.rs (Rust)

Code before:
#![warn(rust_2018_idioms)]
#![cfg(feature = "full")]

use lazy_static::lazy_static;
use std::error::Error;
use std::panic;
use std::sync::{Arc, Mutex, Once};
use tokio::runtime::{Builder, Handle, Runtime};

fn test_panic<Func: FnOnce() + panic::UnwindSafe>(func: Func) -> Option<String> {
    lazy_static! {
        static ref SET_UP_PANIC_HOOK: Once = Once::new();
        static ref PANIC_MUTEX: Arc<Mutex<()>> = Arc::new(Mutex::new(()));
        static ref PANIC_FILE: Mutex<String> = Mutex::new(String::new());
    }

    SET_UP_PANIC_HOOK.call_once(|| {
        panic::set_hook(Box::new(|panic_info| {
            let panic_location = panic_info.location().unwrap();
            PANIC_FILE
                .lock()
                .unwrap()
                .clone_from(&panic_location.file().to_string());
        }));
    });

    {
        let _guard = PANIC_MUTEX.lock();

        let result = panic::catch_unwind(func);

        if result.is_err() {
            Some(PANIC_FILE.lock().ok()?.clone())
        } else {
            None
        }
    }
}

#[test]

Review comment (suggestion):
We already have the dependencies to do this. There's no reason to add lazy_static.
```suggestion
use parking_lot::{const_mutex, Mutex};

static SET_UP_PANIC_HOOK: Once = Once::new();
static PANIC_MUTEX: Mutex<()> = const_mutex(());
static PANIC_FILE: Mutex<String> = const_mutex(String::new());
```

Code after:
#![warn(rust_2018_idioms)]
#![cfg(feature = "full")]

use futures::future;
use parking_lot::{const_mutex, Mutex};
use std::error::Error;
use std::panic;
use std::sync::Arc;
use tokio::runtime::{Builder, Handle, Runtime};

fn test_panic<Func: FnOnce() + panic::UnwindSafe>(func: Func) -> Option<String> {
    static PANIC_MUTEX: Mutex<()> = const_mutex(());

    {
        let _guard = PANIC_MUTEX.lock();
        let panic_file: Arc<Mutex<Option<String>>> = Arc::new(Mutex::new(None));

        let prev_hook = panic::take_hook();
        {
            let panic_file = panic_file.clone();
            panic::set_hook(Box::new(move |panic_info| {
                let panic_location = panic_info.location().unwrap();
                panic_file
                    .lock()
                    .clone_from(&Some(panic_location.file().to_string()));
            }));
        }

        let result = panic::catch_unwind(func);
        // Return to the previously set panic hook (maybe default) so that we get nice error
        // messages in the tests.
        panic::set_hook(prev_hook);

        if result.is_err() {
            panic_file.lock().clone()
        } else {
            None
        }
    }
}
Repository: tokio-rs/tokio
File: tokio/tests/rt_panic.rs (Rust)

Code before:
        if result.is_err() {
            panic_file.lock().clone()
        } else {
            None
        }
    }
}

#[test]
fn test_current_handle_panic_caller() -> Result<(), Box<dyn Error>> {
    let panic_location_file = test_panic(|| {
        let _ = Handle::current();
    });

    // The panic location should be in this file
    assert_eq!(&panic_location_file.unwrap(), file!());

    Ok(())
}

#[test]
fn test_into_panic_panic_caller() -> Result<(), Box<dyn Error>> {
    let panic_location_file = test_panic(move || {
        let rt = basic();
        rt.block_on(async {
            let handle = tokio::spawn(async { future::pending::<()>().await });

            handle.abort();

            let err = handle.await.unwrap_err();
            assert!(!&err.is_panic());

            let _ = err.into_panic();
        });
    });

    // The panic location should be in this file
    assert_eq!(&panic_location_file.unwrap(), file!());

    Ok(())
}

#[test]
fn test_builder_worker_threads_panic_caller() -> Result<(), Box<dyn Error>> {
    let panic_location_file = test_panic(|| {
        let _ = Builder::new_multi_thread().worker_threads(0).build();
    });

    // The panic location should be in this file
    assert_eq!(&panic_location_file.unwrap(), file!());


Review comment (suggestion):
Maybe this is simpler?
```suggestion
            let handle = tokio::spawn(future::pending::<()>());
```

Code after:
        if result.is_err() {
            panic_file.lock().clone()
        } else {
            None
        }
    }
}

#[test]
fn current_handle_panic_caller() -> Result<(), Box<dyn Error>> {
    let panic_location_file = test_panic(|| {
        let _ = Handle::current();
    });

    // The panic location should be in this file
    assert_eq!(&panic_location_file.unwrap(), file!());

    Ok(())
}

#[test]
fn into_panic_panic_caller() -> Result<(), Box<dyn Error>> {
    let panic_location_file = test_panic(move || {
        let rt = basic();
        rt.block_on(async {
            let handle = tokio::spawn(future::pending::<()>());

            handle.abort();

            let err = handle.await.unwrap_err();
            assert!(!&err.is_panic());

            let _ = err.into_panic();
        });
    });

    // The panic location should be in this file
    assert_eq!(&panic_location_file.unwrap(), file!());

    Ok(())
}

#[test]
fn builder_worker_threads_panic_caller() -> Result<(), Box<dyn Error>> {
    let panic_location_file = test_panic(|| {
        let _ = Builder::new_multi_thread().worker_threads(0).build();
    });

    // The panic location should be in this file
    assert_eq!(&panic_location_file.unwrap(), file!());
Repository: tokio-rs/tokio
File: tokio/tests/net_panic.rs (Rust)

Code before:
#![warn(rust_2018_idioms)]
#![cfg(feature = "full")]

use std::error::Error;
use tokio::net::{TcpListener, TcpStream};
use tokio::runtime::{Builder, Runtime};

mod support {
    pub mod panic;
}
use support::panic::test_panic;

#[test]
fn udp_socket_from_std_panic_caller() -> Result<(), Box<dyn Error>> {
    use std::net::SocketAddr;
    use tokio::net::UdpSocket;

    let addr = "127.0.0.1:8080".parse::<SocketAddr>().unwrap();
    let std_sock = std::net::UdpSocket::bind(addr).unwrap();
    std_sock.set_nonblocking(true).unwrap();

    let panic_location_file = test_panic(|| {
        let rt = runtime_without_io();
        rt.block_on(async {
            let _sock = UdpSocket::from_std(std_sock);
        });
    });

    // The panic location should be in this file
    assert_eq!(&panic_location_file.unwrap(), file!());

    Ok(())
}

#[test]
fn tcp_listener_from_std_panic_caller() -> Result<(), Box<dyn Error>> {
    let std_listener = std::net::TcpListener::bind("127.0.0.1:8080").unwrap();
    std_listener.set_nonblocking(true).unwrap();


Review comment (suggestion):
```suggestion
#[test]
#[cfg(not(target_os = "wasi"))]
fn udp_socket_from_std_panic_caller() -> Result<(), Box<dyn Error>> {
```

Code after:
#![warn(rust_2018_idioms)]
#![cfg(all(feature = "full", not(target_os = "wasi")))]

use std::error::Error;
use tokio::net::{TcpListener, TcpStream};
use tokio::runtime::{Builder, Runtime};

mod support {
    pub mod panic;
}
use support::panic::test_panic;

#[test]
fn udp_socket_from_std_panic_caller() -> Result<(), Box<dyn Error>> {
    use std::net::SocketAddr;
    use tokio::net::UdpSocket;

    let addr = "127.0.0.1:8080".parse::<SocketAddr>().unwrap();
    let std_sock = std::net::UdpSocket::bind(addr).unwrap();
    std_sock.set_nonblocking(true).unwrap();

    let panic_location_file = test_panic(|| {
        let rt = runtime_without_io();
        rt.block_on(async {
            let _sock = UdpSocket::from_std(std_sock);
        });
    });

    // The panic location should be in this file
    assert_eq!(&panic_location_file.unwrap(), file!());

    Ok(())
}

#[test]
fn tcp_listener_from_std_panic_caller() -> Result<(), Box<dyn Error>> {
    let std_listener = std::net::TcpListener::bind("127.0.0.1:8080").unwrap();
    std_listener.set_nonblocking(true).unwrap();
Repository: tokio-rs/tokio
File: tokio/src/lib.rs (Rust)

Code before:

// Includes re-exports used by macros.
//
// This module is not intended to be part of the public API. In general, any
// `doc(hidden)` code is not part of Tokio's public and stable API.
#[macro_use]
#[doc(hidden)]
pub mod macros;

#[cfg(feature = "fs")]
#[cfg_attr(docsrs, doc(cfg(feature = "fs")))]
pub mod fs;

cfg_fs_internal! {
    #[cfg(not(feature = "fs"))]
    #[allow(dead_code)]
    #[allow(unreachable_pub)]
    pub(crate) mod fs;
}

mod future;

pub mod io;
pub mod net;

mod loom;
mod park;

cfg_process! {
    pub mod process;
}

#[cfg(any(
    feature = "fs",
    feature = "io-std",
    feature = "net",
    all(windows, feature = "process"),
))]
mod blocking;

cfg_rt! {
    pub mod runtime;
}

pub(crate) mod coop;

cfg_signal! {
    pub mod signal;
}

cfg_signal_internal! {

Review comment (suggestion):
I don't think this is necessary considering that `tokio::io::{stdin,stdout}` have already been implemented by this strategy without using `tokio::fs`.

Code after:

#[cfg(all(
    not(tokio_unstable),
    target_arch = "wasm32",
    any(
        feature = "fs",
        feature = "io-std",
        feature = "net",
        feature = "process",
        feature = "rt-multi-thread",
        feature = "signal"
    )
))]
compile_error!("Only features sync,macros,io-util,rt are supported on wasm.");

// Includes re-exports used by macros.
//
// This module is not intended to be part of the public API. In general, any
// `doc(hidden)` code is not part of Tokio's public and stable API.
#[macro_use]
#[doc(hidden)]
pub mod macros;

cfg_fs! {
    pub mod fs;
}

mod future;

pub mod io;
pub mod net;

mod loom;
mod park;

cfg_process! {
    pub mod process;
}

#[cfg(any(
    feature = "fs",
    feature = "io-std",
    feature = "net",
    all(windows, feature = "process"),
))]
mod blocking;

cfg_rt! {
    pub mod runtime;
}
Repository: tokio-rs/tokio
File: tokio/src/process/mod.rs (Rust)

Code before:

        let stdout_fut = read_to_end(&mut stdout_pipe);
        let stderr_fut = read_to_end(&mut stderr_pipe);

        let (status, stdout, stderr) = try_join3(self.wait(), stdout_fut, stderr_fut).await?;

        // Drop happens after `try_join` due to <https://github.com/tokio-rs/tokio/issues/4309>
        drop(stdout_pipe);
        drop(stderr_pipe);

        Ok(Output {
            status,
            stdout,
            stderr,
        })
    }
}

pin_project! {
    /// The standard input stream for spawned children.
    ///
    /// This type implements the `AsyncWrite` trait to pass data to the stdin handle of
    /// handle of a child process asynchronously.
    #[derive(Debug)]
    pub struct ChildStdin {
        #[pin]
        inner: imp::ChildStdio,
    }
}

pin_project! {
    /// The standard output stream for spawned children.
    ///
    /// This type implements the `AsyncRead` trait to read data from the stdout
    /// handle of a child process asynchronously.
    #[derive(Debug)]
    pub struct ChildStdout {
        #[pin]
        inner: imp::ChildStdio,
    }
}

pin_project! {
    /// The standard error stream for spawned children.
    ///
    /// This type implements the `AsyncRead` trait to read data from the stderr
    /// handle of a child process asynchronously.
    #[derive(Debug)]
    pub struct ChildStderr {
        #[pin]
        inner: imp::ChildStdio,

Review comment (suggestion):
There are still a bunch of unnecessary uses of `pin_project!`.

Code after:
        let stdout_fut = read_to_end(&mut stdout_pipe);
        let stderr_fut = read_to_end(&mut stderr_pipe);

        let (status, stdout, stderr) = try_join3(self.wait(), stdout_fut, stderr_fut).await?;

        // Drop happens after `try_join` due to <https://github.com/tokio-rs/tokio/issues/4309>
        drop(stdout_pipe);
        drop(stderr_pipe);

        Ok(Output {
            status,
            stdout,
            stderr,
        })
    }
}

/// The standard input stream for spawned children.
///
/// This type implements the `AsyncWrite` trait to pass data to the stdin handle of
/// handle of a child process asynchronously.
#[derive(Debug)]
pub struct ChildStdin {
    inner: imp::ChildStdio,
}

/// The standard output stream for spawned children.
///
/// This type implements the `AsyncRead` trait to read data from the stdout
/// handle of a child process asynchronously.
#[derive(Debug)]
pub struct ChildStdout {
    inner: imp::ChildStdio,
}

/// The standard error stream for spawned children.
///
/// This type implements the `AsyncRead` trait to read data from the stderr
/// handle of a child process asynchronously.
#[derive(Debug)]
pub struct ChildStderr {
    inner: imp::ChildStdio,
}

impl ChildStdin {
    /// Creates an asynchronous `ChildStdin` from a synchronous one.
    ///
    /// # Errors
    ///
    /// This method may fail if an error is encountered when setting the pipe to
    /// non-blocking mode, or when registering the pipe with the runtime's IO
Repository: tokio-rs/tokio
File: tokio-stream/src/wrappers.rs (Rust)

Code before:

cfg_net! {
    mod tcp_listener;
    pub use tcp_listener::TcpListenerStream;

    #[cfg(unix)]
    mod unix_listener;
    #[cfg(unix)]
    pub use unix_listener::UnixListenerStream;
}

cfg_io_util! {
    mod split;
    pub use split::SplitStream;

    mod lines;
    pub use lines::LinesStream;
}

cfg_fs! {
    mod read_dir;
    pub use read_dir::ReadDirStream;
}

mod stream_close;
pub use stream_close::StreamNotifyClose;


Review comment (suggestion):
The `wrappers` module is documented as:

> Wrappers for Tokio types that implement `Stream`.

Despite being a wrapper, this type doesn't fall into that category. I would like to put it outside the module instead.

Code after:
//! Wrappers for Tokio types that implement `Stream`.

/// Error types for the wrappers.
pub mod errors {
    cfg_sync! {
        pub use crate::wrappers::broadcast::BroadcastStreamRecvError;
    }
}

mod mpsc_bounded;
pub use mpsc_bounded::ReceiverStream;

mod mpsc_unbounded;
pub use mpsc_unbounded::UnboundedReceiverStream;

cfg_sync! {
    mod broadcast;
    pub use broadcast::BroadcastStream;

    mod watch;
    pub use watch::WatchStream;
}

cfg_signal! {
    #[cfg(unix)]
    mod signal_unix;
    #[cfg(unix)]
    pub use signal_unix::SignalStream;

    #[cfg(any(windows, docsrs))]
    mod signal_windows;
    #[cfg(any(windows, docsrs))]
    pub use signal_windows::{CtrlCStream, CtrlBreakStream};
}

cfg_time! {
    mod interval;
    pub use interval::IntervalStream;
}

cfg_net! {
    mod tcp_listener;
    pub use tcp_listener::TcpListenerStream;

    #[cfg(unix)]
    mod unix_listener;
    #[cfg(unix)]
    pub use unix_listener::UnixListenerStream;
}

cfg_io_util! {
    mod split;
    pub use split::SplitStream;

    mod lines;
    pub use lines::LinesStream;
}

cfg_fs! {
    mod read_dir;
    pub use read_dir::ReadDirStream;
}