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

Allow software tasks to diverge (return -> !) and give them 'static context. #1043

Merged
merged 10 commits into from
Mar 27, 2025

Conversation

Ddystopia
Copy link
Contributor

@Ddystopia Ddystopia commented Mar 22, 2025

This pull request allows software tasks to return ! and gives Context<'static> to them. In practice this means that polling a task is statically guaranteed to return Poll::Pending. The code will to run until the program's end, thus 'static is justified.

'static is really a superpower and is incredibly useful, as described in the book. Many tasks are not meant to ever end, and 'static can reduce amount of unwanted lifetime annotations, allows better interaction with statics, which allows developers to see usage of the RAM in fine-grained detail and has other great properties.

@datdenkikniet
Copy link
Contributor

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 'static lifetime. It would be great if you could add a section/mention of this new feature in the section you linked to highlight another way of getting this lifetime.

Copy link
Collaborator

@korken89 korken89 left a 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.

@Ddystopia
Copy link
Contributor Author

Ddystopia commented Mar 23, 2025

@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?

@datdenkikniet
Copy link
Contributor

datdenkikniet commented Mar 23, 2025

An additional note: to be extra clear, it may be good to mention in the book that you must/should have a .await somewhere in the diverging path to ensure that tasks at the same or other priorities still get to run.

@Ddystopia
Copy link
Contributor Author

Ddystopia commented Mar 23, 2025

An additional note: to be extra clear, it may be good to mention in the book that you must/should have a .await somewhere in the diverging path to ensure that tasks at the same or other priorities still get to run.

This is already mentioned in the book:

While hardware tasks are assumed to run-to-completion (and return), software tasks may be started (spawned) once and run forever, on the condition that any loop (execution path) is broken by at least one await (yielding operation).

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.

@Ddystopia
Copy link
Contributor Author

@korken89 I added an example, as we discussed yesterday.

@Ddystopia Ddystopia requested a review from korken89 March 27, 2025 11:42
Copy link
Collaborator

@korken89 korken89 left a 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!

@korken89 korken89 added this pull request to the merge queue Mar 27, 2025
Merged via the queue into rtic-rs:master with commit cb7d053 Mar 27, 2025
62 checks passed
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.

3 participants