Skip to content

Commit f61b9fc

Browse files
committed
Fix Vec::get_ref match arms to borrow drop-owning struct elements as pointers instead of copying them, preventing nested Vec double-free on repeated scans. Add test_vec_get_ref_scan_nested_vec.ion and record match pattern bindings in IR for nested scans.
1 parent 3697969 commit f61b9fc

10 files changed

Lines changed: 289 additions & 21 deletions

File tree

.cursor/skills/writing-ion-code/references/verified-patterns.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ s.push_str("hi");
4747
let lit: String = "hello";
4848
```
4949

50-
`Vec::get` / `Vec::pop` return `Option<T>` and **move** the element out. For read-only scans use `Vec::get_ref(&v, i)` which returns `Option<&T>` (local temporary only; cannot return or store). `Vec::set(&mut v, index, value)` returns `int` (0 = ok).
50+
`Vec::get` / `Vec::pop` return `Option<T>` and **move** the element out. For read-only scans use `Vec::get_ref(&v, i)` which returns `Option<&T>` (local temporary only; cannot return or store). In `match Option::Some(x)`, `x` is `&T`: copy types bind by value; structs with owned fields bind as a pointer so nested `Vec` fields are not dropped. `Vec::set(&mut v, index, value)` returns `int` (0 = ok).
5151

5252
## Box
5353

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
- **LSP**: diagnostics, hover, completion, go-to-definition; multi-error reporting; symbol table mirroring; diagnostics cleared on close; hover fixes for `let` bindings and module-qualified calls. Contiguous `//` doc comments attached to declarations (including `pub` items) for hover.
1010
- **Tooling**: GitHub Actions CI (Linux and Windows), pinned toolchain (1.96.0), `test_expectations.tsv` manifest, `--version`, line-numbered errors, Cursor agent skills. Split `tc` and `cgen` into submodules. `ion-build` driver and `ion.toml` manifests (`single`/`multi`, `out_dir`, `cflags`, `ldflags`, `stdlib_paths`, `emit_in_source`); per-example manifests and `build_hello`/`build_bad_main` harness smoke tests. Shared `discover_import_config` and stdlib search paths for `ion-compiler`, `ion-build`, and LSP. Integration harness precompiles `runtime/ion_runtime.c` once per run (`RUNTIME_OBJ`). `writing-ion-code` agent skill; `creating-ion-skills` examples index lists all eight project skills. Documented checked-in `examples/*.c` codegen snapshots in README and integration-test skill; regenerated example C output. Fixed `researching-pl-literature` skill `paper-seeds` reference formatting.
1111
- **Docs**: README, CONTRIBUTING, ION_SPEC, and agent skills aligned on project layout, `ion-build` workflow, `emit_in_source`, stdlib import order, LSP features and limitations, `src/build/` checklists, portable Git Bash paths for `test_runner.sh`, and rebuilding release binaries before harness or example C regen (stale `target/release/ion-compiler` note).
12-
- **Fixes**: match rvalue codegen, `Vec` struct drops, channel codegen, parser handling of `alias::call()`, scope-drop for moved-into-call bindings, HTTP server on Windows, integration harness on Windows. Cgen return-unwind: stop marking bindings dropped after inner-branch `return` (restores outer-path drops), dedupe `_`-prefixed unused silences, mark call arguments moved at emission to avoid double-free on unwind, and unify all function returns via `emit_function_return` (including diverging rvalue `match` arms). Cgen struct field move-out neutralizes owned fields after partial move, deferred to statement end when the move is a call argument; `Vec::push` passes address of struct lvalues; non-generic enums emit before structs; tuple IR uses resolved element types and `tuple_Vec_T` mangling with compound-zero `ret_val` for tuple returns (functions and fn literals). Integer indexing and `Vec<i32>` inference in the type checker. Match rvalue arms: reject diverging arms mixed with value arms; structural control-flow analysis for arm bodies (nested `if`/`else`, `loop` without `break`, `unsafe` blocks); reject mixed diverge and value-producing paths within one arm; cgen assigns through `if`/`else` value branches. `fmt::int_to_string` uses `String::push_byte` per digit instead of per-digit `push_str` branches; integration test asserts `0`, negatives, and `int::MIN`. `int::MIN`/`int::MAX` on integer primitives; `Vec::new()`/`with_capacity()` infer `T` from a `let` type annotation. Clippy fix in `portable_source_label`. `Vec::get`/`Vec::pop`: IR infers `Option<T>` from the vector argument; cgen dereferences `&mut Vec<T>` parameters, unpacks runtime `Option` via `ion_option_from_raw`, and uses a temp for struct-returning `Vec::push`/`Vec::set` call values. Grouped `match` switch arms: scope drops before the case `break` (GCC `-Wimplicit-fallthrough` under `-Werror`); guarded arms in a shared variant case break inside the guard `if` so fallback arms do not run. Cgen: enum literals lower to compound initializers (no per-variant `_new` helpers); unused bindings and parameters silenced with `(void)` at scope unwind instead of blanket `ION_MAYBE_UNUSED`. Cgen monomorphization: `Vec<T>`/`Option<T>` typedefs mangle Ion type names (`Vec_String`, not C typedefs); `match Vec::get`/`Vec::pop` resolves `Option<T>` from return type or vector element type; `String::push_str` with owned `String` reads source `.data`/`.len`. Integration tests `test_vec_string_mangle`, `test_vec_get_multi_option`, `test_string_push_str_owned`, `test_vec_get_putback`. ION_SPEC documents `Vec::get` move-out and put-back scan. **`Vec::get_ref`**: stack-local `Option<&T>` for read-only vector peek; shared borrow on the vector owner; integration tests `test_vec_get_ref_*`. `String::len` codegen: null-check the `String` pointer, not `&local` (fixes GCC `-Waddress` under CI `-Werror`).
12+
- **Fixes**: match rvalue codegen, `Vec` struct drops, channel codegen, parser handling of `alias::call()`, scope-drop for moved-into-call bindings, HTTP server on Windows, integration harness on Windows. Cgen return-unwind: stop marking bindings dropped after inner-branch `return` (restores outer-path drops), dedupe `_`-prefixed unused silences, mark call arguments moved at emission to avoid double-free on unwind, and unify all function returns via `emit_function_return` (including diverging rvalue `match` arms). Cgen struct field move-out neutralizes owned fields after partial move, deferred to statement end when the move is a call argument; `Vec::push` passes address of struct lvalues; non-generic enums emit before structs; tuple IR uses resolved element types and `tuple_Vec_T` mangling with compound-zero `ret_val` for tuple returns (functions and fn literals). Integer indexing and `Vec<i32>` inference in the type checker. Match rvalue arms: reject diverging arms mixed with value arms; structural control-flow analysis for arm bodies (nested `if`/`else`, `loop` without `break`, `unsafe` blocks); reject mixed diverge and value-producing paths within one arm; cgen assigns through `if`/`else` value branches. `fmt::int_to_string` uses `String::push_byte` per digit instead of per-digit `push_str` branches; integration test asserts `0`, negatives, and `int::MIN`. `int::MIN`/`int::MAX` on integer primitives; `Vec::new()`/`with_capacity()` infer `T` from a `let` type annotation. Clippy fix in `portable_source_label`. `Vec::get`/`Vec::pop`: IR infers `Option<T>` from the vector argument; cgen dereferences `&mut Vec<T>` parameters, unpacks runtime `Option` via `ion_option_from_raw`, and uses a temp for struct-returning `Vec::push`/`Vec::set` call values. Grouped `match` switch arms: scope drops before the case `break` (GCC `-Wimplicit-fallthrough` under `-Werror`); guarded arms in a shared variant case break inside the guard `if` so fallback arms do not run. Cgen: enum literals lower to compound initializers (no per-variant `_new` helpers); unused bindings and parameters silenced with `(void)` at scope unwind instead of blanket `ION_MAYBE_UNUSED`. Cgen monomorphization: `Vec<T>`/`Option<T>` typedefs mangle Ion type names (`Vec_String`, not C typedefs); `match Vec::get`/`Vec::pop` resolves `Option<T>` from return type or vector element type; `String::push_str` with owned `String` reads source `.data`/`.len`. Integration tests `test_vec_string_mangle`, `test_vec_get_multi_option`, `test_string_push_str_owned`, `test_vec_get_putback`. ION_SPEC documents `Vec::get` move-out and put-back scan. **`Vec::get_ref`**: stack-local `Option<&T>` for read-only vector peek; shared borrow on the vector owner; match arms bind `&T` as a pointer (no by-value copy of struct elements with nested owned fields); IR records match pattern bindings for nested scans; integration tests `test_vec_get_ref_*` including `test_vec_get_ref_scan_nested_vec`. `String::len` codegen: null-check the `String` pointer, not `&local` (fixes GCC `-Waddress` under CI `-Werror`).
1313
- **Examples**: `text_summary` (fixture file), `data_lib` (multi-module), `channel_worker` (flat single-file), `todo_demo` (interactive stdin, `Vec` of structs with `String`). Per-example `*.toml` manifests for `ion-build`. `http_server` accepts clients until stdin `quit`.
1414
- **Multi-file fixes**: merge private struct types for cross-module type checking; per-module C symbol prefixes in multi-file codegen (`io_print_int`); walk-up `runtime/` discovery for nested build directories. Integration tests `test_multi_struct`, `test_multi_fmt_io`. Example policy: each demo under `examples/<name>/` with `ion.toml`; build output under `target/` only (no committed `.c`).
1515

