-
Notifications
You must be signed in to change notification settings - Fork 242
Allow software tasks to diverge (return -> !) and give them 'static context.
#1043
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
Conversation
|
I think this is quite a cool feature to have! Sadly I don't have enough know-how to give a useful insight on the soundness of this PR, but I do have one suggestion: as you mention, the book nicely highlights the superpowers you get from a |
korken89
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR!
This makes perfect sense since the introduction of async tasks, where before that it was unsound to have -> !.
I will add it to the weekly meeting agenda so we can discuss this. On the implementation, I'd like you to add at least one test checking this new functionality.
|
@korken89 Thanks for the review. Can you please point me at the example of such test? I only see compile-fail ui tests, that's probably not what you mean? Because I don't really know how to confirm something with compile-fail test 😅. Maybe you meant something like example or doc test? |
|
An additional note: to be extra clear, it may be good to mention in the book that you must/should have a |
This is already mentioned in the book:
By the way, when we read the documentation, we were confused by this paragraph because it looks like a requirement that rtic enforces at compile time, and we didn't understand how that could work. |
|
@korken89 I added an example, as we discussed yesterday. |
korken89
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, thank you!
Each mutex is generated uniquely for each task, it is unsound to send them between tasks. But they are `Send`. Before, it wasn't an issue, because you couldn't share non-`'static` data between them, but with \rtic-rs#1043 you can make the mutex `'static`. Thus we need to use actual tools that Rust provides and out out from `Send`. Currently, mutexes are simple ZSTs with `PhantomData<&'a ()>`, which is `Send`. We replace it with `PhantomData<(&'a (), *const u8)>`, and return `Sync` back via `unsafe` implementation. It is trivially sound, because mutexes have no method methods that accept `&self`. See https://doc.rust-lang.org/std/sync/struct.Exclusive.html for details.
Each mutex is generated uniquely for each task, it is unsound to send them between tasks. But they are `Send`. Before, it wasn't an issue, because you couldn't share non-`'static` data between them, but with \rtic-rs#1043 you can make the mutex `'static`. Thus we need to use actual tools that Rust provides and out out from `Send`. Currently, mutexes are simple ZSTs with `PhantomData<&'a ()>`, which is `Send`. We replace it with `PhantomData<(&'a (), *const u8)>`, and return `Sync` back via `unsafe` implementation. It is trivially sound, because mutexes have no method methods that accept `&self`. See https://doc.rust-lang.org/std/sync/struct.Exclusive.html for details.
This pull request allows software tasks to return
!and givesContext<'static>to them. In practice this means that polling a task is statically guaranteed to returnPoll::Pending. The code will to run until the program's end, thus'staticis justified.'staticis really a superpower and is incredibly useful, as described in the book. Many tasks are not meant to ever end, and'staticcan reduce amount of unwanted lifetime annotations, allows better interaction withstatics, which allows developers to see usage of the RAM in fine-grained detail and has other great properties.