Add WasmWriter trait for writing to wasm memory
#610
Conversation
+ impl WasmWriter for WasmGenerator for future optimizations
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #610 +/- ##
==========================================
+ Coverage 85.65% 85.78% +0.13%
==========================================
Files 46 46
Lines 24554 24700 +146
Branches 24554 24700 +146
==========================================
+ Hits 21032 21190 +158
+ Misses 1672 1657 -15
- Partials 1850 1853 +3 ☔ View full report in Codecov by Sentry. |
| @@ -721,36 +721,50 @@ pub fn placeholder_for_type(ty: ValType) -> Val { | |||
| } | |||
| } | |||
|
|
|||
| pub trait WasmWriter { | |||
| type Error: Into<Error>; | |||
There was a problem hiding this comment.
Do we really need the constraint? Not sure what benefits it brings to the table.
| .set(&mut writer.0, Val::I32(offset)) | ||
| .map_err(|e| Error::Wasm(WasmError::Runtime(e)))?; | ||
|
|
||
| // Call the function | ||
| func.call(&mut store, &wasm_args, &mut results) | ||
| func.call(&mut writer.0, &wasm_args, &mut results) | ||
| .map_err(|e| { | ||
| error_mapping::resolve_error(e, instance, &mut store, &epoch, &clarity_version) | ||
| error_mapping::resolve_error(e, instance, &mut writer.0, &epoch, &clarity_version) | ||
| })?; | ||
|
|
||
| // If the function returns a value, translate it into a Clarity `Value` | ||
| wasm_to_clarity_value(&return_type, 0, &results, memory, &mut &mut store, epoch) | ||
| wasm_to_clarity_value(&return_type, 0, &results, writer.1, &mut writer.0, epoch) |
There was a problem hiding this comment.
We should keep store and memory, it's clearer.
| let value_type = writer | ||
| .0 | ||
| .data() | ||
| .contract_analysis |
There was a problem hiding this comment.
Not super convinced of all those usages of writer.0. The following operations usually have nothing to do with the writer. I understand it's a borrow checker thing, but I'm not super happy with the result...
There was a problem hiding this comment.
Maybe destructuring the tuple into caller and memory immediately can avoid repeatedly accessing tuple indices (i.e., writer.0 and writer.1) and help the borrow checker keep track of mutable and immutable references.
let (mut caller, memory) = (caller, memory); on line 127. (haven't tested that)
This PR adds a trait that is going to be used for writing directly to wasm memory. It is going to be useful for future optimizations where writing to wasm memory will be needed.