ION_SPEC.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1021,7 +1021,7 @@ Note that:
10211021
- `Vec<T>` is `Send` if `T: Send`.
10221022
- `Vec::new()` and `Vec::with_capacity()` infer `T` from a `let` type annotation when present (e.g. `let v: Vec<i32> = Vec::new()`).
10231023
- `Vec::get()` and `Vec::pop()` return `Option<T>` to handle out-of-bounds or empty cases. Both **move** the element out of the vector. To preserve vector length after a read-only scan, either use `Vec::get_ref()` (below) or copy fields and `Vec::set()` a rebuilt struct literal to put the value back (nested `Vec` fields still move on put-back).
1024-
- `Vec::get_ref()` returns `Option<&T>`: a **local, stack-only borrow** of an in-place element. It does not move or hollow the slot. The result is only valid as a short-lived binding within the current function (for example in a `match` arm). It cannot be returned, stored in structs or enums, sent on channels, or cross `spawn`. While an `&T` from `get_ref` is active, the root owner of the vector (the binding behind `&Vec<T>`) is shared-borrowed: `&mut Vec<T>` on that owner, `Vec::set`, `Vec::push`, and `Vec::pop` on the same vector are rejected until the borrow ends. Out-of-range or negative indices yield `Option::None`. Nested inspection (`order.lines` then `get_ref`) follows the same root-owner borrow rules as field paths (Section 5.3).
1024+
- `Vec::get_ref()` returns `Option<&T>`: a **local, stack-only borrow** of an in-place element. It does not move or hollow the slot. The result is only valid as a short-lived binding within the current function (for example in a `match` arm). Match arms bind the element as `&T`; codegen uses `T*` for types with owned fields and copies by value for copy types, so repeated scans over `Vec<struct-with-nested-Vec>` do not double-free nested fields. It cannot be returned, stored in structs or enums, sent on channels, or cross `spawn`. While an `&T` from `get_ref` is active, the root owner of the vector (the binding behind `&Vec<T>`) is shared-borrowed: `&mut Vec<T>` on that owner, `Vec::set`, `Vec::push`, and `Vec::pop` on the same vector are rejected until the borrow ends. Out-of-range or negative indices yield `Option::None`. Nested inspection (`order.lines` then `get_ref`) follows the same root-owner borrow rules as field paths (Section 5.3).
10251025
- `Vec::set()` requires a mutable reference and returns an error code (0 for success, non-zero for failure). After shared borrows from `get_ref` end, `Vec::set` on the same index is allowed.
10261026

