-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
RFC: Task spawn options interface #2954
Comments
It'd be cool if /me shamelessly wants to see his feature used more :) |
I've no strong preference on this API. So long as the default is easy to type and the variants can all be specified in O(number-of-non-defaults) worth of work, I don't really care if we use FRU, this sort of "fluent" OO thing, an optional list of enums ... whatever. I'm slightly less fond of the name duplication you get by omitting the "chain start" call in this style, mind you. Might prefer |
@graydon In this case I think I lean towards the most absurdly-over-the-top-easy-to-type thing. How about |
I suppose. Don't personally care a lot. Though I think people will complain. Should probably be something with a little more meaning. Setup? |
"task::task()"? (edit: argh, it won't show up in code format if it's the only thing) |
Also, taskgroup handles can probably fit easily here:
|
Probably the reason that move-on-self got mentioned is that some of the options create ports and so forth and those would only be valid for one spawn. Nothing in this API prevents you from doing something like:
I believe joinable was the example that would cause a problem. I am not sure that the old API prevented this condition either, though: I know @brson wanted to use a resource to prevent copying at some point, but I can't remember if we decided to just say "don't do that". |
builders are currently noncopyable, by means of a dummy port<()>. i think i'd like to get rid of that, in hopes of move mode on self, and use an option with runtime-copy-failure as a holdover. |
If we want builder to be noncopyable, why not just make it a class with a destructor? |
eventually it should be noncopyable, but in order to do the chaining thing without move-self it needs to be copyable for now (or at least reconstructable). something like:
|
Also I think there should be some default functions that don't require you to go through even this more-minimal builder interface. don't wanna get too heavy with these, but don't want to require the builder interface in the majority of cases either. |
This is implemented. |
cron auto-PR: can't seem to avoid repeating the branch name...
The "CBMC latest" workflow is composed of two jobs (`regression` and `perf`) which perform testing with the most recent development version of CBMC. At present, the `regression` jobs are not actually testing with the CBMC that we build from source, but the CBMC installed through the setup scripts, as revealed in rust-lang#2954. This PR changes the `regression` jobs so that they use `cmake` to build. This allows the runner to pick up the recently-built CBMC development version instead of the one installed through setup scripts, as it's done in the `perf` jobs. Unfortunately, [this CI run](https://github.com/adpaco-aws/rmc/actions/runs/7390380572) doesn't demonstrate the fix as it should due to an unrelated breaking change in the latest CBMC version. However, rust-lang#2952 provides more context in case you need it.
A couple weeks ago we discussed being able to 'chain' task spawn options, which would be much more fun to write than the current builder interface.
I envision:
With the functions at the top level returning a default set. Then you'll never need to type "builder" - all of these will be valid:
One problem is noncopyability of certain things that might get put in builder - like wrappers and notify ports (or future stuff...? not entirely clear on that).
Move mode on self can solve that; until then, we could perhaps make it a runtime error by doing the standard option dance. (When move mode on self appears, we can make it a build error without changing the interface.)
An attempt to use a builder with
none
in it would fail at runtime.The text was updated successfully, but these errors were encountered: