Repository navigation
Conversation
`a = b = 5`, `rizz c = (b = 5);`, `(b = 5) + 1` and `i = j = 0` in a flex init all parsed, but no value evaluator handled NODE_ASSIGNMENT: the inner write was skipped or its value read as 0, and "Invalid integer expression" was printed. execute_assignment() now wraps execute_assignment_slot(), which returns the address and descriptor it wrote through. Each value evaluator (int/long/short/float/double/bool/pointer/string) performs the write and yields the target's value after it, in the target's type as in C: a plain variable is re-read, any other target is read back from the written address so `x = arr[k++] = 5` evaluates `k++` once. The static type queries (runtime and semantic) report the target's type. create_assignment_target_node() no longer calls get_current_modifiers(): Bison reduces an assignment nested in an initializer before the declaration, so it consumed the declaration's pending thicc -- leaving `thicc rizz u = (t = 8000000000);` a 32-bit rizz, and turning the write in `thicc rizz v = (ra[1] = 5);` into an 8-byte store into a 4-byte rizz element. Co-Authored-By: Claude Opus 5.5 <[email protected]>
leo-aa88
left a comment
leo-aa88
left a comment
There was a problem hiding this comment.
REQUEST CHANGES
The core design is right. execute_assignment_slot() returns the address and descriptor it wrote through, so an assignment expression reads back exactly what was stored, converted to the target's type, without evaluating the target twice. I verified single evaluation on a local build:
yapping("%d", b = b + 1),id(b = b + 1),edgy ((c = c + 1) == 1)andgoon ((n = n - 1) > 0)each run the assignment exactly once.rizz x = arr[k++] = 5;leavingk == 1follows from reading back through the stored address.
The parser fix is also correct and important: create_assignment_target_node() was consuming the enclosing declaration's pending modifiers.
There are two problems: CI is red, and after the required merge with main the PR's own contract ("yields the target's value ... in the target's type") breaks for nonut targets.
1. CI is red: static-analysis fails, so build and test never ran
BLOCKING (merge gate).
ast.c:7169:5: error: switch has 2 consecutive identical branches [bugprone-branch-clone,-warnings-as-errors]
The new case NODE_ASSIGNMENT: in is_expression() is byte-identical to the NODE_OPERATION case right above it. Fold it in as a fall-through label (case NODE_OPERATION: case NODE_ASSIGNMENT:) with the comment. Until then, none of the new fixtures have run in CI.
2. After merging main, an assignment expression drops the target's signedness, bringing #371 back through a new form
BLOCKING. CONFIRMED on this branch merged locally with current main (4988c99, which has #391). That merge is required anyway, because tests/expected_results.json conflicts.
nonut rizz u = 0;
thicc rizz t = (u = 3000000000); 🚽 -1294967296 (C: 3000000000)
(u = 3000000000) > 5 🚽 0 (C: 1)
(u = 3000000000) / 2 🚽 3647483648 (C: 1500000000)
u > 5 🚽 1 (the plain read is right)The PR adds NODE_ASSIGNMENT to every type query it knows about (get_expression_type, get_expression_pointer_level, expression_is_long, infer_runtime_expression_type_noeval, and the two semantic mirrors), each delegating to the target. #391 added a sixth: signedness, expression_is_unsigned() via static_value_descriptor(). That query has no NODE_ASSIGNMENT case, so an assignment to a nonut target reads as signed int. That's the exact wrong output from #371's issue text.
The read-back has the same blind spot: assigned_slot_long() sign-extends an identifier target through (long long)evaluate_expression_int(...) and a non-identifier one through numeric_load(..., VAR_INT), even though slot->mods.is_unsigned is sitting right there in the slot.
"Has the target's type" has to include all of the target's type. When you rebase:
- add
case NODE_ASSIGNMENT:tostatic_value_descriptor(), delegating to the target, soexpression_is_unsigned()/operation_is_unsigned()see it; - zero-extend in
assigned_slot_long()when the slot's descriptor is a 32-bit unsigned int; - add the three lines above to
assignment_expression.brainrot.
The fixture currently has no nonut target at all, so it can't catch this.
3. Minor: one "pre-existing" bullet in the description is now stale
"ta[1] = 5000000000; on a thicc rizz ta[2] stores only 32 bits" was fixed by #390: element stores now take their width from the declaration via get_expression_modifiers(). On the merged build, ta[1] = 5000000000; thicc rizz v = (ta[0] = 6000000000); gives 5000000000 6000000000 6000000000. Update the description so the next reader doesn't go hunting for a bug that's gone.
VERDICT
The implementation is sound on its own base. The defect is in how the PR defines "an assignment expression has its target's type": as a list of type queries patched one by one. The list was complete when the branch was cut and is incomplete against current main, because #391 added a signedness query and a read-back path that ignore NODE_ASSIGNMENT (implementation, from a cross-PR interaction). Since the merge with main is mandatory (JSON conflict), the result as it would land is wrong for nonut targets.
CI is also red on a one-line branch-clone lint. Fix the lint, merge main, add NODE_ASSIGNMENT to the descriptor walk and zero-extension to the read-back, and cover it with a nonut target fixture. Not mergeable as is.
| // For operations, check if the result type matches the target type | ||
| return get_expression_type(node) == type; | ||
| } | ||
| case NODE_ASSIGNMENT: |
There was a problem hiding this comment.
BLOCKING (CI). Identical to the NODE_OPERATION branch above, so clang-tidy bugprone-branch-clone fails static-analysis and build/test are skipped. Make it a fall-through label on that case.
| if (slot->target->type == NODE_IDENTIFIER) | ||
| return expression_is_long(slot->target) | ||
| ? evaluate_expression_long(slot->target) | ||
| : (long long)evaluate_expression_int(slot->target); |
There was a problem hiding this comment.
BLOCKING (after merging main). Sign-extends regardless of the target's signedness. For a nonut rizz u, thicc rizz t = (u = 3000000000); yields -1294967296 (C: 3000000000). The numeric_load(..., VAR_INT) path below does the same for element/field targets. Zero-extend when slot->mods.is_unsigned (32-bit). Also add NODE_ASSIGNMENT to #391's static_value_descriptor(): without it, (u = 3000000000) > 5 is 0 and / 2 is 3647483648, the issue #371 bug again. (Finding 2.)
| return false; | ||
| switch (node->type) | ||
| { | ||
| case NODE_ASSIGNMENT: |
There was a problem hiding this comment.
MAJOR. This delegation pattern (assignment has its target's type) is repeated per query, and on current main the list is one short: #391's signedness query (expression_is_unsigned() → static_value_descriptor()) has no NODE_ASSIGNMENT case. Add it there when you merge main. (Finding 2.)
Description
An assignment used as an expression didn't work. That covers chaining (
a = b = 5), nesting (rizz c = (b = 5);,(b = 5) + 1), aflexinit (i = j = 0), conditions (goon ((n = n - 1) > 0)), and arguments (yapping("%d", b = 9)). The grammar already accepted all of these. But no value evaluator had aNODE_ASSIGNMENTcase, so the inner write was skipped (or its value read as 0) andInvalid integer expressionwas printed.As in C, an assignment expression now performs the write and yields the target's value after the write, in the target's type. So
smol s; (s = 70000)is4464, andii = ff = 3.7givesff = 3.70andii = 3.How:
execute_assignment()is now a thin wrapper overexecute_assignment_slot(), which returns the address it wrote and the descriptor it wrote through.NODE_ASSIGNMENTby performing the write and reading the value back.arr[k++],arr[bump(&m)],*p,s.f) is read back from the address that was written. Its subexpressions therefore run exactly once:rizz x = arr[k++] = 5;leavesk == 1.get_expression_type,get_expression_pointer_level,expression_is_long,infer_runtime_expression_type_noeval,is_expression(used to marshal native arguments), and the semantic analyzer'sinfer_expression_type/infer_expression_pointer_level.Parser fix needed for this to be safe.
create_assignment_target_node()calledget_current_modifiers(), which consumes and resets the parser's pending declaration modifiers. Bison reduces an assignment nested in an initializer before the declaration itself. So inthicc rizz v = (ra[1] = 5);the inner assignment took the declaration'sthicc, which caused two problems:vbecame a 32-bitrizz, sothicc rizz u = (t = 8000000000)gave3705032704;rizzelementra[1]became an 8-bytepacked_long_store. UBSan reports a misaligned store, and it writes past the element.An assignment node now gets empty modifiers and leaves the parser state alone. Statement-level assignments already got empty modifiers, because the state is reset after every declaration, so their behavior is unchanged.
Writing a
deadassvariable inside an assignment expression is still refused during semantic analysis, before anything runs (newassignment_expression_const_failfixture).Pre-existing, not changed here (each reproduces on
mainas a plain statement):ta[1] = 5000000000;on athicc rizz ta[2]stores only 32 bits. An array or struct element write takes its width from the assignment node's modifiers rather than from the element's declaration. An assignment expression yields exactly what was stored, so it matches the statement form.i = 3.7;on arizzprintsCannot use double in integer context, then stores3.rizz *p;) is rejected:Type mismatch in initialization of 'p' ... at line 1.b = a;) is unsupported (Unsupported assignment type), so struct chains are too.Related Issue
Fixes #374
Type of Change
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").make format-checklocally (ormake formatto fix)test_cases/assignment_expression.brainrotcovers:edgyandgoonconditions;smoltruncation,thicc);arr[k++],arr[bump(&m)]);chadelement, struct fields, pointer variables,*p, and a pointer initializer;rant,yapandcaptargets;bussinvalue.test_cases/assignment_expression_const_fail.brainrotlocks in the const check.make test: 544 passed.make format-check: clean.make valgrind: exit 0,ERROR SUMMARY: 0and no definite leaks on all 478 fixtures.🤖 Generated with Claude Code