Skip to content

impl IoSafe for std::io::PipeReader & std::io::PipeWriter #237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 5, 2025

Conversation

jk-ozlabs
Copy link
Contributor

It would be handy to have async wrappers for the pipe types (from std::io::pipe()), so impl IoSafe for both.

The pipe types were introduced in 1.87, but the MSRV for async-io appears to be 1.63 currently. So, protect the impls with a cfg(has_pipe), and use autocfg to set has_pipe, like was done in a20076f ("Implement I/O-safe traits (#84)")

@jk-ozlabs
Copy link
Contributor Author

Looks like emit_possibility was only in autocfg 1.5, so I have split into a simpler emit_has_type() check, that has earlier compat with the minimum versions test.

This means we have two cfg()s, but they directly align with the type availability.

Copy link
Collaborator

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jk-ozlabs
Copy link
Contributor Author

Thanks!

and thanks for the review! 😄

build.rs Outdated
Comment on lines 10 to 12
// emit expected config settings; may be replaced with
// autocfg::emit_possibility if we update to autocfg>=1.5
println!("cargo:rustc-check-cfg=cfg(async_io_no_pipe)");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be moved to Cargo.toml, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I am happy to implement it there if preferred.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, now it's in Cargo.toml's [lints.rust].

It would be handy to have async wrappers for the pipe types (from
std::io::pipe()), so impl IoSafe for both.

The pipe types were introduced in 1.87, but the MSRV for async-io
appears to be 1.63 currently. So, protect the impls with cfg()s
and use autocfg to detect the type availbility, similar to what was done
in a20076f ("Implement I/O-safe traits (smol-rs#84)")

Signed-off-by: Jeremy Kerr <[email protected]>
Copy link
Member

@notgull notgull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@notgull notgull requested a review from taiki-e August 5, 2025 06:04
@taiki-e taiki-e merged commit be049a8 into smol-rs:master Aug 5, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants