From c89416dc69a37775426043c014d3333370d4104a Mon Sep 17 00:00:00 2001 From: Cod-e-Codes Date: Thu, 13 Aug 2026 20:25:52 -0400 Subject: [PATCH 1/2] Drop Vec elements on scope exit and infer Option::None from call arguments. Owned Vec and Box values leaked nested heap on drop, and unannotated Option::None could not use a function parameter as expected type. --- .../references/bug-hotspots.md | 3 +- .../references/language-constraints.md | 2 +- .../references/verified-patterns.md | 2 +- .github/workflows/ci.yml | 2 + CHANGELOG.md | 7 + Cargo.lock | 2 +- Cargo.toml | 2 +- ION_SPEC.md | 10 +- docs/ABI.md | 6 +- src/cgen/builtins.rs | 111 +++++++++- src/cgen/drop.rs | 197 +++++++++++++++++- src/cgen/mod.rs | 8 +- src/tc/builtins.rs | 32 ++- src/tc/mod.rs | 24 ++- tests/README.md | 8 +- tests/test_box_string_scope_drop.ion | 6 + tests/test_expectations.tsv | 12 ++ tests/test_option_none_call_arg.ion | 20 ++ tests/test_option_none_call_arg_middle.ion | 20 ++ tests/test_result_err_call_arg.ion | 17 ++ tests/test_vec_string_scope_drop.ion | 7 + tests/test_vec_struct_string_scope_drop.ion | 12 ++ 22 files changed, 471 insertions(+), 39 deletions(-) create mode 100644 tests/test_box_string_scope_drop.ion create mode 100644 tests/test_option_none_call_arg.ion create mode 100644 tests/test_option_none_call_arg_middle.ion create mode 100644 tests/test_result_err_call_arg.ion create mode 100644 tests/test_vec_string_scope_drop.ion create mode 100644 tests/test_vec_struct_string_scope_drop.ion diff --git a/.cursor/skills/finding-ion-bugs/references/bug-hotspots.md b/.cursor/skills/finding-ion-bugs/references/bug-hotspots.md index 8b8cc49..c974bbd 100644 --- a/.cursor/skills/finding-ion-bugs/references/bug-hotspots.md +++ b/.cursor/skills/finding-ion-bugs/references/bug-hotspots.md @@ -6,7 +6,7 @@ - **Reference escape**: `&` stored in struct, returned, sent on channel, captured by `spawn`. Locals and params must also reject off-stack stores (`Box<&T>`, `Vec<&T>`); `is_reference_containing` on decls/returns is not enough (`test_box_ref_let_error.ion`). `Option<&T>` stack temporaries from `get_ref` stay legal. - **Send**: non-Send types on channels or in spawn closures; `Box` and channel element variance - **Recursive types**: `is_reference_containing` / `is_send` / `is_eq_type` / `type_needs_drop` need a visiting set; without one, `Box`/`Vec`/`Option>` self-reference stack-overflows at decl time. Representability (`InfiniteSize`) treats only `Box`/`Vec`/`RawPtr` as size boundaries — `Option` is still infinite size; `Option>` is not. Do not “stop at Box” inside the no-escape walker or `Box<&T>` silently passes. -- **Generic enum `None`**: `Option::None` has no payload, so `T` is inferred only from `expr_expected` / return type. Unannotated `let empty = Option::None` must error with cannot-infer, not a later `Option` vs `Option>` mismatch (`test_option_none_unannotated_error.ion`). Same-expression `Node { next: Option::None }` is fine (`test_option_none_struct_field.ion`). +- **Generic enum `None`**: `Option::None` has no payload, so `T` is inferred from `expr_expected` / return type (let annotation, struct field, call argument, or return). Unannotated `let empty = Option::None` must error with cannot-infer, not a later `Option` vs `Option>` mismatch (`test_option_none_unannotated_error.ion`). Same-expression `Node { next: Option::None }` is fine (`test_option_none_struct_field.ion`). Direct call arguments `take(Option::None)` are fine (`test_option_none_call_arg.ion`). - **Match-arm result types**: `infer_block_result_type` reads recorded `TypeInfo` expr types plus control-flow shape (diverge vs value). It must not call `check_expr` again after `check_stmt` (`test_vec_get_putback_named.ion`). - **Match on `&GenericEnum`**: peel `Ref` before building the type-param subst map in `add_pattern_bindings` (see `test_match_ref_generic_enum_arith.ion`); bare `if let Type::Generic` misses `Ref { Generic { … } }` and leaves bindings as `&T` - **`resolve_type_name` and `&Enum` params**: must recurse into `Ref` so `&Flag` becomes `Ref { Enum }` (parser stores enum names as `Struct`); otherwise calls get `expected &Flag, got &Flag` from Struct vs Enum mismatch @@ -29,6 +29,7 @@ CLI errors use `TypeCheckError` Debug form (`UseAfterMove { ... }`). LSP reforma - **IR must use `TypeInfo`**: after a successful type-check, every lowered expression id has a canonical type. Missing id is a compiler bug, never a syntactic `int` fallback. One-off patches for this class shipped in 0.1.10 / 0.1.13 / 0.1.16; 0.1.17 makes the checker the source of truth (`let x = y`, `Box::new(StructLit)`, `Box::unwrap`, `Enum::Variant`). Tests: `test_unannotated_let_non_int.ion`, `test_box_new_struct_let*.ion`, `test_box_unwrap_struct_let.ion`, `test_enum_unannotated_let.ion`. - **Generic match subst**: `substitute_types_in_expr` must substitute `Match.scrutinee_type` (not only the inner expr / `enum_type`). Leaving `Slot` emits `Slot_T` (`examples/handle_table`). - **`Box::unwrap`**: copy `T` out, then `ion_box_free` the box pointer; do not drop `T` (nested heap in the payload would double-free). Move-mark the argument so scope-exit drop does not free again (`test_box_unwrap_same_scope.ion`). A `run` exit code cannot catch the leak; Linux CI leak-sanitizer step uses `detect_leaks=1` on unwrap tests. +- **`Vec` element drop**: scope-exit drop must iterate remaining elements when `T` needs destruction, then `ion_vec_free`. `Vec::get` of such `T` hollows the slot; `Vec::set` drops the previous element. Copy `T` stays memcpy-only (`test_vec_string_scope_drop.ion`, `test_vec_get_set.ion`). Do not drop nested owned fields through a `get_ref` binding. - **`String::len`**: null-check the `String*` value, not `&local` (`-Waddress` under CI `-Werror`) - **`len` method routing**: `"len"` is a Vec, String, and Slice method. IR/cgen must classify the receiver first (`receiver_is_slice` / `receiver_is_string`) before the `vec_methods` table, or `s.len()` on `&[]T` becomes `Vec::len` (same class as the v0.1.1 `String::len` mis-route). `vec.len()` must still become `Vec::len` (`test_slice_len.ion`, `test_method_call_basic.ion`). - **String literal call args**: parameters typed `String` need `ion_string_from_literal` at the call site, not only on `let s: String = "…"`. Pass compilation-wide `TypeInfo.function_params` into multi-file cgen (`println`, `io::println`, `io_println`) so imported callees see `String` (`test_string_call_arg_literal.ion`, `test_multi_fmt_io.ion`). diff --git a/.cursor/skills/ion-lang/references/language-constraints.md b/.cursor/skills/ion-lang/references/language-constraints.md index a1d2ae5..3c60ad5 100644 --- a/.cursor/skills/ion-lang/references/language-constraints.md +++ b/.cursor/skills/ion-lang/references/language-constraints.md @@ -35,7 +35,7 @@ APIs that would return `&T` in Rust must use owned values, indices, or the patte - Stack by default; `Box` for explicit heap - `defer` for deterministic cleanup at scope exit -- `Vec`, `String` drop at scope end (runtime-assisted) +- `Vec`, `String` drop at scope end (elements of a dropping `T`, then the backing array) ## Unsafe boundaries diff --git a/.cursor/skills/writing-ion-code/references/verified-patterns.md b/.cursor/skills/writing-ion-code/references/verified-patterns.md index 9d26cff..5d6956b 100644 --- a/.cursor/skills/writing-ion-code/references/verified-patterns.md +++ b/.cursor/skills/writing-ion-code/references/verified-patterns.md @@ -22,7 +22,7 @@ let p: Point = Point { x: 1, y: 2 }; ## Enum variants -Tuple: `Option::Some(42)`, `Option::None`. +Tuple: `Option::Some(42)`, `Option::None`. `take(Option::None)` infers `T` from the parameter type ([tests/test_option_none_call_arg.ion](../../../../tests/test_option_none_call_arg.ion)); unannotated `let empty = Option::None` still needs an annotation. Struct: `Status::Ok { value: 10 }`. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a22322c..41adbe5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -97,6 +97,8 @@ jobs: test_box_new_struct_let 10 test_box_new_struct_let_annotated 10 test_box_unwrap_same_scope 3 + test_box_string_scope_drop 0 + test_vec_string_scope_drop 0 EOF - name: Generated C thread sanitizer (Linux) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b41df1..0da8d9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 0.1.19 - 2026-08-13 + +- **Codegen**: `Vec` scope-exit drop now drops remaining elements when `T` needs destruction, then `ion_vec_free`. This impacts any `Vec`, `Vec>`, or `Vec` of structs/enums with owned fields (previously the backing array was freed and elements leaked). `Vec::get` of such `T` hollows the slot (already specified as move-out). `Vec::set` drops the previous element. `Vec` and other Copy elements are unchanged. `Box` drops `T` before `ion_box_free` when `T` needs destruction; `Box::unwrap` still does not drop `T`. +- **Type checker**: unannotated `Option::None` / other no-payload generic variants infer `T` from a call argument's parameter type (any position), matching return and struct-field positions. Unannotated `let empty = Option::None` still requires an annotation. This impacts passing `Option::None` or `Result::Err(...)` directly into a function. +- **Tests**: `test_vec_string_scope_drop.ion`, `test_vec_struct_string_scope_drop.ion`, `test_box_string_scope_drop.ion`, `test_option_none_call_arg.ion`, `test_option_none_call_arg_middle.ion`, `test_result_err_call_arg.ion`. Linux CI leak-sanitizer covers the Vec/Box string scope-drop tests. +- **Docs**: ION_SPEC §4.4 / §5.5 / §8.2, ABI Vec/Box drop, bug hotspots, verified patterns. + ## 0.1.18 - 2026-08-13 - **Type checker**: `Box<&T>` / `Vec<&T>` (and arrays/channels of references) are `ReferenceEscape` as locals and parameters, not only as struct fields. This impacts any program that boxed or vectored a reference (`let b: Box<&int> = Box::new(&x)` previously compiled). Stack-local `Option<&T>` from `Vec::get_ref` is unchanged. diff --git a/Cargo.lock b/Cargo.lock index da9423a..9c59bef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -323,7 +323,7 @@ dependencies = [ [[package]] name = "ion-compiler" -version = "0.1.18" +version = "0.1.19" dependencies = [ "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 11a122d..dd80f44 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ion-compiler" -version = "0.1.18" +version = "0.1.19" edition = "2024" [[bin]] diff --git a/ION_SPEC.md b/ION_SPEC.md index d6c775e..af3ef7d 100644 --- a/ION_SPEC.md +++ b/ION_SPEC.md @@ -671,7 +671,7 @@ Ion supports a **local, Hindley–Milner-inspired inference**: The inference engine is intentionally limited: - No higher-rank polymorphism. -- Generic enum variants with no payload (`Option::None`) infer type arguments only from an adjacent expected type (a `let` annotation, a struct field, or a return type). They do not take `T` from a later statement; without that context the compiler requires an annotation. +- Generic enum variants with no payload (`Option::None`) infer type arguments only from an adjacent expected type (a `let` annotation, a struct field, a function parameter / call argument, or a return type). They do not take `T` from a later statement; without that context the compiler requires an annotation. - Generic type parameters may declare optional **trait bounds** (`Copy`, `Eq`, `Send`). Bounds are checked at monomorphization: each concrete instantiation must satisfy every bound on the corresponding parameter. There are no user-defined traits; bounds name structural capabilities checked by the compiler (see Section 4.8). - Structural `Send` still applies per instantiation even without an explicit bound: for a generic type `Wrapper`, each monomorphized `Wrapper` is `Send` if and only if all of its fields (with `T` replaced by `U`) are `Send`. @@ -896,8 +896,8 @@ fn process() { When a binding goes out of scope (block exit, function return), owned values with heap resources are dropped automatically: -- `Box`: `ion_box_free` -- `Vec`: `ion_vec_free` +- `Box`: drop `T` (when it needs destruction), then `ion_box_free` +- `Vec`: drop remaining elements (when `T` needs destruction), then `ion_vec_free` - `String`: `ion_string_free` - `Sender` / `Receiver`: `ion_channel_handle_drop` (refcounted; freed when both ends are dropped) - Struct fields with owned heap types (`Box`, `Vec`, `String`, channels, or nested structs/enums containing them) are dropped in declaration order when the struct goes out of scope. @@ -1078,9 +1078,9 @@ Note that: - `Vec` is `Send` if `T: Send`. - `Vec::new()` and `Vec::with_capacity()` infer `T` from a `let` type annotation when present (e.g. `let v: Vec = Vec::new()`). -- `Vec::get()` and `Vec::pop()` return `Option` 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). +- `Vec::get()` and `Vec::pop()` return `Option` to handle out-of-bounds or empty cases. Both **move** the element out of the vector. For a `T` that needs destruction, `Vec::get` hollows the slot (zero-fills it) so later vector drop does not free the moved value again. Copy `T` is left in place (move and copy are indistinguishable). 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). - `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`; for enum elements, an inner `match` on that binding dispatches variants directly (no unary `*` deref). Copy fields in struct or enum variant patterns bind as `T`; non-copy fields bind as `&T`. Codegen uses `T*` for types with owned fields and copies by value for copy types, so repeated scans over `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`) is shared-borrowed: `&mut Vec` 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). Field paths through `&Struct` that are already references (for example `order.lines` when `order: &Order`) are passed to `Vec` methods without an extra `&`. -- `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. +- `Vec::set()` requires a mutable reference and returns an error code (0 for success, non-zero for failure). When `T` needs destruction, the previous element at that index is dropped before the new value is written. After shared borrows from `get_ref` end, `Vec::set` on the same index is allowed. 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. When slots in a growable table can be reused, prefer `Handle` / `Arena` in Section 8.6 over a raw `int` index. diff --git a/docs/ABI.md b/docs/ABI.md index 9fccb1b..1d24074 100644 --- a/docs/ABI.md +++ b/docs/ABI.md @@ -44,7 +44,8 @@ Stable beta expectations: - `Vec::new`, `Vec::push`, `Vec::pop`, `Vec::get`, `Vec::get_ref`, `Vec::set`, `Vec::len`, and `Vec::capacity` remain available through the stdlib/builtin surface. -- Dropping `Vec` drops owned elements when `T` needs destruction. +- Dropping `Vec` drops owned elements when `T` needs destruction (compiler drop glue over `0..len`, then `ion_vec_free`). The runtime helper does not take a destructor callback. +- `Vec::get` of a dropping `T` copies the element into `Option` and hollows the slot. `Vec::set` of a dropping `T` drops the previous element before overwrite. - Bounds-sensitive operations either return `Option` where documented or trigger the runtime panic path for checked indexing. - `Vec::get` / `Vec::pop` return heap `Option` blobs from the runtime; generated @@ -79,7 +80,8 @@ Stable beta expectations: - `Box::new` allocates and owns a value. - `Box::unwrap` consumes the box, copies the payload out by value, and `ion_box_free`s the allocation. It does not drop `T`; the caller owns the copy. -- Dropping a `Box` drops the payload and releases the allocation once. +- Dropping a `Box` drops the payload when `T` needs destruction, then + releases the allocation once. ## Arrays and slices diff --git a/src/cgen/builtins.rs b/src/cgen/builtins.rs index 0d9b19d..f27232c 100644 --- a/src/cgen/builtins.rs +++ b/src/cgen/builtins.rs @@ -236,13 +236,58 @@ impl Codegen { let elem_c_type = self.resolve_vec_elem_c_type(&args[0], return_type); let deref_vec = self.vec_ion_ptr_expr(&args[0], &vec_code); - code.push_str("ion_vec_get((ion_vec_t*)("); - code.push_str(&deref_vec); - code.push_str("), "); - code.push_str(&index_code); - code.push_str(", sizeof("); - code.push_str(&elem_c_type); - code.push_str("))"); + let elem_ty = self.vec_elem_type_from_arg(&args[0]); + let hollow = elem_ty.as_ref().is_some_and(|t| self.type_needs_drop(t)); + if hollow { + let n = self.temp_var_counter; + self.temp_var_counter += 1; + let gv = format!("_ion_gv{n}"); + let gi = format!("_ion_gi{n}"); + let gr = format!("_ion_gr{n}"); + code.push_str("({ ion_vec_t* "); + code.push_str(&gv); + code.push_str(" = (ion_vec_t*)("); + code.push_str(&deref_vec); + code.push_str("); int "); + code.push_str(&gi); + code.push_str(" = "); + code.push_str(&index_code); + code.push_str("; void* "); + code.push_str(&gr); + code.push_str(" = ion_vec_get("); + code.push_str(&gv); + code.push_str(", "); + code.push_str(&gi); + code.push_str(", sizeof("); + code.push_str(&elem_c_type); + code.push_str(")); if ("); + code.push_str(&gr); + code.push_str(" && *(int*)"); + code.push_str(&gr); + code.push_str(" == 0 && "); + code.push_str(&gv); + code.push_str(" && "); + code.push_str(&gv); + code.push_str("->data) { memset((char*)"); + code.push_str(&gv); + code.push_str("->data + (size_t)"); + code.push_str(&gi); + code.push_str(" * sizeof("); + code.push_str(&elem_c_type); + code.push_str("), 0, sizeof("); + code.push_str(&elem_c_type); + code.push_str(")); } "); + code.push_str(&gr); + code.push_str("; })"); + } else { + code.push_str("ion_vec_get((ion_vec_t*)("); + code.push_str(&deref_vec); + code.push_str("), "); + code.push_str(&index_code); + code.push_str(", sizeof("); + code.push_str(&elem_c_type); + code.push_str("))"); + } return Some(code); } @@ -348,6 +393,7 @@ impl Codegen { value_code = std::mem::replace(&mut self.output, old_output); let elem_c_type = self.resolve_vec_elem_c_type(&args[0], return_type); + let drop_old = elem_ty.as_ref().is_some_and(|t| self.type_needs_drop(t)); let value_is_lvalue = matches!( args[2], @@ -356,7 +402,56 @@ impl Codegen { | IREexpr::Var(_) | IREexpr::FieldAccess { .. } ); - if value_is_lvalue { + if drop_old { + let n = self.temp_var_counter; + self.temp_var_counter += 1; + let sv = format!("_ion_sv{n}"); + let si = format!("_ion_si{n}"); + let slot = format!("(({elem_c_type}*)(({sv})->data))[{si}]"); + let drop_old_stmt = elem_ty + .as_ref() + .map(|t| self.capture_drop_at_path(&slot, t)) + .unwrap_or_default(); + code.push_str("({ ion_vec_t* "); + code.push_str(&sv); + code.push_str(" = (ion_vec_t*)("); + code.push_str(&deref_vec); + code.push_str("); int "); + code.push_str(&si); + code.push_str(" = "); + code.push_str(&index_code); + code.push_str("; "); + let value_ptr = if value_is_lvalue { + format!("&{value_code}") + } else if matches!(args[2], IREexpr::Call { .. }) { + code.push_str(&elem_c_type); + code.push_str(" _ion_set_val = "); + code.push_str(&value_code); + code.push_str("; "); + "&_ion_set_val".to_string() + } else { + format!("&(({elem_c_type}){{{value_code}}})") + }; + code.push_str("if ("); + code.push_str(&sv); + code.push_str(" && "); + code.push_str(&si); + code.push_str(" >= 0 && (size_t)"); + code.push_str(&si); + code.push_str(" < "); + code.push_str(&sv); + code.push_str("->len) { "); + code.push_str(&drop_old_stmt); + code.push_str(" } ion_vec_set("); + code.push_str(&sv); + code.push_str(", "); + code.push_str(&si); + code.push_str(", "); + code.push_str(&value_ptr); + code.push_str(", sizeof("); + code.push_str(&elem_c_type); + code.push_str(")); })"); + } else if value_is_lvalue { code.push_str("ion_vec_set((ion_vec_t*)("); code.push_str(&deref_vec); code.push_str("), "); diff --git a/src/cgen/drop.rs b/src/cgen/drop.rs index 895bb26..a4d69e1 100644 --- a/src/cgen/drop.rs +++ b/src/cgen/drop.rs @@ -70,7 +70,151 @@ impl Codegen { self.emit_drop_at_path(name, ty); } + /// Capture drop statements for embedding in a GNU statement expression. + pub(crate) fn capture_drop_at_path(&mut self, path: &str, ty: &Type) -> String { + let mut captured = String::new(); + let old_output = std::mem::replace(&mut self.output, captured); + let old_indent = self.indent_level; + self.indent_level = 0; + self.emit_drop_at_path(path, ty); + captured = std::mem::replace(&mut self.output, old_output); + self.indent_level = old_indent; + captured + } + + fn fresh_temp(&mut self, prefix: &str) -> String { + let n = self.temp_var_counter; + self.temp_var_counter += 1; + format!("{prefix}{n}") + } + + fn drop_function_name(&self, ty: &Type) -> String { + let c = self.type_to_c(ty); + let sanitized: String = c + .chars() + .map(|ch| match ch { + 'a'..='z' | 'A'..='Z' | '0'..='9' | '_' => ch, + _ => '_', + }) + .collect(); + format!("_ion_drop_{sanitized}") + } + + fn is_drop_adt(&self, ty: &Type) -> bool { + self.struct_decl_for_type(ty).is_some() || self.enum_decl_for_type(ty).is_some() + } + + /// True when inlined drop glue for this ADT would recurse in the compiler + /// (for example `Node` containing `Option>`). + fn adt_needs_named_drop(&self, ty: &Type) -> bool { + if !self.is_drop_adt(ty) || !self.type_needs_drop(ty) { + return false; + } + self.adt_drop_reaches_self(ty, &mut HashSet::new()) + } + + fn adt_drop_reaches_self(&self, ty: &Type, visiting: &mut HashSet) -> bool { + let resolved = resolve_type_alias(ty, &self.type_aliases); + if let Type::Box { inner } = &resolved { + return self.adt_drop_reaches_self(inner, visiting); + } + if let Type::Vec { elem_type } = &resolved { + return self.adt_drop_reaches_self(elem_type, visiting); + } + if self.is_drop_adt(&resolved) { + let key = self.drop_function_name(&resolved); + if !visiting.insert(key.clone()) { + return true; + } + let cyclic = if let Some((decl, substitutions)) = self.struct_decl_for_type(&resolved) { + decl.fields.iter().any(|field| { + let field_ty = Self::substitute_field_types(&field.ty, &substitutions); + self.adt_drop_reaches_self(&field_ty, visiting) + }) + } else if let Some((decl, substitutions)) = self.enum_decl_for_type(&resolved) { + decl.variants.iter().any(|variant| { + if let Some(named_fields) = &variant.named_fields { + named_fields.iter().any(|(_, field_ty)| { + let ft = Self::substitute_field_types(field_ty, &substitutions); + self.adt_drop_reaches_self(&ft, visiting) + }) + } else { + variant.payload_types.iter().any(|payload_ty| { + let ft = Self::substitute_field_types(payload_ty, &substitutions); + self.adt_drop_reaches_self(&ft, visiting) + }) + } + }) + } else { + false + }; + visiting.remove(&key); + return cyclic; + } + false + } + + fn collect_named_drop_types(&self) -> Vec { + let mut seen = HashSet::new(); + let mut types = Vec::new(); + let mut consider = |ty: Type| { + if self.adt_needs_named_drop(&ty) { + let name = self.drop_function_name(&ty); + if seen.insert(name) { + types.push(ty); + } + } + }; + for name in self.struct_map.keys() { + consider(Type::Struct(name.clone())); + } + for name in self.enum_map.keys() { + consider(Type::Enum(name.clone())); + } + for (base, params) in self.generic_instantiations.values() { + consider(Type::Generic { + name: base.clone(), + params: params.clone(), + }); + } + types.sort_by_key(|a| self.drop_function_name(a)); + types + } + + pub(crate) fn emit_named_drop_functions(&mut self) { + let types = self.collect_named_drop_types(); + if types.is_empty() { + return; + } + for ty in &types { + let name = self.drop_function_name(ty); + let c_ty = self.type_to_c(ty); + self.writeln(&format!("static void {name}({c_ty} *p);")); + } + self.writeln(""); + for ty in &types { + let name = self.drop_function_name(ty); + let c_ty = self.type_to_c(ty); + self.writeln(&format!("static void {name}({c_ty} *p) {{")); + self.indent_level += 1; + self.emit_drop_adt_inline("(*p)", ty); + self.indent_level -= 1; + self.writeln("}"); + self.writeln(""); + } + } + pub(crate) fn emit_drop_at_path(&mut self, path: &str, ty: &Type) { + if self.is_drop_adt(ty) && self.adt_needs_named_drop(ty) { + let fn_name = self.drop_function_name(ty); + self.write_indent(); + self.writeln(&format!("{fn_name}(&({path}));")); + return; + } + self.emit_drop_adt_inline(path, ty); + } + + fn emit_drop_adt_inline(&mut self, path: &str, ty: &Type) { if let Some((decl, substitutions)) = self.struct_decl_for_type(ty) { let fields: Vec<(String, Type)> = decl .fields @@ -152,15 +296,52 @@ impl Codegen { let resolved = resolve_type_alias(ty, &self.type_aliases); match resolved { - Type::Box { .. } => { - self.write_indent(); - self.writeln(&format!("if ({path}) {{ ion_box_free({path}); }}")); + Type::Box { inner } => { + let inner = *inner; + if self.type_needs_drop(&inner) { + self.write_indent(); + self.writeln(&format!("if ({path}) {{")); + self.indent_level += 1; + self.emit_drop_at_path(&format!("(*({path}))"), &inner); + self.write_indent(); + self.writeln(&format!("ion_box_free({path});")); + self.indent_level -= 1; + self.write_indent(); + self.writeln("}"); + } else { + self.write_indent(); + self.writeln(&format!("if ({path}) {{ ion_box_free({path}); }}")); + } } - Type::Vec { .. } => { - self.write_indent(); - self.writeln(&format!( - "if ({path}) {{ ion_vec_free((ion_vec_t*)({path})); }}" - )); + Type::Vec { elem_type } => { + let elem_type = *elem_type; + if self.type_needs_drop(&elem_type) { + let idx = self.fresh_temp("_ion_di"); + let elem_c = self.type_to_c(&elem_type); + let slot = format!("(({elem_c}*)(({path})->data))[{idx}]"); + self.write_indent(); + self.writeln(&format!("if ({path}) {{")); + self.indent_level += 1; + self.write_indent(); + self.writeln(&format!( + "for (size_t {idx} = 0; {idx} < ({path})->len; {idx}++) {{" + )); + self.indent_level += 1; + self.emit_drop_at_path(&slot, &elem_type); + self.indent_level -= 1; + self.write_indent(); + self.writeln("}"); + self.write_indent(); + self.writeln(&format!("ion_vec_free((ion_vec_t*)({path}));")); + self.indent_level -= 1; + self.write_indent(); + self.writeln("}"); + } else { + self.write_indent(); + self.writeln(&format!( + "if ({path}) {{ ion_vec_free((ion_vec_t*)({path})); }}" + )); + } } Type::String => { self.write_indent(); diff --git a/src/cgen/mod.rs b/src/cgen/mod.rs index 7b7073d..e2b59ef 100644 --- a/src/cgen/mod.rs +++ b/src/cgen/mod.rs @@ -586,6 +586,8 @@ impl Codegen { } } + self.emit_named_drop_functions(); + // Generate extern function prototypes for extern_block in &program.extern_blocks { self.generate_extern_block(extern_block); @@ -913,6 +915,8 @@ impl Codegen { } } + self.emit_named_drop_functions(); + // Generate extern function prototypes (declarations only, implementations come from headers) for extern_block in &program.extern_blocks { for ext_fn in &extern_block.functions { @@ -3247,8 +3251,8 @@ impl Codegen { self.write(")"); continue; } - if matches!(param_ty, Some(Type::String)) { - self.generate_expr_with_type(arg, Some(&Type::String)); + if let Some(ref pty) = param_ty { + self.generate_expr_with_type(arg, Some(pty)); } else { self.generate_expr(arg); } diff --git a/src/tc/builtins.rs b/src/tc/builtins.rs index c366b68..e344944 100644 --- a/src/tc/builtins.rs +++ b/src/tc/builtins.rs @@ -137,7 +137,21 @@ impl TypeChecker { }); } let vec_ty = self.check_expr(&call_expr.args[0])?; - let value_ty = self.check_expr(&call_expr.args[1])?; + let expected_elem = match &vec_ty { + Type::Ref { + inner, + mutable: true, + } => match inner.as_ref() { + Type::Vec { elem_type } => Some(elem_type.as_ref().clone()), + _ => None, + }, + _ => None, + }; + let value_ty = if let Some(elem) = &expected_elem { + self.check_expr_with_expected(&call_expr.args[1], elem)? + } else { + self.check_expr(&call_expr.args[1])? + }; if let Type::Ref { inner: ref inner_ty, mutable: true, @@ -325,7 +339,21 @@ impl TypeChecker { } let vec_ty = self.check_expr(&call_expr.args[0])?; let index_ty = self.check_expr(&call_expr.args[1])?; - let value_ty = self.check_expr(&call_expr.args[2])?; + let expected_elem = match &vec_ty { + Type::Ref { + inner, + mutable: true, + } => match inner.as_ref() { + Type::Vec { elem_type } => Some(elem_type.as_ref().clone()), + _ => None, + }, + _ => None, + }; + let value_ty = if let Some(elem) = &expected_elem { + self.check_expr_with_expected(&call_expr.args[2], elem)? + } else { + self.check_expr(&call_expr.args[2])? + }; if !self.is_integer_type(&index_ty) { return Err(TypeCheckError::TypeMismatch { expected: "integer type".to_string(), diff --git a/src/tc/mod.rs b/src/tc/mod.rs index ad8c877..2193351 100644 --- a/src/tc/mod.rs +++ b/src/tc/mod.rs @@ -2390,6 +2390,17 @@ impl TypeChecker { self.check_expr(init) } + fn check_expr_with_expected( + &mut self, + expr: &Expr, + expected: &Type, + ) -> Result { + let prev = self.expr_expected.replace(expected.clone()); + let ty = self.check_expr(expr); + self.expr_expected = prev; + ty + } + /// `let x: Arena = new();` infers `T = int` from the annotation vs the /// generic function's return type (same idea as `Vec::new()`). fn infer_zero_arg_generic_call( @@ -3805,9 +3816,10 @@ impl TypeChecker { } for (arg_expr, param_ty) in call_expr.args.iter().zip(fn_params.iter()) { - let arg_ty = self.check_expr(arg_expr)?; - let resolved_arg_ty = self.resolve_type_name(&arg_ty)?; let resolved_param_ty = self.resolve_type_name(param_ty)?; + let arg_ty = + self.check_expr_with_expected(arg_expr, &resolved_param_ty)?; + let resolved_arg_ty = self.resolve_type_name(&arg_ty)?; let numeric_coerced = Self::can_coerce_numeric(&resolved_arg_ty, &resolved_param_ty); if !numeric_coerced @@ -3903,12 +3915,12 @@ impl TypeChecker { // Check argument types and mark as moved for (arg_expr, param) in call_expr.args.iter().zip(func_decl_params.iter()) { - let arg_ty = self.check_expr(arg_expr)?; - let resolved_arg_ty = self.resolve_type_name(&arg_ty)?; let resolved_param_ty = substitute_generic_types_impl( &self.resolve_type_name(¶m.ty)?, &generic_substitutions, ); + let arg_ty = self.check_expr_with_expected(arg_expr, &resolved_param_ty)?; + let resolved_arg_ty = self.resolve_type_name(&arg_ty)?; // Special case: allow string literals to be passed as *u8 for extern functions // This enables calling C functions like printf with string literals @@ -4184,9 +4196,9 @@ impl TypeChecker { // Check argument types for (arg_expr, param) in desugared_call.args.iter().zip(params.iter()) { - let arg_ty = self.check_expr(arg_expr)?; - let resolved_arg_ty = self.resolve_type_name(&arg_ty)?; let resolved_param_ty = self.resolve_type_name(¶m.ty)?; + let arg_ty = self.check_expr_with_expected(arg_expr, &resolved_param_ty)?; + let resolved_arg_ty = self.resolve_type_name(&arg_ty)?; // Check for numeric coercion and type equality let types_match = diff --git a/tests/README.md b/tests/README.md index 3d54c6f..12e9975 100644 --- a/tests/README.md +++ b/tests/README.md @@ -84,6 +84,9 @@ The test runner prints pass/fail counts when it finishes. Do not rely on hardcod - `test_defer_basic.ion` - Defer statements - `test_defer_block.ion` - Block-scoped defer - `test_scope_drop_block.ion` - Automatic Vec drop at block exit +- `test_vec_string_scope_drop.ion` - `Vec` scope-exit drop frees each `String` then the backing array (exit 0); cgen asserts element `ion_string_free` and `ion_vec_free`; Linux CI leak-sanitizer +- `test_vec_struct_string_scope_drop.ion` - struct field `Vec` drops elements then the array (exit 0) +- `test_box_string_scope_drop.ion` - `Box` drops the `String` then `ion_box_free` (exit 0); Linux CI leak-sanitizer - `test_struct_field_drop.ion` - Struct and enum field drops at block exit (nested String fields, enum payload) - `test_struct_field_drop_vec.ion` - Struct field holding `Vec` drop at block exit (exit 46) - `test_struct_field_drop_box.ion` - Box field drop at block exit (exit 44) @@ -112,6 +115,9 @@ The test runner prints pass/fail counts when it finishes. Do not rely on hardcod - `test_enum_generic_unannotated_let.ion` - unannotated `let x = Option::Some(42)` types as `Option` (exit 42); cgen asserts `Option_int x =` - `test_option_none_unannotated_error.ion` - `let empty = Option::None` then use as `Option>` needs a type annotation (`cannot infer type parameter`) - `test_option_none_struct_field.ion` - `Node { next: Option::None }` infers `T` from the field (exit 2) +- `test_option_none_call_arg.ion` - `take(Option::None)` infers `T` from the parameter (exit 3) +- `test_option_none_call_arg_middle.ion` - `take2(5, Option::None)` infers `T` in a non-final argument (exit 5) +- `test_result_err_call_arg.ion` - `take(Result::Err(7))` infers `T` from the parameter (exit 7) - `test_unannotated_let_non_int.ion` - unannotated `let q = p` / `let n = w.p` / `let v = origin()` keep struct types, not default int (exit 5); cgen asserts `Point q =` / `Point n =` / `Point v =` - `test_enum_generic.ion` - Generic enum types - `test_result_custom_enum.ion` - `Result` via `stdlib/result.ion` (Ok and Err, exit 0) @@ -412,7 +418,7 @@ Special cases (not in the manifest): - `COMPILER`: Path to the ion-compiler binary (default: `../target/release/ion-compiler`) - `ION_BUILD`: Path to the ion-build binary (default: `../target/release/ion-build`) - `CC`: C compiler to use (default: `gcc`) -- `CFLAGS`: Extra C compiler flags for generated C and the precompiled runtime (default: empty). CI uses `-fsanitize=address,undefined` for sanitizer smoke (`detect_leaks=0`) and a leak-sanitizer step (`detect_leaks=1`) on `Box::unwrap` tests, and runs the full harness with `-Wall -Wextra -Werror` on Linux. +- `CFLAGS`: Extra C compiler flags for generated C and the precompiled runtime (default: empty). CI uses `-fsanitize=address,undefined` for sanitizer smoke (`detect_leaks=0`) and a leak-sanitizer step (`detect_leaks=1`) on `Box::unwrap` tests plus `Vec` / `Box` scope-drop tests, and runs the full harness with `-Wall -Wextra -Werror` on Linux. - `LDFLAGS`: Extra C linker flags for generated test executables (default: empty). Pair with `CFLAGS` for sanitizer runtime flags when needed. - `RUNTIME_OBJ`: Path to the precompiled runtime object file (default: `.ion_test_runtime.o` in `tests/`). Rebuilt when `runtime/ion_runtime.c` is newer than the object. diff --git a/tests/test_box_string_scope_drop.ion b/tests/test_box_string_scope_drop.ion new file mode 100644 index 0000000..d9c54e8 --- /dev/null +++ b/tests/test_box_string_scope_drop.ion @@ -0,0 +1,6 @@ +// Box scope-exit drop must free the String, then the box allocation. +fn main() -> int { + let s: String = String::from("boxed"); + let b: Box = Box::new(s); + return 0; +} diff --git a/tests/test_expectations.tsv b/tests/test_expectations.tsv index c84e13e..05f172f 100644 --- a/tests/test_expectations.tsv +++ b/tests/test_expectations.tsv @@ -58,6 +58,18 @@ test_enum_generic_unannotated_let.ion run 42 test_enum_generic_unannotated_let.ion cgen Option_int x = ^[[:space:]]*int x = test_option_none_unannotated_error.ion error cannot infer type parameter test_option_none_struct_field.ion run 2 +test_option_none_call_arg.ion run 3 +test_option_none_call_arg.ion cgen take((Option_int) +test_option_none_call_arg_middle.ion run 5 +test_result_err_call_arg.ion run 7 +test_vec_string_scope_drop.ion run 0 +test_vec_string_scope_drop.ion cgen ion_string_free(((ion_string_t**)((v)->data)) +test_vec_string_scope_drop.ion cgen ion_vec_free((ion_vec_t*)(v)) +test_vec_struct_string_scope_drop.ion run 0 +test_vec_struct_string_scope_drop.ion cgen ion_vec_free((ion_vec_t*)(b.items)) +test_box_string_scope_drop.ion run 0 +test_box_string_scope_drop.ion cgen ion_string_free((*(b))) +test_box_string_scope_drop.ion cgen ion_box_free(b) test_unannotated_let_non_int.ion run 5 test_unannotated_let_non_int.ion cgen Point q = ^[[:space:]]*int q = test_unannotated_let_non_int.ion cgen Point n = ^[[:space:]]*int n = diff --git a/tests/test_option_none_call_arg.ion b/tests/test_option_none_call_arg.ion new file mode 100644 index 0000000..dc3f1f1 --- /dev/null +++ b/tests/test_option_none_call_arg.ion @@ -0,0 +1,20 @@ +// Option::None infers T from a function parameter's expected type. +enum Option { + Some(T); + None; +} + +fn take(o: Option) -> int { + match o { + Option::Some(v) => { + return v; + } + Option::None => { + return 3; + } + } +} + +fn main() -> int { + return take(Option::None); +} diff --git a/tests/test_option_none_call_arg_middle.ion b/tests/test_option_none_call_arg_middle.ion new file mode 100644 index 0000000..dcfa68f --- /dev/null +++ b/tests/test_option_none_call_arg_middle.ion @@ -0,0 +1,20 @@ +// Option::None infers T in a non-final call-argument position. +enum Option { + Some(T); + None; +} + +fn take2(x: int, o: Option) -> int { + match o { + Option::Some(v) => { + return x + v; + } + Option::None => { + return x; + } + } +} + +fn main() -> int { + return take2(5, Option::None); +} diff --git a/tests/test_result_err_call_arg.ion b/tests/test_result_err_call_arg.ion new file mode 100644 index 0000000..424ade6 --- /dev/null +++ b/tests/test_result_err_call_arg.ion @@ -0,0 +1,17 @@ +// Result::Err infers T from the parameter type; E comes from the payload. +import "stdlib/result.ion" as result; + +fn take(r: Result) -> int { + match r { + Result::Ok(v) => { + return v; + } + Result::Err(e) => { + return e; + } + } +} + +fn main() -> int { + return take(Result::Err(7)); +} diff --git a/tests/test_vec_string_scope_drop.ion b/tests/test_vec_string_scope_drop.ion new file mode 100644 index 0000000..13673e4 --- /dev/null +++ b/tests/test_vec_string_scope_drop.ion @@ -0,0 +1,7 @@ +// Vec scope-exit drop must free each element's heap buffer. +fn main() -> int { + let mut v: Vec = Vec::new(); + Vec::push(&mut v, String::from("one")); + Vec::push(&mut v, String::from("two")); + return 0; +} diff --git a/tests/test_vec_struct_string_scope_drop.ion b/tests/test_vec_struct_string_scope_drop.ion new file mode 100644 index 0000000..d71b63b --- /dev/null +++ b/tests/test_vec_struct_string_scope_drop.ion @@ -0,0 +1,12 @@ +// Struct field Vec drop must free elements, then the backing array. +struct Bag { + items: Vec; +} + +fn main() -> int { + let mut b: Bag = Bag { + items: Vec::new(), + }; + Vec::push(&mut b.items, String::from("leaked?")); + return 0; +} From f8912030ee2164160f7ab29f9d3133f31c297086 Mon Sep 17 00:00:00 2001 From: Cod-e-Codes Date: Thu, 13 Aug 2026 20:39:01 -0400 Subject: [PATCH 2/2] Only emit named drop helpers for types on their own drop cycle. Option wrapping a cyclic Vec was getting an unused static helper, which Linux gcc -Werror rejected. --- src/cgen/drop.rs | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/cgen/drop.rs b/src/cgen/drop.rs index a4d69e1..693dcd4 100644 --- a/src/cgen/drop.rs +++ b/src/cgen/drop.rs @@ -106,42 +106,56 @@ impl Codegen { /// True when inlined drop glue for this ADT would recurse in the compiler /// (for example `Node` containing `Option>`). + /// + /// Only the types that appear on their own drop cycle get a helper. + /// `Option` wrapping a cyclic `Vec` inlines and calls + /// `_ion_drop_Forest`; emitting `_ion_drop_Option_Forest` would be unused. fn adt_needs_named_drop(&self, ty: &Type) -> bool { if !self.is_drop_adt(ty) || !self.type_needs_drop(ty) { return false; } - self.adt_drop_reaches_self(ty, &mut HashSet::new()) + let start = self.drop_function_name(ty); + self.adt_drop_reaches_start(ty, &start, &mut HashSet::new(), true) } - fn adt_drop_reaches_self(&self, ty: &Type, visiting: &mut HashSet) -> bool { + fn adt_drop_reaches_start( + &self, + ty: &Type, + start: &str, + visiting: &mut HashSet, + is_root: bool, + ) -> bool { let resolved = resolve_type_alias(ty, &self.type_aliases); if let Type::Box { inner } = &resolved { - return self.adt_drop_reaches_self(inner, visiting); + return self.adt_drop_reaches_start(inner, start, visiting, is_root); } if let Type::Vec { elem_type } = &resolved { - return self.adt_drop_reaches_self(elem_type, visiting); + return self.adt_drop_reaches_start(elem_type, start, visiting, is_root); } if self.is_drop_adt(&resolved) { let key = self.drop_function_name(&resolved); - if !visiting.insert(key.clone()) { + if !is_root && key == start { return true; } + if !visiting.insert(key.clone()) { + return false; + } let cyclic = if let Some((decl, substitutions)) = self.struct_decl_for_type(&resolved) { decl.fields.iter().any(|field| { let field_ty = Self::substitute_field_types(&field.ty, &substitutions); - self.adt_drop_reaches_self(&field_ty, visiting) + self.adt_drop_reaches_start(&field_ty, start, visiting, false) }) } else if let Some((decl, substitutions)) = self.enum_decl_for_type(&resolved) { decl.variants.iter().any(|variant| { if let Some(named_fields) = &variant.named_fields { named_fields.iter().any(|(_, field_ty)| { let ft = Self::substitute_field_types(field_ty, &substitutions); - self.adt_drop_reaches_self(&ft, visiting) + self.adt_drop_reaches_start(&ft, start, visiting, false) }) } else { variant.payload_types.iter().any(|payload_ty| { let ft = Self::substitute_field_types(payload_ty, &substitutions); - self.adt_drop_reaches_self(&ft, visiting) + self.adt_drop_reaches_start(&ft, start, visiting, false) }) } })