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

Enable serialization of source locations for AstSerdeOptions #1177

Open
Dominik1999 opened this issue Dec 10, 2023 · 1 comment
Open

Enable serialization of source locations for AstSerdeOptions #1177

Dominik1999 opened this issue Dec 10, 2023 · 1 comment
Assignees

Comments

@Dominik1999
Copy link
Contributor

Dominik1999 commented Dec 10, 2023

I believe this will serialize note scripts without source locations. Serializing with source locations now is a bit cumbersome - so, maybe we should just create two issues for now:

First issue would be in Miden VM to enable serialization of source locations by simply setting the appropriate property for AstSerdeOptions. For example, here, it should work something like this:

let note_script_bytes = note_script_ast.to_bytes(AstSerdeOptions {
    serialize_imports: true,
    serialize_source_locations: true,
});

The second issue would be here so that we don't forget to come back and fix this once the issue in Miden VM is fixed.

Originally posted by @bobbinth in 0xPolygonMiden/miden-base#339 (comment)

@igamigo
Copy link

igamigo commented Jan 8, 2024

This should be done for ModuleAst as well. The following tests currently fails because source locations are not being serialized correctly:

    let account = account::mock_account(Some(account_id.into()), Felt::ZERO, None, &assembler);

    let account_module = account.code().module();
    let account_module_bytes = account_module.to_bytes(AstSerdeOptions{serialize_imports:true});

    let reconstructed_ast = ModuleAst::from_bytes(&account_module_bytes).unwrap();

    assert_eq!(*account_module, reconstructed_ast)

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 a pull request may close this issue.

4 participants