Repository navigation
feat: C array-to-pointer decay, array parameters, and scalar pointer indexing (#389) - #390
Conversation
The documented way to hand an array to a function -- a pointer parameter given &arr[0] -- only half worked for scalar pointers: p[1] = 99; Error: Variable 'p' is not an array *(p + 1) = 99; Error: syntax error, unexpected EQUALS so a callee could read the caller's array through *(p + i) but could not index it, and could only write through a temporary pointer. Struct pointers have had p[i] since #311; this is the scalar counterpart. - evaluate_multi_array_access() gains a scalar-pointer branch next to the struct one: *(p + i) with the stride from the pointee's descriptor, one index only, no bounds check (a pointer has no extent, as in C). - resolve_array_access_element() reports the element as the pointee (one pointer level less), so every value load/store reads the right width; the semantic analyzer's static mirrors agree via the shared is_indexable_scalar_pointer() predicate. - assignment_target accepts `* ( expression )`, the store matching the `*(p + i)` read that already parsed. No new grammar conflicts. - enter_function_scope() now copies a pointer parameter's modifiers. Dropping them made a `giga rizz *p` parameter stride by sizeof(int), so p[1] read the upper half of element 0 (caught by mutation in the new fixture). README and the language reference now say to pass &arr[0] (there is no array-to-pointer decay) and document p[i] / *(p + i) = v in §7.8. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Removes the README limitation "Arrays cannot be passed or returned by value (only via a pointer parameter ...)" by giving arrays C semantics, instead of documenting the &xs[0] workaround. - Decay: where a pointer is expected, the semantic analyzer rewrites a 1-D array (`xs`, or an array struct field) into `&xs[0]` in place (decay_array_operand() -> decay_array_node_in_place()): a pointer parameter's argument, a pointer initializer / assigned value (incl. pointer-array braced initializers), the bussin value of a pointer-returning function, the operand of `*`, `xs +/- int`, and a comparison against a pointer. &xs[0] already works on every one of those paths, so no runtime code changes. - `T a[]` / `T a[N]` parameters are pointers, as C adjusts them (new param_declarator rule; no new grammar conflicts). - Still refused, each naming the fix: a 2-D array (decays to a pointer-to-row, unspellable here), `&xs` (used to yield the address of the interpreter's own storage slot), and an array to a non-pointer parameter (#308, message updated). - New check: a scalar pointer parameter requires a pointer to the same type. User-call pointer arguments were never type-checked, so `f(&fs[0])` silently passed float bits as ints; decay made that one keystroke away. SymbolEntry gains array_rank so a 2-D array can be told apart. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Conflict: tests/expected_results.json -- both sides appended entries at the end; kept all of them. Also closes a hole the merge exposed: #381 rejects `cx[i] = v` on a deadass array, but with decay `*cx = v` / `*(cx + i) = v` (previously "Cannot dereference a non-pointer expression") would have written it through the decayed pointer. Those direct writes are now refused with the same error, checked before the operand decays. New fixture: array_decay_const_fail. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The wasm release dry-run failed two fixtures on values that are platform-dependent, not wrong: - array_decay printed maxxing() of an array parameter (a pointer): 8 natively, 4 on wasm32. It now checks that the size equals a pointer variable's, printing 1 everywhere. - scalar_pointer_index read 5000000000 through a `giga rizz *`, but giga is C long -- 32 bits on wasm32 (#177), where the value does not fit. Switched to thicc (long long, 8 bytes everywhere). Mutation re-checked: dropping the pointer-parameter modifier copy still prints "thicc: 0 705032704" and fails. Comments that called giga 64-bit now say thicc. Co-Authored-By: Claude Opus 5.5 <[email protected]>
leo-aa88
left a comment
There was a problem hiding this comment.
REQUEST CHANGES
The decay-as-rewrite idea is sound. &xs[0] is already the one path every consumer understands, and rewriting into it means no runtime code has to learn that an identifier can be an address. The problem is everything around the rewrite: the new type check for pointer arguments ignores width, the const guard pattern-matches syntax, the two spellings of the same element access now stride differently, and CI is red on the PR's own fixtures.
(Posted as a COMMENT review only because GitHub refuses REQUEST_CHANGES on a self-authored PR; the verdict is the heading above.)
1. The new "same type" pointer check ignores width, and p[i] turns that into a heap overflow
BLOCKING. CONFIRMED (ASan).
The PR introduces a check so that "a scalar pointer parameter now requires a pointer to the same type". It compares only VarType and pointer_level. giga rizz * and rizz * are both VAR_INT / level 1, so they compare equal. Meanwhile the new p[i] branch strides by var->desc.modifiers, which is 8 bytes for giga.
skibidi f(giga rizz *p) { p[1] = 7; }
skibidi main { rizz xs[2] = {1, 2}; f(xs); bussin 0; }==ERROR: AddressSanitizer: heap-buffer-overflow ... WRITE of size 4
#0 write_value_to_address ast.c:4324
#1 execute_assignment ast.c:7084
On main this program could not exist. p[1] on a scalar pointer was rejected, and *(p + 1) strides by 4 (see #3). The check was added specifically because "decay made the mistake one keystroke away". It catches chad vs rizz and waves through rizz vs giga rizz, the only mismatch here that corrupts memory. nonut rizz * vs rizz also passes (it silently stores -294967296), but that one stays in bounds.
The docs now say "The argument's element type must match the pointer's". The runtime disagrees. The check has to compare the element's storage width (modifiers) as well as the VarType, and it should fail closed when the argument's descriptor can't be inferred instead of skipping (actual_type != NONE &&).
2. The deadass guard is a syntax match, and *(cx + 1 + 0) = 9 gets past it
BLOCKING. CONFIRMED.
The PR says *cx = v and *(cx + i) = v are "refused with the same error so decay isn't a way around const". The check recognizes exactly three shapes: cx, cx ± e and e + cx. Add one more term and it no longer applies:
deadass rizz cx[3] = {1, 2, 3};
*(cx + 1 + 0) = 9;
*(cx + 0 + 2) = 8;
yapping("%d %d %d", cx[0], cx[1], cx[2]); 🚽 prints: 1 9 8Neither line could be written on main: *( ... ) = v didn't parse and cx + 1 didn't decay. This PR opened a direct write path into a const array, with no pointer variable involved. That is the exact thing its own description says it prevents. ಠ_ಠ
A syntactic shape list will always be one parenthesis behind. Find the array the address is derived from structurally: walk the +/- chain through its pointer-typed side (the side infer_expression_pointer_level reports as a pointer, or that array_expression_rank reports as an array) down to the array it came from, and run the const check on that. Then add the three-term cases to array_decay_const_fail.
3. p[i] and *(p + i) now address different memory on a giga rizz *
MAJOR. CONFIRMED.
The issue's contract and the new reference section both say "p[i] is *(p + i)". The new branch in evaluate_multi_array_access() computes its own stride from the Variable's modifiers. The comment explains why: "Unlike that function this reads the Variable's own modifiers rather than the node's, which is what makes a giga rizz * stride 8". pointer_arith_scale() (ast.c ~4070) still uses expr->modifiers, so the arithmetic spelling strides by 4:
skibidi show(giga rizz *p) { yapping("arith: %ld %ld", *(p + 1), *(p + 2)); }
giga rizz g[3] = {10, 20, 30}; show(g); 🚽 arith: 0 20 (upper half of g[0], then g[1])The wrong stride in pointer_arith_scale() was found and then worked around in a second, parallel stride computation, rather than fixed in the one function both spellings should share. The description lists it as "not fixed here", but this PR is the one that makes p[i] == *(p + i) a documented guarantee. Either route both through the same stride source (the Variable descriptor) or don't document the equivalence for 64-bit pointees yet.
4. CI is red: the wasm dry-run fails on this PR's own fixtures
BLOCKING (merge gate).
Release dry-run wasm fails on two fixtures:
array_decay:sizes: 12 8vssizes: 12 4.maxxing(a)on a pointer parameter is 4 on wasm32.scalar_pointer_index:giga: 5000000000 3vsgiga: 705032704 3.gigais 32-bit on wasm32 (#177).
Both are known ILP32 differences, and tests/run_wasm_tests.mjs already has WASM_EXPECTED_OVERRIDES for exactly this. Add entries for both fixtures, with a comment like the neighbouring ones. Alternatively, move the platform-dependent lines into their own fixtures so a pointer-size override doesn't also hide the decay assertions.
Note (pre-existing, not blocking)
The new docs advertise bussin xs from a pointer-returning function. With a local xs, any read through the returned pointer is a host-level heap-use-after-free under ASan. It is the same with bussin &xs[0] today, so this PR didn't cause it. But C compilers diagnose this (-Wreturn-local-addr), and the analyzer now sees the exact shape at the decay site. Rejecting bussin <local array> there would be cheap.
VERDICT
The decay mechanism itself is the right architecture. The defects are in the two contracts this PR adds on top of it. The pointer-argument type check is described as "same type" but implemented as "same VarType". Combined with the new modifier-aware p[i] stride, that turns a type-check gap into an out-of-bounds heap write (implementation and specification). The const guard is described as closing the decay route around deadass but is implemented as a list of AST shapes, and one extra + 0 defeats it (abstraction). p[i] vs *(p + i) shows the same pattern of fixing a shared mechanism locally instead of at its source (duplicated machinery).
CI is red, and the problems that matter are not the ones CI caught. The current fixtures don't cover a mismatched pointee width or a const array written through a longer address expression. Please fix #1, #2 and #4, add adversarial fixtures for #1 and #2, and fix or un-document #3. Not mergeable as is.
| infer_expression_pointer_level(arg->expr, analyzer); | ||
| VarType actual_type = | ||
| infer_expression_type(arg->expr, analyzer); | ||
| if (actual_pl > 0 && actual_type != NONE && |
There was a problem hiding this comment.
BLOCKING. This compares VarType and level only. giga rizz * vs a decayed rizz array compares equal, and the new p[i] path then strides 8 bytes over 4-byte elements. ASan: heap-buffer-overflow on skibidi f(giga rizz *p) { p[1] = 7; } called as f(xs) with rizz xs[2]. Compare the pointee's width modifiers too, and fail closed when actual_type == NONE instead of skipping the check. (Finding 1.)
| const-traced. */ | ||
| ASTNode *operand = node->data.op.left->data.unary.operand; | ||
| ASTNode *written = NULL; | ||
| if (array_expression_rank(operand, analyzer)) |
There was a problem hiding this comment.
BLOCKING. This only recognizes cx, cx ± e and e + cx. *(cx + 1 + 0) = 9; on a deadass rizz cx[3] passes and prints 1 9 8, so decay is a way around const. Follow the pointer-typed side of the +/- chain down to the array the address comes from instead of matching fixed shapes. (Finding 2.)
| yyerror("Indexing a null pointer"); | ||
| exit(EXIT_FAILURE); | ||
| } | ||
| size_t stride = get_type_size_for_descriptor( |
There was a problem hiding this comment.
MAJOR. This is a second stride computation, kept separate because pointer_arith_scale() reads the node's modifiers and gets giga wrong. As a result p[1] and *(p + 1) read different elements on a giga rizz * (*(p + 1) on {10, 20, 30} gives 0), while the new docs promise p[i] is *(p + i). Fix the stride source once and have both spellings use it. ಠ_ಠ (Finding 3.)
| "float_inc_dec": "2.50\n3.50\n2.50\n1.50\n3.50\n2.50\n4.50\n6", | ||
| "lex_error_unexpected_tilde": "Error: unexpected character '~' at line 8\nParsing failed", | ||
| "lex_error_unexpected_dollar": "Error: unexpected character '$' at line 6\nParsing failed", | ||
| "scalar_pointer_index": "bump: 199\ncaller sees: 100 99 3\nfill: 0 10 20\nfill_via_deref: 1 11 21\nchad: 0.5 1.5 2.5\ngigachad: 0.25 0.75 3.00\nyap: abZ\nsmol: 1 2 300\ncap: 0 1\nnonut: 1 4000000000\ngiga: 5000000000 3\nrelative: 10 21 30 40\nvalue: 42 20 40\ncondition: ok\nsizeof: 4 8\nmulti-level: 1 50", |
There was a problem hiding this comment.
BLOCKING (CI). giga: 5000000000 is LP64-only. The wasm dry-run gets 705032704 (32-bit giga, #177). Needs a WASM_EXPECTED_OVERRIDES entry in tests/run_wasm_tests.mjs, or move the platform-dependent lines into a separate fixture. (Finding 4.)
| "lex_error_unexpected_dollar": "Error: unexpected character '$' at line 6\nParsing failed", | ||
| "scalar_pointer_index": "bump: 199\ncaller sees: 100 99 3\nfill: 0 10 20\nfill_via_deref: 1 11 21\nchad: 0.5 1.5 2.5\ngigachad: 0.25 0.75 3.00\nyap: abZ\nsmol: 1 2 300\ncap: 0 1\nnonut: 1 4000000000\ngiga: 5000000000 3\nrelative: 10 21 30 40\nvalue: 42 20 40\ncondition: ok\nsizeof: 4 8\nmulti-level: 1 50", | ||
| "scalar_pointer_two_index_fail": "Error: A pointer takes exactly one index -- index a multi-level pointer one level at a time at line 20", | ||
| "array_decay": "fill: 0 10 20\nsum: 30 sum3: 30\nsizes: 12 8\ninit: 0 99\nassign: 11\nderef: 5 11 7\ncompare: ok\nstructs: 101.5 102.5\nfield: 24\nreturned: 80\nyap: Hi\npointers: 2\nrows: 5 60", |
There was a problem hiding this comment.
BLOCKING (CI). sizes: 12 8 is maxxing of a pointer, which is 4 on wasm32, so the wasm dry-run fails here. Same fix as scalar_pointer_index. (Finding 4.)
leo-aa88
left a comment
There was a problem hiding this comment.
REQUEST CHANGES
Re-review of 78d969f ("test: make the #389 fixtures pass on wasm32"). This commit addresses finding 4 only. semantic_analyzer.c is byte-identical to the previously reviewed head, and the ast.c changes are comment-only, so findings 1–3 from the previous review still stand as written.
Finding 4: wasm fixtures (addressed in the fixtures; CI still running)
Switching giga → thicc is better than a WASM_EXPECTED_OVERRIDES entry. It keeps one expected string for both targets, and the new fixture comment states the real reason (giga is C long, which is 32-bit on ILP32). Comparing maxxing(a) to maxxing(probe) instead of printing a platform-dependent number is the right idea too. One weakness is noted inline. The wasm dry-run hadn't reported yet when this was posted.
Still open
1. Heap overflow through the width-blind pointer check — BLOCKING, unchanged
The check at semantic_analyzer.c:2804 still compares VarType and pointer_level only. skibidi f(giga rizz *p) { p[1] = 7; } called as f(xs) with rizz xs[2] still type-checks. Natively, giga is 8 bytes (LP64), so p[1] writes past the 8-byte buffer: the same ASan heap-buffer-overflow as before. The rename to thicc in the fixtures adds a second spelling that hits the same hole (thicc rizz * vs rizz array). Nothing in this commit touches the check, so the docs' "the argument's element type must match the pointer's" is still not true.
2. deadass bypass via *(cx + 1 + 0) = 9 — BLOCKING, unchanged
The shape-matching const guard at semantic_analyzer.c:3985 hasn't changed. A const array is still writable with one extra + 0.
3. p[i] ≠ *(p + i) for 8-byte pointees — MAJOR, unchanged
evaluate_multi_array_access() still uses its own stride, from the Variable's modifiers, while pointer_arith_scale() uses the node's. The only change here is the comment saying thicc instead of giga. The comment now documents the divergence for two type spellings instead of one, and still nothing fixes it. ಠ_ಠ
VERDICT
The wasm fix is reasonable. The two blocking defects are still there: an out-of-bounds heap write that the PR's own new type check fails to prevent (implementation and specification), and a const guard that pattern-matches AST shapes instead of tracing where the address came from (abstraction). Neither depends on CI, so a green run won't change this verdict. Fix #1 and #2 with adversarial fixtures (a mismatched-width pointer parameter, and a const array written through a three-term address), and either unify the stride source or stop documenting p[i] == *(p + i). Not mergeable as is.
| 🚽 than printed: a pointer is 8 bytes natively but 4 on wasm32. | ||
| cap param_is_pointer_sized(rizz a[]) { | ||
| rizz *probe = a; | ||
| bussin maxxing(a) == maxxing(probe); |
There was a problem hiding this comment.
MINOR. Platform-neutral, but it only proves anything natively. On wasm32 a pointer and an int are both 4 bytes, so if a regression bound rizz a[] as a scalar rizz, this would still print 1 there. The native run catches it (4 ≠ 8), so this isn't blocking. If the intent is "this is a pointer, not an int", comparing against maxxing(rizz) and asserting != would only hold on LP64. Consider a short comment that the native run is the one that discriminates.
…st trace, one stride source Finding 1 (blocking): the pointer-argument check compared VarType and pointer level only, so `giga rizz *` and `rizz *` matched and `p[1]` on the 8-byte stride was an ASan heap-buffer-overflow. It now compares the pointee's storage modifiers too (infer_storage_modifiers() / same_storage_modifiers(): long, long long, unsigned), and fails closed when a pointer argument's pointee cannot be inferred. SymbolEntry gains the declared modifiers. Finding 2 (blocking): the deadass guard matched three AST shapes, so `*(cx + 1 + 0) = 9` wrote a const array. address_source_lvalue() now walks pointer arithmetic to any depth (and through `&lv`) to the lvalue the address is computed from, then runs #381's const check on it. Finding 3 (major): `p[i]` strode by the Variable's modifiers while `p + i` read the parse-time node's. Both now use get_expression_modifiers() (the declaration's modifiers) and pointee_stride(). The same source sets the width of loads through `*` (new OP_DEREFERENCE case in expression_is_long/evaluate_expression_long) and of stores (execute_assignment for non-identifier targets) -- which also fixes 64-bit stores into plain thicc/giga array elements and through pointers, previously truncated to 32 bits. Review note: `bussin <local array>` (and `xs + i`, a local struct's array field) is now refused at the decay site; `salty` locals and arrays reached through a pointer parameter are still fine. Inline minor: array_decay notes that only the native run discriminates pointer-vs-rizz parameter size. Fixtures: pointer_width_consistency (new), array_decay_local_return_fail (new), array_decay_fail (+giga/nonut width mismatches both ways), array_decay_const_fail (+three-term, nested, &cx[i], struct-field forms). Mutation-checked: the width comparison and the pre-fix stride/store code each fail the new fixtures. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Addressed in ab6dddc. 1. Width-blind pointer check. Fixed. The check now compares the pointee's storage modifiers ( 2. 3. 4. wasm. Already fixed in 78d969f; the dry run is green. Note (local array return). Done. Inline minor. The Local results: |
leo-aa88
left a comment
There was a problem hiding this comment.
COMMENT
Re-review of ab6dddc. This commit fixes the three open findings at their root causes, not with special cases. I verified each one against a local build of this head:
| Finding | Before | Now |
|---|---|---|
| 1. width-blind pointer check → heap overflow | f(xs) into giga rizz *p, ASan heap-buffer-overflow at p[1] = 7 |
expected a pointer to long int (level 1), got a pointer to int |
2. deadass bypass |
*(cx + 1 + 0) = 9 wrote the const array |
Cannot modify const variable |
3. p[i] ≠ *(p + i) |
arithmetic strode 4 bytes on an 8-byte pointee | thicc {10, 5000000000, 30} reads 5000000000 30 both ways; *(q + 2) = 6000000000 round-trips |
address_source_lvalue()follows the address operand recursively instead of enumerating AST shapes, which is the right abstraction.pointee_stride()+get_expression_modifiers()givep + i,p[i], loads and stores one source of truth. As a side effect, this also fixes the pre-existing 64-bit store truncation the description had listed as out of scope.- The
bussin <local array>rejection closes the use-after-free I flagged as a note. - I also checked block-scoped pointers (declared inside
flex/edgybodies) against the new width check; they're accepted and stride correctly.
What remains is one merge gate and two smaller defects.
1. CI is red: static-analysis fails on a dead store, so build and test never ran
BLOCKING (merge gate).
ast.c:7161:19: error: Value stored to 'mods' during its initialization is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
The new else { mods = get_expression_modifiers(target); } means both branches now assign mods, so TypeModifiers mods = node->modifiers; is dead. Declare it without that initializer. Because the job failed, build, test and every release dry-run were skipped. None of this commit's new fixtures (pointer_width_consistency, array_decay_local_return_fail, the extended const and fail cases) or the wasm run has executed in CI yet.
2. The new store-width lookup re-evaluates struct-target subscripts: side effects run 4× (was 3×)
MINOR, CONFIRMED.
get_expression_modifiers(target) on a NODE_STRUCT_ACCESS goes through resolve_struct_access(), which evaluates the base, including any index expression inside it:
rizz idx(rizz *c) { *c = *c + 1; bussin 0; }
gang S ss[2];
ss[idx(&c)].v = 5; 🚽 main: idx() runs 3 times; this head: 4 timesRunning it three times is a pre-existing bug, and array and *(p + e) targets stay at one evaluation. But this line adds one more evaluation of user side effects just to learn a static property (the field's declared modifiers). Use static_struct_field() first here, or reuse the StructField that the address resolution already finds, instead of resolving the whole access again.
3. "Fails closed" for native pointers only applies when the native is called inline
MINOR, CONFIRMED.
With the test-native library (STDROT_LIB_PATH=tests/libstdrot.so):
rizz first(rizz *p) { bussin p[0]; }
rizz *p = test_ptr_source();
first(p); 🚽 accepted
first(test_ptr_source()); 🚽 "cannot determine what this pointer points to"The same pointer with the same information is refused as an argument but accepted once it goes through an initializer, so the new docs line "a pointer ... returned by a native is refused" is only half true. first(test_ptr_source()) also ran on main (with *p), so this is a small regression for the inline form. C converts an opaque pointer implicitly, and Brainrot has no cast to opt in with, so pick one rule and apply it everywhere:
- treat a type-erased native
STDROT_PTRreturn likeskibidi *(accept it, as C does forvoid *), or - refuse it at the initializer as well.
VERDICT
The architecture problems from the last two rounds are fixed properly. Width is now part of the pointer type contract. Const tracing follows the address operand. Stride, load width and store width come from one descriptor lookup. No blocking correctness or memory-safety defect remains that I can demonstrate.
The PR still isn't mergeable, for a mechanical reason: CI is red on a one-line dead store, and as a result the new fixtures haven't run in CI at all. Fix the dead store. Ideally also address #2, since adding a side-effecting evaluation to every struct-field store is the kind of thing that ships unnoticed, and make #3's rule consistent. Once build, test and the wasm dry-run are green, this is an LGTM from me.
| target_type/target_pointer_level resolved correctly above via | ||
| get_expression_type()/get_expression_pointer_level(), which both | ||
| route through resolve_struct_access(). */ | ||
| else |
There was a problem hiding this comment.
BLOCKING (CI). This new else means every path now reassigns mods, so its initializer at line 7161 (TypeModifiers mods = node->modifiers;) is a dead store. clang-analyzer DeadStores fails static-analysis (ast.c:7161:19), and build/test are skipped. Drop the initializer.
| modifiers are parser state. Reading the node's is why `g[1] = | ||
| 5000000000` on a `giga rizz g[N]`, `p[1] = ...` and `*(p + 1) = | ||
| ...` through a `giga rizz *` all stored 32 bits (#389 review). */ | ||
| mods = get_expression_modifiers(target); |
There was a problem hiding this comment.
MINOR. For a struct target this runs resolve_struct_access() again, which evaluates index expressions in the base. ss[idx(&c)].v = 5 now calls idx() 4 times (3 on main). The modifiers are a static property of the field declaration, so use static_struct_field() or reuse the field the address resolution already found, instead of re-running user side effects.
| arg->expr, analyzer, &actual_mods); | ||
| char error_msg[MAX_BUFFER_LEN]; | ||
| error_msg[0] = '\0'; | ||
| if (actual_pl > 0 && !known) |
There was a problem hiding this comment.
MINOR. Fail-closed only applies to the inline form: first(test_ptr_source()) is refused, but rizz *p = test_ptr_source(); first(p); is accepted with exactly the same information, and the inline form ran on main. Either accept type-erased native pointers here as C does for void * (Brainrot has no cast to opt in with), or refuse them at the initializer too, so the docs sentence is true either way.
…ve-pointer rule 1. (blocking, CI) execute_assignment(): both branches now set `mods`, so its initializer was a dead store (clang-analyzer DeadStores failed static-analysis, which skipped build/test). Declared without one. `make tidy` is clean locally. 2. get_expression_modifiers() resolved struct targets with resolve_struct_access(), which evaluates index expressions in the base: `ss[idx(&c)].v = 5` called idx() 4 times (3 on main). The field's modifiers are a property of its declaration, so they now come from declared_struct_field() -- a no-evaluation lookup over variables, struct-array elements, member chains and static_struct_field()'s cases -- with the runtime walk only as a fallback. Back to 3, as on main. 3. A native's STDROT_PTR result was refused inline (`f(native())`) but accepted through a pointer variable. STDROT_PTR is type-erased by contract, so it now converts like C's `void *` in both spellings: only the pointer level is checked. New fixture native_ptr_user_pointer_param (test natives, so listed in WASM_PRODUCTION_SKIP). Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Addressed in 3a5c0f3. 1. Dead store (CI). Fixed. 2. Struct-target re-evaluation. Fixed. 3. Native pointer rule. I went with your first option. A Local results: |
Description
This PR removes the README limitation "Arrays cannot be passed or returned by value (only via a pointer parameter, which aliases the caller's array like in C)" by giving arrays C semantics. That bullet is deleted.
Before, an array couldn't reach a function at all:
fill(xs, 3)was rejected ("int arrays cannot be passed to a function").rizz sum(rizz a[], rizz n)was a syntax error.&xs[0]workaround was only half usable, because a scalar pointer couldn't be indexed (p[1]) or written through*(p + i) = v.Now:
As in C, a function still can't return an array, only a pointer, and that's the language's design rather than a gap. Copying an array by value, which C doesn't do either, stays with #194.
1. Array-to-pointer decay
xs, or an array struct field likebox.vals) into&xs[0](decay_array_operand()→decay_array_node_in_place()). That happens for:rizz *rows[2] = {xs, ys}bussinvalue of a pointer-returning function*xs ± integermaxxing(xs)is the array's size, and natives still getyaparrays as strings.&xs[0]already works on every path: arguments, initializers, returns, pointer arithmetic, struct-tag checks and the interpreter. So no runtime code had to learn that an array name can stand for an address. Nodes live in the AST arena, so the in-place rewrite frees and leaks nothing.gang E pool[N]→gang E *) and arrays of pointers (rizz *ptrs[N]→rizz **) decay the same way.2. Array parameters
T a[]andT a[N]are pointer parameters, exactly as C adjusts them (newparam_declaratorrule inlang.y; no new grammar conflicts).Nisn't checked, andmaxxing(a)in the callee is a pointer's size.3. Indexing and storing through scalar pointers
p[i]works onrizz *,chad *,yap *, and so on, including multi-level pointers. It was previously supported only on struct pointers (feat: struct pointer arithmetic and indexing (#311) #316).*(p + i) = v;now parses.enter_function_scope()now copies a pointer parameter's modifiers. Before, agiga rizz *pparameter used a 4-byte stride; the fixture catches the mutation.4. One source for pointee width (review finding 3)
p[i]andp + iused to read different modifiers: the variable's forp[i], the parse-time node's forp + i. Both now useget_expression_modifiers(), which reads the modifiers from the declaration, and the sharedpointee_stride(). Loads through*/*(p + i)and stores (execute_assignmentfor non-identifier targets) take their width from the same place.This also fixes an existing bug: on
main, storing a 64-bit value into agiga/thiccarray element (g[1] = 5000000000) or through a pointer kept only the low 32 bits.What is still rejected, with a message that names the fix
f(m)for a 2-Drizz m[2][2]rizz (*)[2], which Brainrot can't spell. The message suggests&m[0][0].f(fs)with achad fs[N], orf(xs)with arizz xs[N]for agiga rizz */nonut rizz *parametermainf(&fs[0])silently passed float bits as ints. A scalar pointer parameter now requires a pointer to the same storage: same element type and width/sign modifiers.giga rizz *vsrizz *matters most, becausep[1]there strides 8 bytes over 4-byte elements. A pointer argument whose pointee can't be determined is refused (fail closed). The exception is a native'sSTDROT_PTRresult, which is type-erased by contract and converts like C'svoid *: only its level is checked, whether passed inline or through a pointer variable. Askibidi *parameter still accepts any pointer.&xsmainit silently returned the address of the interpreter's internal slot, not the array (*pprinted560for{4, 5}). It's now an error that points toxs.*cx = v,*(cx + i + j) = vor*(&cx[i]) = von adeadassarraycx[i] = v, which #381 rejects. The array is found structurally, through pointer arithmetic of any depth and through&lv, then #381's const check runs on it. Passingcxto a pointer parameter is still allowed, exactly as&cx[0]already was: Brainrot has no pointer-to-const type, and #381 deliberately doesn't trace const through pointer variables.bussin xs/bussin xs + 1/bussin b.valswhere the array is the function's own local-Wreturn-local-addr).salty(static) locals and arrays reached through a pointer parameter are fine.*p/p[].Existing bugs found while testing, not fixed here
rows[0][1]on arizz *rows[N]prints nothing, without an error.yaparrays.yap cs[3] = {'a','b','c'}fails with "Unsupported array type for initialization".Related Issue
Fixes #389. Also covers the C-style array-parameter part of #194; by-value array copies are not implemented.
Type of Change
Not a breaking change: every program that ran before still runs. The
&xsand pointer-type-mismatch errors only reject programs that previously produced garbage values.Checklist
test_cases/*.brainrotfor language-visible behavior; a host-side C test wired intomake testfor internal APIs Brainrot source cannot reach). Required for every bug fix, feature, refactor, performance change, and breaking change — not optional, not "if applicable". See CONTRIBUTING.md ("TESTS ARE MANDATORY").array_decay: every decay context, each checked by writing through the decayed pointer and reading the caller's array back. Alsoa[]/a[N]parameters,maxxingon both sides, struct arrays, an array struct field (as an argument and as abussinvalue), ayaparray to a native and to ayap *, an array of pointers, and a pointer-array braced initializer.array_decay_fail: the 2-D array, a mismatched element type (decayed and explicit&fs[0]), and&xs.array_decay_const_fail: writes to adeadassarray:*cx,*(cx + 1),*(1 + cx), the three-term forms from the review, nested*(2 + (cx - 1)),*(&cx[1]),*(&cx[0] + 1), and a const struct's array field. Ayappingcomes first, so the test fails if the program ever starts running.array_decay_local_return_fail: returning a local array, local array plus offset, and a local struct's array field are refused;saltyand pointer-parameter returns are not.native_ptr_user_pointer_param: a native's opaque pointer passed inline and through a pointer variable to a typed parameter; both are accepted and write the same storage. It uses the test natives, so it's inWASM_PRODUCTION_SKIP.pointer_width_consistency:p[i]and*(p + i)give the same element ongigaandthiccpointers. 64-bit stores throughp[i],*(p + i),*p, into a plainthiccelement, and through a pointer to a scalar all keep 64 bits. Before this change, every line printed wrong values.scalar_pointer_index:p[i]and*(p + i) = vacross every scalar width, each read past index 0 so a wrong stride shows up as a wrong value. Also a local pointer, a negative index,maxxing(p[0]), andrizz **.scalar_pointer_two_index_fail:pp[0][1].array_arg_scalar_param_failandstruct_array_by_value_arg_fail: expected output updated for the new message.make format-checklocally (ormake formatto fix)Local results:
make test550 passed.make format-checkclean.make valgrind484/484 clean (0 errors, nothing definitely or indirectly lost).make tidyclean.make cppcheckwas not run: the local cppcheck is 2.7 and the Makefile requires ≥ 2.13.🤖 Generated with Claude Code