10271027
For cross-function or long-lived access, Ion still favors an **index/handle style**: helpers return indices or keys and callers re-index within their own function bodies.

docs/ABI.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,9 @@ Stable beta expectations:
5151
- `Vec::get_ref` returns a **stack-local** `Option<&T>`: codegen fills tag and a
5252
pointer into the vector buffer (or `None` when out of bounds). No runtime heap
5353
`Option` and no `ion_option_from_raw`. Monomorphized names use the `ref_`
54-
prefix (for example `Option_ref_int`, `Option_ref_Product`). `match` on
55-
`Option::Some` binds by dereferencing the payload (`*arg0`) for use in the arm.
54+
prefix (for example `Option_ref_int`, `Option_ref_Product`). Match arms on
55+
`Option::Some(x)` bind drop-owning `T` as `T*`; copy types bind by value from
56+
`*arg0`. Scope cleanup must not drop nested owned fields through the binding.
5657
- Monomorphized container typedefs use Ion type names (`Vec_String`,
5758
`Option_Customer`), not C runtime typedefs (`ion_string_t`, etc.).
5859
- `&mut Vec<T>` parameters codegen as `Vec_T**`; builtins dereference once when

src/cgen/builtins.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,18 @@ impl Codegen {
237237

238238
// Vec::get_ref<T>(vec: &Vec<T>, index: int) -> Option<&T> (stack-local, no move-out)
239239
if callee == "Vec::get_ref" && args.len() == 2 {
240-
let option_name = return_type
240+
let effective_return_type = return_type.cloned().or_else(|| {
241+
self.vec_elem_type_from_arg(&args[0])
242+
.map(|elem| Type::Generic {
243+
name: "Option".to_string(),
244+
params: vec![Type::Ref {
245+
inner: Box::new(elem),
246+
mutable: false,
247+
}],
248+
})
249+
});
250+
let option_name = effective_return_type
251+
.as_ref()
241252
.map(|t| {
242253
mangle_type_name(
243254
"Option",
@@ -248,7 +259,8 @@ impl Codegen {
248259
)
249260
})
250261
.unwrap_or_else(|| "Option_int_".to_string());
251-
let ref_c_type = return_type
262+
let ref_c_type = effective_return_type
263+
.as_ref()
252264
.and_then(|t| {
253265
if let Type::Generic { params, .. } = t
254266
&& params.len() == 1
@@ -276,7 +288,7 @@ impl Codegen {
276288

277289
let elem_c_type = self.resolve_vec_elem_c_type(
278290
&args[0],
279-
return_type.and_then(|t| {
291+
effective_return_type.as_ref().and_then(|t| {
280292
if let Type::Generic { params, .. } = t
281293
&& params.len() == 1
282294
&& let Type::Ref { inner, .. } = &params[0]

0 commit comments

Comments
 (0)