Skip to content

Change traits to take &mut self instead of &self #16

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

Closed
wants to merge 1 commit into from

Conversation

f-bro
Copy link

@f-bro f-bro commented Jun 21, 2017

Copy link
Member

@japaric japaric left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request @f-bro!

With these changes all the futures examples will need to be updated to take e.g. S by value otherwise code like this won't compile:

fn test<'a, S>(s: &'a mut S) -> impl Future<Item = (), Error = S::Error> + 'a
where
    S: Serial,
{
    write(s, b'H').and_then(move |_| write(s, b'e'))
}
error[E0504]: cannot move `s` into closure because it is borrowed
  --> src/main.rs:33:44
   |
33 |     write(s, b'H').and_then(move |_| write(s, b'e'))
   |           - borrow of `*s` occurs here     ^ move into closure occurs here

error: aborting due to previous error(s)

This is known futures "issue" (well not an issue if follows from how futures work).

For example write will have to be changed to look like this:

fn write<S>(mut s: S, byte: u8) -> impl Future<Item = S, Error = S::Error>
where
    S: Serial,
{
    future::poll_fn(move || {
        try_nb!(s.write(byte));
        Ok(Async::Ready(s))
    })
}

Except that that doesn't work either so you'll have to figure out some solution. Once that works then we should be able to rewrite foo to look like this:

fn test<'a, S>(s: S) -> impl Future<Item = S, Error = S::Error>
where
    S: Serial,
{
    write(s, b'H').and_then(move |s| write(s, b'e'))
}

@homunkulus
Copy link
Contributor

☔ The latest upstream changes (presumably 7d904f5) made this pull request unmergeable. Please resolve the merge conflicts.

japaric pushed a commit that referenced this pull request Oct 17, 2017
Change traits to take `&mut self`, where appropriate

This pull request supersedes #16, which seems stalled. It is based on #19. Only the last two commits are actually relevant to this pull request itself. I suggest merging #19 first, then I can rebase this one.
@hannobraun
Copy link
Member

@japaric This pull request has been inactive for a while, and the changes have been made in another pull request. I think this can be closed.

@japaric
Copy link
Member

japaric commented Dec 1, 2017

Yes, thanks for the heads up @hannobraun.

@f-bro Thanks for the original implementation!

@japaric japaric closed this Dec 1, 2017
peckpeck pushed a commit to peckpeck/embedded-hal that referenced this pull request Nov 10, 2022
16: Change maintaining team r=ryankurte a=mathk



Co-authored-by: mathk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants