SY-3880: Arc String Conversion Support#2298
Conversation
Allows numeric primitives (i8-i64, u8-u64, f32, f64) to convert to str via the existing typecast syntax; floats use shortest round-trippable formatting. Reverse direction (str -> numeric) remains rejected. Routing lives in a single Resolver.EmitNumericToString dispatcher so the planned f-string feature can reuse it.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## rc #2298 +/- ##
==========================================
+ Coverage 64.75% 64.78% +0.03%
==========================================
Files 2592 2596 +4
Lines 112597 113543 +946
Branches 8346 8396 +50
==========================================
+ Hits 72911 73562 +651
- Misses 33565 33830 +265
- Partials 6121 6151 +30
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
emilbon99
left a comment
emilbon99
left a comment
There was a problem hiding this comment.
Looks pretty good overall. Comments are mostly nitpicks.
| const auto off = this->offsets[j]; | ||
| this->state.output(j)->resize(off); | ||
| if (this->string_outputs[j]) { | ||
| *this->state.output( |
There was a problem hiding this comment.
The formatting here is weird. See if you can coerce the formatter to be nicer. Also you can get rid of the {} since there is a single expression.
| } | ||
|
|
||
| // EmitStringFromI32 emits a call to string.from_i32. | ||
| func (r *Resolver) EmitStringFromI32(w *wasm.Writer, wID int) { |
There was a problem hiding this comment.
Do all of these sub-functions need to be public? Is there anywhere we call anything other than EmitNumericToString?
There was a problem hiding this comment.
Collapsed them all into EmitNumericToString since nothing else called them.
| } | ||
|
|
||
| // EmitStringFromU32 emits a call to string.from_u32. | ||
| func (r *Resolver) EmitStringFromU32(w *wasm.Writer, wID int) { |
There was a problem hiding this comment.
Seems like a lot of duplicated code across these functions. output type is always the same. ONly thing that varies is method name and input type. Could always pass the input type as a parameter then do fmt.Sprintf("string.from_%s", t). Would cut out vast majority of changes in this file.
There was a problem hiding this comment.
Folded into a single EmitNumericToString with a switch picking the input type and suffix, then "string.from_" + suffix since it's marginally faster than fmt.Sprintf()
| Outputs: types.Params{{Name: ir.DefaultOutputParam, Type: types.I64()}}, | ||
| }), | ||
| }, | ||
| "from_i32": { |
There was a problem hiding this comment.
Kind of interesting that the type signatures are duplicated here and in the wasm writer. Nothing super wrong with it but I did notice it.
There was a problem hiding this comment.
Added EmitFixedCall that looks up the signature from the SymbolResolver by name, so string.go is the sole source of truth now.
2 participants
Issue Pull Request
Linear Issue
SY-3880
Description
Adds support for converting numeric primitives (i8-i64, u8-u64, f32, f64) to strings via Arc's existing typecast syntax (e.g.,
str(42),str(3.14)). Floats use shortest round-trippable formatting matching Go'sstrconv.FormatFloat(_, 'g', -1, bitSize); the reverse direction (str to numeric) remains rejected.str()also works as a flow stage (channel -> str -> log). Both Go and C++ WASM runtimes now materialize string-typed outputs into a fresh series at the end of each tick rather than attempting to resize variable-density buffers in place, which previously caused a runtime trap.Conversion routing lives in a single
Resolver.EmitNumericToStringdispatcher so the planned f-string feature can reuse it without duplicating the source-type to host-fn switch.Basic Readiness
Greptile Summary
This PR adds
str()typecast support that converts numeric primitives (i8–i64, u8–u64, f32, f64) to their decimal string representation, and fixes a runtime trap that occurred when attempting to resize variable-density string buffers in place during WASM tick evaluation.Resolver.EmitNumericToString, dispatching to one of six newstring.from_*host functions (backed bystrconv.FormatFloatwith the correctbitSizeon the Go side andstd::to_chars(…, std::chars_format::general)on the C++ side). Float formatting matches Go'sstrconv.FormatFloat(_, 'g', -1, bitSize)contract.Resizeon a variable-density buffer (which would panic or trap). Thestr()cast also works as an inline flow stage (channel -> str -> log).Confidence Score: 5/5
The change is safe to merge. The numeric-to-string conversion path is well-isolated, the variable-density buffer fix correctly avoids the Resize trap in both runtimes, and test coverage spans unit, integration, and end-to-end layers across all numeric source types.
The core logic — accumulating string handles into a temporary slice per tick and materialising a fresh series at the end — is symmetric between Go and C++ runtimes and verified by multi-sample tests. The Go Data-only assignment is correct because telem.Series.Len() for variable-density types always recomputes from Data (Resize panics on variable-density, so cachedLength is never set persistently for string series). Float formatting uses the right bitSize (32 vs 64) to match Go's strconv contract. No logic errors or unsafe paths were found.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["str(x) in Arc source"] --> B{Analyzer isValidCast} B -->|"numeric source"| C["Compiler: compileTypeCast\nhint suppressed for str target"] B -->|"str source"| D["Rejected: cannot cast"] C --> E["EmitCast → EmitNumericToString"] E --> F{source kind} F -->|"i8/i16/i32"| G["string.from_i32"] F -->|"u8/u16/u32"| H["string.from_u32"] F -->|"i64"| I["string.from_i64"] F -->|"u64"| J["string.from_u64"] F -->|"f32"| K["string.from_f32"] F -->|"f64"| L["string.from_f64"] G & H & I & J --> M["Host fn: FormatInt/Uint → string handle uint32"] K & L --> N["Host fn: FormatFloat 'g' -1 bitSize → string handle uint32"] M & N --> O["WASM stack: i32 handle"] O --> P{Runtime: string output?} P -->|yes| Q["Accumulate into stringResults per tick"] P -->|no| R["setValueAt numeric output"] Q --> S["End of tick: materialise fresh Series from accumulated strings"] R --> T["Resize to sample count"]Reviews (4): Last reviewed commit: "SY-3380: Implement feedback" | Re-trigger Greptile