Enable serialization of source locations for AstSerdeOptions
#1211
Conversation
The sentence describing the column $k_0$ had its order reversed, and the table headers were incorrectly named as $x_i$ and $y_i$ instead of $a_i$ and $b_i$ as described in the introduction.
Introduce `std::utils` assembly module
… info This commit changes the `import_info` fields of ProgramAst and ModuleAst to remove the `Option` wrapper, since it does not appear to serve a specific purpose, and both complicates access to the imports, and requires redundant code in a few places to access common information from the imports. It also required fallible checks in a few places where empty module import info would be suitable as a fallback. After this change, one can access the ModuleImports struct directly on both ProgramAst and ModuleAst via the `import_info` function, which returns a reference to the field. Redundant functions in both structs were removed if they are already provided by ModuleImports.
docs(mdbook): minor change in the bitwise chiplet
Expand capabilities of the `debug` instruction
add .editorconfig
Minor doc fixes
Minor doc fix LogUp
…dler Add `emit` instruction and host event handler
Chore: pre-commit sync
* ContextId type * propagate MemoryContextId * fix tests * MemoryContextId -> ContextId * move definition * Remove derive-more dependency * fix repl * fmt * docstring * import newline * Remove `Sub` from `ContextId` * blank line * section name change * blank line * use `ContextId::root()` * fmt * memory context -> execution context
Use RpoRandomCoin from crypto crate
Add logging to the VM
Explanation of field arithmetic
* docs: fix typos * air: fix typos
Add `on_assert_failed()` method to the Host trait
Add tracing to the `Host` interface
…nstants Support hexadecimal literals in constants
There was a problem hiding this comment.
Looks good! Thank you! Not a full review yet - but I left a few comments inline.
I'm also wondering if we should add something like a minify() method to ProgramAst and ModuleAst. This would basically take the current structures and remove all import and source location data (i.e., data which is not strictly need to compile the code).
|
|
||
| // serialize source locations if required | ||
| if options.serialize_source_locations { | ||
| self.write_source_locations(target); |
There was a problem hiding this comment.
Should we get rid of write_source_locations method and move the code here? Is there a reason we'd need it after these chagnes?
|
|
||
| // deserialize source locations if required | ||
| if options.serialize_source_locations { | ||
| module_ast.load_source_locations(source)?; |
There was a problem hiding this comment.
Similar comment as above: should we get rid of load_source_locations as a separate method?
assembly/src/ast/program.rs
Outdated
There was a problem hiding this comment.
Similar comments here as above regarding needing load_source_locations and write_source_locations as separate functions (or at least as separate public functions).
There was a problem hiding this comment.
All these functions (in both ModuleAst and ProgramAst) are used in tests (in assert_correct_program_serialization() and assert_correct_module_serialization functions), so I'm not sure that we should (re)move them. Write and Read functions for ModuleAst are also used in Module in library crate.
assembly/src/library/masl.rs
Outdated
| // write the flag whether the source locations should be serialized | ||
| target.write_bool(self.has_source_locations); |
There was a problem hiding this comment.
Should we replace has_source_locations in MaslLibrary with full serde_options? This way, we treat both options (and potential future options consistently).
|
@Overcastan are you still working on this? |
|
We can probably close this as it will not longer be relevant in MAST-based serialization. |
This PR implements the additional
serialize_source_locationsproperty in theAstSerdeOptions, allowing to choose whether source locations should be serialized.According to this, serialization of source locations was implemented in the
ProgramAstandModuleAststructures.