Update StackOutputs fields
#1268
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
Fumuran
force-pushed
the
andrew-update-stack-outputs
branch
2 times, most recently
from
4f7f8ca
to
7dec536
Compare
bobbinth
approved these changes
Mar 5, 2024
miden/src/examples/mod.rs
Outdated
| @@ -170,7 +170,7 @@ where | |||
| let program_info = ProgramInfo::new(program.hash(), kernel); | |||
|
|
|||
| if fail { | |||
| outputs.stack_mut()[0] += 1; | |||
| outputs.stack_mut()[0] += Felt::new(1); | |||
test-utils/src/lib.rs
Outdated
| @@ -252,7 +252,7 @@ impl Test { | |||
|
|
|||
| let program_info = ProgramInfo::from(program); | |||
| if test_fail { | |||
| stack_outputs.stack_mut()[0] += 1; | |||
| stack_outputs.stack_mut()[0] += Felt::new(1); | |||
core/src/stack/outputs.rs
Outdated
| /// Returns an error if: | ||
| /// - Any of the provided stack elements are invalid field elements. | ||
| /// - Any of the provided overflow addresses are invalid field elements. | ||
| pub fn try_from_values(stack: Vec<u64>, overflow_addrs: Vec<u64>) -> Result<Self, OutputError> { |
There was a problem hiding this comment.
nit: I would rename this to try_from_ints.
|
Oh - and also, let's update the changelog. |
Fumuran
force-pushed
the
andrew-update-stack-outputs
branch
2 times, most recently
from
50e2e09
to
3677c75
Compare
Fumuran
force-pushed
the
andrew-update-stack-outputs
branch
from
3677c75
to
8c66eb0
Compare
bobbinth
reviewed
Mar 6, 2024
| @@ -4,6 +4,7 @@ | |||
|
|
|||
| #### VM Internals | |||
| - Removed unused `find_lone_leaf()` function from the Advice Provider (#1262). | |||
| - Changed fields type of the `StackOutputs` struct from `Vec<u64>` to `Vec<Felt>` (#1268). | |||
There was a problem hiding this comment.
This is a breaking change (i.e., it changes the public interface). We should mark it as such.
There was a problem hiding this comment.
Ah, Indeed, I merged this PR too early. Will it be OK if I make this change in the #1266 PR?
bobbinth
pushed a commit
that referenced
this pull request
Apr 3, 2024
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.
This PR updates the fields of
StackOutputsstruct.It changes them from
Vec<u64>toVec<Felt>to make them consistent with the stack values.