Skip to content

Commit 3697969

Browse files
committed
Fix String::len codegen so AddressOf args null-check the String pointer, not &local (GCC -Waddress under CI -Werror).
1 parent 76560d5 commit 3697969

3 files changed

Lines changed: 8 additions & 4 deletions

File tree

.cursor/skills/finding-ion-bugs/references/bug-hotspots.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ CLI errors use `TypeCheckError` Debug form (`UseAfterMove { ... }`). LSP reforma
2020
- **Match scrutinee move-out**: pattern payload bindings null `match_val_N.data.variant_*` fields when ownership transfers (`statement_match_payload_move_neutralizes_scrutinee`); whole-enum binding arms clear active variant payloads via `emit_match_scrutinee_whole_enum_moved_out` (`whole_enum_binding_neutralizes_scrutinee_payloads`). IR infers `enum_type` from the scrutinee when arms use binding/wildcard only (`infer_match_enum_name`).
2121
- **Return unwind**: all function exits use `emit_function_return` (`ret_val`, `scope_emit_return_unwind`, `goto epilogue`), including diverging `return` inside rvalue `match` arms (`rvalue_match_divergent_return_unwinds_owned`). Value-producing rvalue arms still assign and `break` from the `switch`.
2222
- `Box`, `Vec`, `String` layout vs `runtime/ion_runtime.h`
23+
- **`String::len`**: null-check the `String*` value, not `&local` (`-Waddress` under CI `-Werror`)
2324
- Single-file merge (`merge_modules`) vs `--mode multi` divergences
2425
- `extern "C"` calls only inside `unsafe` blocks in source; cgen must not strip guards
2526

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_*`.
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`).
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

src/cgen/builtins.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -402,9 +402,12 @@ impl Codegen {
402402
// String::len(s: &String) -> int
403403
if callee == "String::len" && args.len() == 1 {
404404
let mut code = String::new();
405-
let needs_deref = self.infer_irexpr_type(&args[0]).is_some_and(
406-
|ty| matches!(ty, Type::Ref { inner, .. } if matches!(*inner, Type::String)),
407-
);
405+
// Only bare Ref{String} vars (e.g. get_ref bindings) need (*r)->len.
406+
// AddressOf (&local) must not use needs_deref: (&s) is always true under -Waddress.
407+
let needs_deref = matches!(&args[0], IREexpr::Var(_))
408+
&& self.infer_irexpr_type(&args[0]).is_some_and(
409+
|ty| matches!(ty, Type::Ref { inner, .. } if matches!(*inner, Type::String)),
410+
);
408411
if needs_deref {
409412
code.push_str("((");
410413
let mut arg_code = String::new();

0 commit comments

Comments
 (0)