feat(processor): expose the process api #1395
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
While doing some testing in the compiler where I wanted to set up some VM state and then execute a program on that state, I realized that there is no currently public API that allows one to access the state of the process to do so, short of using the `internals` feature flag of the crate. Rather than requiring the use of that flag, this commit instead makes public the [Process] struct, but with all of its fields private unless the `internals` flag is enabled. This makes it possible to use the public APIs of the various traits implemented by `Process` to interact with it in a more fine-grained fashion. It is still expected that most people will use the `execute` or `execute_iter` APIs, but this makes it possible to take control over that yourself when necessary/useful. I should also note that we were making the [Process] struct visible for ourselves under `#[cfg(test)]`, so it is clearly useful in certain cases (namely testing).
bitwalker
added
enhancement
bobbinth
approved these changes
Jul 15, 2024
There was a problem hiding this comment.
Looks good! Thank you!
The initial intent was that if people want to interact with the Process struct directly, it is most likely for testing purposes and they would enable this via the internals feature. But I don't see any harm in exposing the process struct the way you have in this PR.
As an aside, we should probably rename internals feature into testing. @Fumuran - could you open issue/PR for this?
Comment on lines
+165
to
+175
| /// A [Process] is the underlying execution engine for a Miden [Program]. | ||
| /// | ||
| /// Typically, you do not need to worry about, or use [Process] directly, instead you should prefer | ||
| /// to use either [execute] or [execute_iter], which also handle setting up the process state, | ||
| /// inputs, as well as compute the [ExecutionTrace] for the program. | ||
| /// | ||
| /// However, for situations in which you want finer-grained control over those steps, you will need | ||
| /// to construct an instance of [Process] using [Process::new], invoke [Process::execute], and then | ||
| /// get the execution trace using [ExecutionTrace::new] using the outputs produced by execution. | ||
| #[cfg(not(any(test, feature = "internals")))] | ||
| struct Process<H> | ||
| pub struct Process<H> |
There was a problem hiding this comment.
Thank you for writing these docs!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While doing some testing in the compiler where I wanted to set up some VM state and then execute a program on that state, I realized that there is no currently public API that allows one to access the state of the process to do so, short of using the
internalsfeature flag of the crate.Rather than requiring the use of that flag, this commit instead makes public the [Process] struct, but with all of its fields private unless the
internalsflag is enabled. This makes it possible to use the public APIs of the various traits implemented byProcessto interact with it in a more fine-grained fashion.It is still expected that most people will use the
executeorexecute_iterAPIs, but this makes it possible to take control over that yourself when necessary/useful. I should also note that we were making the [Process] struct visible for ourselves under#[cfg(test)], so it is clearly useful in certain cases (namely testing).