Skip to content
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

[DISCUSSION] Value over core::future #14

Open
boomshroom opened this issue Oct 10, 2018 · 8 comments
Open

[DISCUSSION] Value over core::future #14

boomshroom opened this issue Oct 10, 2018 · 8 comments

Comments

@boomshroom
Copy link

With futures becoming standardized and integrated into the core library, I feel like this crate has less reason to exist. The biggest part of this crate is type nb::Result<T, E> = Result<T, nb::Error<E>>, but in the current nightly, this can be replicated with core::task::Poll<Result<T, E>>. The biggest reason I can see to use this library is the fact that it doesn't require nightly. It may be a while before core::{future, task} becomes stabalized, but even so, if I'm doing no_std stuff, I tend to be using nightly anyways (for things like asm).

That being said, where do you see this library going in the near or distant future given futures being adopted into the standard library? I'm not saying to shut this down and switch everyone to futures, I just want to get an idea of where things are headed. Thank you for any discussion. (I also apologize if the issue tracker isn't the best place for discussion.)

@hannobraun
Copy link
Member

@boomshroom

It may be a while before core::{future, task} becomes stabalized, but even so, if I'm doing no_std stuff, I tend to be using nightly anyways (for things like asm).

There has been a push to get embedded development on stable for the upcoming Rust 2018 release. I don't think we should deprecate nb before we have a stable replacement.

That being said, where do you see this library going in the near or distant future given futures being adopted into the standard library? I'm not saying to shut this down and switch everyone to futures, I just want to get an idea of where things are headed. Thank you for any discussion. (I also apologize if the issue tracker isn't the best place for discussion.)

I've had this thought for a while now, that nb could be replaced one day, and we'd standardize on Future and async/await instead. I haven't followed the latest developments though, and I'm not up to date on current plans. So I don't know if/when this might become viable.

I think it would be very valuable, if you or anyone else wanted to look into this, and report back what Rust's async future means for nb and embedded-hal.

@boomshroom
Copy link
Author

Thank you. Your input exactly what I asked for. So it seems as though there's a chance of deprecation once futures hit stable, but it's still not set in stone.

@boomshroom
Copy link
Author

Std futures have now been stabilized in nightly and will be arriving in a few months in 1.36. The merge can be found here: rust-lang/rust#59739

@XOSplicer
Copy link

Rust 1.36 has been releases with a stabilized Future trait.
Can this fact lead this conversation any further?

@hannobraun
Copy link
Member

@XOSplicer I'm not speaking for the project, only for myself, but I think what's required here is someone to figure out what using Future actually means in practice. If I had the time to do this, I'd fork embedded-hal, create Future-based versions of the traits and figure out what's required to make that work in a real program.

Maybe there are others that know more than me, but until one of them shows up here and tells us what's up, I assume someone will have to do the actual work :-)

@XOSplicer
Copy link

XOSplicer commented Jul 14, 2019

@hannobraun

what's required here is someone to figure out what using Future actually means in practice.

This is very true.

To this time I do also not have a lot of experience in async rust, perhaps the async book is a good place for me to start in order to read up on the current developments of the async story in rust core.

However in the meantime I might suggest a code snippet to be compatible with core::task::Poll:

/// Future adapter
///
/// This operations transforms a `nb::Result` to a `core::task::Poll`
///
#[cfg(feature = "unstable")]
#[macro_export]
macro_rules! poll {
    ($e:expr) => {
        match $e {
            Err($crate::Error::Other(e)) => core::task::Poll::Ready(core::result::Result::Err(e)),
            Err($crate::Error::WouldBlock) => core::task::Poll::Pending,
            Ok(x) => core::task::Poll::Ready(core::result::Result::Ok(x)),
        }
    }
}

@XOSplicer
Copy link

XOSplicer commented Jul 14, 2019

This code could also be implemented as a plain function. There is no need for it to be defined as a macro.

Maybe something like this is more usable:

pub fn into_poll<T, E>(r: crate::Result<T, E>)
    -> ::core::task::Poll<::core::result::Result<T, E>>
{
    match r {
        Err(crate::Error::Other(e)) => ::core::task::Poll::Ready(core::result::Result::Err(e)),
        Err(crate::Error::WouldBlock) => ::core::task::Poll::Pending,
        Ok(x) => ::core::task::Poll::Ready(core::result::Result::Ok(x)),
    }
}

pub fn as_poll<T, E>(r: &crate::Result<T, E>)
    -> ::core::task::Poll<::core::result::Result<&T, &E>>
{
    match r {
        Err(crate::Error::Other(ref e)) => ::core::task::Poll::Ready(core::result::Result::Err(e)),
        Err(crate::Error::WouldBlock) => ::core::task::Poll::Pending,
        Ok(ref x) => ::core::task::Poll::Ready(core::result::Result::Ok(x)),
    }
}

pub fn as_mut_poll<T, E>(r: &mut crate::Result<T, E>)
    -> ::core::task::Poll<::core::result::Result<&mut T, &mut E>>
{
    match r {
        Err(crate::Error::Other(ref mut e)) => ::core::task::Poll::Ready(core::result::Result::Err(e)),
        Err(crate::Error::WouldBlock) => ::core::task::Poll::Pending,
        Ok(ref mut x) => ::core::task::Poll::Ready(core::result::Result::Ok(x)),
    }
}

@piegamesde
Copy link

Thanks to the comments above sharing their async solutions. In that spirit, maybe someone will find my block_async! macro useful:

macro_rules! block_async {
    ($e:expr) => {
        loop {
            #[allow(unreachable_patterns)]
            match $e {
                Err(nb::Error::Other(e)) => {
                    #[allow(unreachable_code)]
                    break Err(e)
                }
                Err(nb::Error::WouldBlock) => {
                    async_std::task::yield_now().await;
                }
                Ok(x) => break Ok(x),
            }
        }
    };
}

It currently depends on async_std but if you look at the code behind yield_now that really isn't a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants