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

Fix CairoRunner::write_output so that it prints missing and relocatable values #853

Merged
merged 2 commits into from
Feb 17, 2023

Conversation

fmoletta
Copy link
Member

@fmoletta fmoletta commented Feb 17, 2023

Currently our vm only prints values in the output segment if they are integer values, and fails if they are not.
This PR aims to fix this behaviour and also print missing and relocatable values as the original vm does.
Expample program:

%builtins output

func main{output_ptr: felt*}() {
    //Memory Gap + Relocatable value
    assert [output_ptr + 1] = cast(output_ptr, felt);
    let output_ptr = output_ptr + 2;
    return ();
}

Example program output:

Program Output: 
<missing>
2:0

Depends on #852

@fmoletta fmoletta changed the base branch from main to memory-get-option February 17, 2023 19:12
@fmoletta fmoletta marked this pull request as ready for review February 17, 2023 19:13
@codecov
Copy link

codecov bot commented Feb 17, 2023

Codecov Report

Merging #853 (ee0b2c8) into memory-get-option (2016148) will decrease coverage by 0.01%.
The diff coverage is 97.56%.

@@                  Coverage Diff                  @@
##           memory-get-option     #853      +/-   ##
=====================================================
- Coverage              96.78%   96.78%   -0.01%     
=====================================================
  Files                     69       69              
  Lines                  28726    28757      +31     
=====================================================
+ Hits                   27802    27832      +30     
- Misses                   924      925       +1     
Impacted Files Coverage Δ
src/vm/runners/cairo_runner.rs 97.53% <97.56%> (-0.01%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

.get(&Relocatable::from((segment_index as isize, i)))
{
Some(val) => match val.as_ref() {
MaybeRelocatable::Int(num) => format!("{}", num.to_bigint()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a later refactor: it could be a better idea to implement Display for Felt instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is implemented, the reason for using to_bigint() is to change the range of the felt (from [0,P] to [-P/2, P/2})

@juanbono juanbono merged commit eaa7ab1 into memory-get-option Feb 17, 2023
@juanbono juanbono deleted the fix-write-output branch February 17, 2023 20:29
fmoletta added a commit that referenced this pull request Feb 23, 2023
* Use only option for Memory.get

* Fix some tests + refactor range_check validation

* use proper error for get_memory_holes

* Move MaybeRelocatable methods get_int_ref & get_reloctable to Option

* Fix tests

* Clippy

* Fix `CairoRunner::write_output` so that it prints missing and relocatable values (#853)

* Print relocatables & missing members in write_output

* Add test
Oppen pushed a commit that referenced this pull request Mar 6, 2023
* Use only option for Memory.get

* Fix some tests + refactor range_check validation

* use proper error for get_memory_holes

* Move MaybeRelocatable methods get_int_ref & get_reloctable to Option

* Fix tests

* Clippy

* Fix `CairoRunner::write_output` so that it prints missing and relocatable values (#853)

* Print relocatables & missing members in write_output

* Add test

* Move errors outputed by math_utils to MathError

* Start moving relocatable operations to MathError

* Fix tests

* Remove math-related errors from vm error

* Move conversion errors to MathError

* Move type conversions to MathError

* Remove unused errors

* Clippy

* Clippy

* Simplify addition

* Simplify addition

* Clippy

* Add math_errors.rs

* Check for overflows in relocatable operations (#859)

* Catch possible overflows in Relocatable::add

* Move sub implementations to trait impl

* Swap sub_usize for - operator

* Vheck possible overflows in Add<i32>

* Fix should_panic test

* remove referenced add

* Replace Relocatable methods for trait implementations

* Catch overflows in mayberelocatable operations

* Fix keccak

* Clippy
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.

None yet

3 participants