feat: string indexing and slicing -- s[i] and s[i:j] (#251)

[leo-aa88]

Description

The two syntax halves of string stdlib v1. The four builtins (yaplen/yapcat/yapcmp/yapidx) landed in #327; this is the indexing that reads the same byte buffer they measure — and it's what makes pulling a piece out of a string possible at all.

rant s = "hello world";

yap  c   = s[0];        🚽 'h'     -- one byte, as a yap
rant sub = s[0:5];      🚽 "hello" -- a NEW rant

Both of the issue's open questions are answered the way it recommended: both slice bounds required (no s[:j], s[i:], s[:]) and no negative indices.

• Bytes, not runes — a two-byte UTF-8 character is two indices.
• s[i] → yap, index in [0, len). s[i:j] → new rant, half-open [i, j), requiring 0 <= i <= j <= len.
• Out of range is a hard error, not a clamp: silently returning something shorter than asked for turns an indexing mistake into wrong output rather than a stopped program.
• A slice returns a fresh copy, never a view — and also a lifetime requirement, since evaluate_expression_string()'s contract is that every result is a buffer its caller owns and frees.
• s[i] is assignable. s[0] = 74; overwrites that byte in place, exactly as yap buf[0] = 74; does, bounds-checked by the same rule as a read. A rant is therefore not immutable — what the library guarantees is narrower: the builtins never modify their arguments, and a slice is a copy rather than a view.

Worth flagging one deliberate asymmetry: s[len] is an error but s[len:len] is legal and empty, because a slice's upper bound is exclusive, so len is a valid boundary though not a valid index. Easy to get wrong; pinned both ways.

Implementation notes

s[i] needed no new node. resolve_array_access_element() is the single choke point for an array access's element type — get_expression_type() and get_expression_pointer_level() both route through it, and so does every scalar evaluator asking numeric_load() what width to read. Saying VAR_CHAR once there is what makes yap c = s[0]; and yapping("%c", s[0]) agree.

s[i:j] is a new NODE_STRING_SLICE, not a second shape of NODE_ARRAY_ACCESS. An access has one-or-more indices selecting an element; a slice has exactly two bounds selecting a range. Sharing a node type would force every existing NODE_ARRAY_ACCESS site to start asking which it was holding.

The grammar rule is spelled out rather than generalised. The obvious expression LBRACKET expression COLON expression RBRACKET makes the parser choose, on seeing LBRACKET after an IDENTIFIER, between reducing to expression (slice) and shifting into multi_dimension_access (array access) — unresolvable with one token of lookahead, since both continue IDENTIFIER LBRACKET expression. Sharing the prefix defers the decision to COLON vs RBRACKET. Conflict-free under bison -Wcounterexamples. The cost: only a named rant can be sliced, so yapcat(a, b)[0:2] doesn't parse. Documented.

One leak found and fixed during development. execute_native_call() registered only NODE_FUNC_CALL as producing an owned string buffer, so yapping("%s", s[0:2]) leaked one buffer per call. LeakSanitizer caught it. The list now also says why a plain rant identifier is deliberately not on it — it borrows the Variable's storage, so freeing it would destroy a live variable.

Related Issue

Completes #251 (the builtins were #327).

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update
• Performance improvement
• Refactor

This adds new grammar rather than changing any existing keyword, and 490 pre-existing tests are untouched.

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have documented my changes in the code or documentation
• I have added tests that prove my change works, at the lowest appropriate layer
• Those tests cover the happy path, error cases, edge cases, and adversarial cases
• If this PR cannot affect program behavior, I explained that in the Description instead of skipping the items above in silence
• I have run make format-check locally (or make format to fix)
• I have run the unit tests locally
• I have run the valgrind memory tests locally
• All new and existing tests pass

Tests

Four fixtures. string_indexing covers both constructs together, including the line pinning that s[6] and s[6:7] agree — separate code paths, one returning an address and the other allocating, and nothing else here would notice them disagreeing about which byte offset 6 is. Three *_fail fixtures each get their own file, since a process can only die once:

fixture	pins
string_index_out_of_bounds_fail	s[len] — the off-by-one, not a wild index any check would catch
string_slice_out_of_bounds_fail	negative start — the design decision, not plain arithmetic
string_slice_non_rant_fail	static rejection of a non-rant base (no before on stdout)
string_index_assignment	s[i] = c writes in place; literals, copy-init, slices and parameters all stay independent
string_index_write_out_of_bounds_fail	an out-of-range write is refused

Two claims written into those comments were verified by mutation, not asserted:

mutation	result
remove the static slice check	string_slice_non_rant_fail prints before and goes red
weaken the index bound >= → >	both bounds fixtures go red
make s[i] a non-lvalue	string_index_assignment goes red, every other string fixture stays green

The write fixture uses index == len deliberately: safe_strdup() allocates len + 1 and NUL-terminates, so offset len is inside the allocation. Weakening the bound to allow it prints helloX — terminator destroyed, %s running past the intended length — with ASan and valgrind both silent, because nothing out-of-bounds happened. Only the fixture catches it.

Two things checked rather than assumed

The UTF-8 bytes print as -61 -87, not 195 169. That's pre-existing signed-char marshalling at the native boundary, not something indexing introduces — verified that yap arr[2]; arr[0] = 195; prints the same -61. So the fixture pins that s[i] agrees with an ordinary yap array element, rather than claiming unsigned semantics it doesn't have.

A leak that is not mine. Rant reassignment (acc = yapcat(acc, x)) leaks a buffer per assignment. Reproduced identically on main with no slices involved — that's #277. Declaration-from-slice and slice-as-argument are both leak-free. The fixtures avoid reassignment for that reason, and say so.

Gate results

gate	result
make test	496 passed (was 490)
valgrind sweep	435 fixtures, exit 0, zero leaks or errors
make format-check	pass
make tidy	pass
bison -Wcounterexamples	no conflicts
make cppcheck	not run locally — this box has cppcheck 2.7, the Makefile requires ≥ 2.13. Left to CI.

As before, the valgrind sweep ran against a build with -fsanitize dropped: ASan's shadow mapping collides with valgrind on this WSL2 kernel. The sanitized build was restored before make test.

🤖 Generated with Claude Code

[leo-aa88]

COMMENT

Careful work. The grammar reasoning is right, the ownership story holds under
everything I threw at it, and the one leak you found was real — I went looking
for a second one and didn't find it. One finding: the feature is more powerful
than the PR says, and the extra power is neither documented nor pinned.

Verified

s[0] / s[6]           h / w
s[0:5] / s[6:11]      hello / world
s[3:3]                empty, yaplen 0
s[11:11]              legal and empty          -- the documented asymmetry
s[6] vs s[6:7]        both 'w'                 -- the two paths agree
s[i] / s[i:i+3]       variable bounds fine
s[5] on len 5         String index out of bounds (index=5, length=5)

The grammar claim checks out. Regenerated with the Makefile's own
-Wcounterexamples and got no conflict warnings. Spelling the rule out
against IDENTIFIER rather than expression is the right trade, and the cost
(yapcat(a,b)[0:2] doesn't parse) is documented rather than discovered.

The ownership story holds. safe_strdup() copies by .len with memcpy,
never strlen, so slicing a non-NUL-terminated rant is safe by construction.
I chased the leak fix across nine consumer shapes — yapping("%s", s[0:2]),
yaplen/yapcat/yapcmp/yapidx of a slice, rant t = s[0:5], a slice
through a user-defined rant parameter, a slice returned from a function,
and a 100-iteration loop — all ASan-clean, and valgrind on an unsanitized build
gives definitely lost: 0 / ERROR SUMMARY: 0 errors. Worth checking against
valgrind specifically since libstdrot.so is built without sanitizers.

Two other things I checked rather than assumed: the non-rant slice rejection
really is static (the fixture's before never reaches stdout), and
indexing/slicing a rant parameter works, which matters now that #314 gives
parameters their own copy.

The s[len]-is-an-error / s[len:len]-is-legal asymmetry is the kind of thing
that gets "fixed" into consistency by someone later, so pinning both directions
was the right call.

The finding

Inline: s[i] is a writable lvalue. s[0] = 'J' turns "hello" into
"Jello", and nothing in the docs, the fixtures, or the PR description says so
— while the description invokes "the immutability rule" as the reason slices
copy.

VERDICT

The implementation is sound and I couldn't break its memory behaviour. What
needs settling is a specification question the code answered by accident:
rant is now a mutable byte buffer, and the PR text says the opposite. Pick
one and pin it.

[leo-aa88]

MAJOR — this returns the address of a byte inside the live Variable's buffer, and the assignment path uses the same function. So s[i] is a writable lvalue:

rant s = "hello";
s[0] = 'J';
yapping("%s", s);     🚽 Jello

That is a new language capability. On main it isn't reachable — s[0] fails with "Variable 's' is not an array" — so this PR is what introduces in-place mutation of a rant's bytes.

The good news first, because I went looking for the dangerous version and it isn't there. It's memory-safe: the write path goes through the same bounds check (s[99] = 'x' → "String index out of bounds (index=99, length=5)"), and there's no aliasing — two rants from the same literal have independent buffers, and rant b = a; copies, so mutating a leaves b alone. I checked all three.

The problem is that nothing says it exists:

• The docs describe s[i] as "s[i] yields a yap — the byte at offset i". Read-only phrasing; no mention of assignment.
• No fixture assigns to s[i]. grep -E '^\s*[a-z_]+\[[^]]*\] *=' test_cases/string_*.brainrot finds nothing.
• The PR description argues slices copy because of "the immutability rule, and also a lifetime requirement". A reader takes that to mean rant is immutable. It isn't, as of this PR.

That last point is what makes this worth more than a doc nit: the justification given for the slice-copy design asserts a property the same PR removes. Either the rule is real and s[i] = c should be rejected, or the rule was only ever about slices-are-not-views and the phrasing needs narrowing.

I'd take the second — mutable s[i] is consistent with yap buf[i] = c and is genuinely useful — but then it needs to be a decision:

• a line in §8.9 next to the s[i] bullet saying the byte is assignable and bounds-checked on write;
• a fixture doing s[0] = 'J' and reading it back, plus one pinning that an out-of-range write is refused. Without them, the next refactor of resolve_array_access_element() or this branch can silently make s[i] = c a no-op, a hard error, or an unchecked write, and every existing test still passes;
• and re-word the "immutability rule" sentence so it claims what it means: a slice is a copy, not a view.

If you'd rather rule it out instead, that's a defensible v1 answer too — but it should be an explicit rejection with its own *_fail fixture, not something that happens to work.

[leo-aa88]

APPROVE

Both halves of the finding closed, and the write fixture is built around an
insight I hadn't seen.

The claim is narrowed everywhere, not just where I pointed

All four sites that overstated it now say the same narrower thing — both docs,
evaluate_string_slice()'s comment, and yapcat.c's. grep -rn immutab
returns only sentences that deny immutability. The distinction the docs
land on is the correct one:

The builtins never modify their arguments… A slice is likewise a copy, not
a view. Note this is narrower than "strings are immutable" — s[i] = c
does write in place.

Keeping the capability is the right call, and "matches yap buf[i] = c
exactly" is the argument that makes it right rather than merely convenient.

The write fixture's index choice is the good part

The commit claims s[len] = c sits inside the allocation, because
safe_strdup() allocates len + 1 and NUL-terminates — so a weakened bound
would be invisible to the sanitizers. I mutation-tested that rather than take
it:

bound >= weakened to >:
  string_index_out_of_bounds_fail        FAILED
  string_index_write_out_of_bounds_fail  FAILED

s[5] = 'X' on a len-5 rant, weakened bound:
  ASan build      → [helloX]   (no ASan report)
  valgrind        → [helloX]   ERROR SUMMARY: 0 errors, definitely lost: 0

Confirmed both ways. The terminator is destroyed and %s runs past the
intended length, and neither sanitizer says a word, because nothing went
out of bounds of the allocation — only the fixture catches it. Choosing
index == len specifically, over any index that ASan would have caught for
free, is what makes that fixture load-bearing instead of decorative. That is
the sharpest bit of test design I've seen in this series.

Everything else

string_index_assignment pins more than I asked for — the write, the length
staying 5, literal independence, copy-init independence, slice-and-source
independence in both directions, parameter isolation, a builtin call
showing arguments untouched, and the yap buf[i] = c form beside it so the
two can't drift. 496 collected, valgrind sweep clean, format-check and
tidy clean.

VERDICT

The first round's implementation was already sound; what was wrong was a
sentence. This fixes the sentence in every place it appeared, keeps the
behaviour on an argued basis rather than an accidental one, and pins it with a
fixture aimed at the one defect the tooling cannot see.

Ship it.