compiler: string subscripts, s[i] (constant index)

A STRING is a length byte at s[0] then the characters, so s[i] is the
one byte at off+i.  ParseSub folds a constant index into boff (the field
an array already uses) and drops r.cls to TChar, so:

  * LoadAtom loads one byte (A0 off / 8A 46 disp) and zeroes AH;
  * Assignment stores one byte when the symbol is TString but r.cls is
    TChar (the element) rather than TString (the whole string);
  * IoCall already classified on arg.cls, so writeln (s[i]) -> wrchar;
  * the reader now classifies read (s[i]) the same way and pushes
    off+boff.

Only a constant index is accepted; a variable index is ENoLib, exactly
as it already was for an array (the runtime address arithmetic TP3 does
in indstr/ialoop is still to come).

This uncovered a latent bug: ParseAtom did `r.boff := 0' AFTER
ParseVar/ParseSub, discarding every subscript.  ParseVar already zeroes
boff before ParseSub, so the reset was pure loss - s[i] addressed off+0,
the length byte.  Nothing caught it because an array cannot be declared
yet, so no fixture had ever exercised the fold.  t44_strindex is red
with the reset restored (reads the length byte) and green without it.

Gates: compile matrix 46/46, run_com_exec 41/41, exec86 41/41, runtime
entries 51/51, OVERALL: ALL PASS.  tpshell clean build 191104 bytes.

Tag: v-TP3-STRINDEX.

SUMMARY.md

@@ -33,6 +33,7 @@ manual, not guessed.
 | String *reads* and string-to-string assignment — `readln(s)`, `s := t` | `v-TP3-STRREAD-ASSIGN` | done, **`readln(s)`/`read(s)` and `s := t` on string variables. Two runtime entries carry selectors 16/17 (`rdstr`, `strstore`); the caller sets the limit in `CX` with a full `MOV CX,imm16`. `rdstr` (TPSRC4 `xrdstr`) reads a line, stops at CR/LF/^Z, leaves the terminator pending for the existing `rdln` wrapper, truncates at the limit, and stores the length last; `strstore` (TPSRC2 `strstore` + TPSRC8 `asnvar`) copies `min(len(t), size(s)-1)` characters, never writes the source, and leaves the destination tail past the new length untouched. The milestone also exposed a pre-existing frame-layout bug: a local `string` was packed with the *integer* rule, so its characters grew UP over the saved BP at `[BP]` and the return address at `[BP+2]` — `readln` into such a local then ran the string's own bytes as its return address. Locals are now packed top-anchored, which keeps every integer offset byte-for-byte and puts a `string[5]` at `BP-6..BP-1`, inside the frame (38/38 executed, 51/51 entry checks)** |
 | Multi-name declarations, both spellings — `var a, b, c : integer`, `procedure p (x, y : integer ; z : integer)` | `v-TP3-MULTIDECL` | done, **definitions and formal parameters read a comma-separated list of NAMES and then one `: type`, exactly TPSRC7 `rdvarlst`/`rdvloop` (VAR) and `prfcnt`/`ccomma` (parameters), with `;` between parameter groups. The older `name : type, name : type` spelling still parses, because both reach the same symbols; the names are created as placeholders and given the group's one type afterwards. One `GetWord` that did not `Skip` first attached the space after a comma to the next name (`" B"`), which is why the first name worked and the second did not; `t42_multidecl` prints each name's value to prove every one landed on its own slot (43/43 compile, 39/39 executed)** |
 | The 4 KiB code window enforced — TP3 error 98 instead of writing over the data | `v-TP3-CODEWINDOW` | done, **`dataBase` is `rtSz + 1000H`, so code and data share one 64 KiB `.COM` segment and code may not reach it. `Ebyte` now refuses past `dataBase` (guarded by `imgReady`, since `dataBase` is 0 until `Compile` measures it) and reports TP3's error 98, "Memory overflow" — the same number `DefVar` already uses when `dc` overflows. `t43_codewindow` emits more than 4 KiB of inline literal bytes and must report `ERR 98`; disabling the check turns the fixture green-as-`OK`, which is the bug (a silently corrupt image), so the matrix row is the proof (44/44 compile)** |
+| String subscripts — `s[i]` read, write, and `readln (s[i])` | `v-TP3-STRINDEX` | done, **a STRING is a length byte at `s[0]` followed by the characters, so `s[i]` is the single byte at `off+i`: reading it yields a `CHAR`, assigning to it stores one byte, and `readln (s[i])` reads one byte at `off+i`. A constant index folds into `boff` as an array's does and `r.cls` drops to `TChar`, so `LoadAtom`, `Assignment` and `IoCall` all see one byte. Only a constant index is accepted — a variable index is `ENoLib`, exactly as it already was for an array. The fix uncovered a latent bug: `ParseAtom` did `r.boff := 0` *after* `ParseVar`/`ParseSub`, discarding every subscript, so `s[i]` addressed `off+0` (the length byte). `ParseVar` already zeroes `boff`, so the reset was redundant and was removed. `t44_strindex` prints every indexed byte and `t45_strindex_read` feeds `readln (s[i])`; the indexed read is red before the parse fix and green after (46/46 compile, 41/41 executed)** |
 
 Every row that names a tag has one, and every tag points at a commit on
 `master`; verified by diffing the rows against `git tag -l`, which is how
@@ -126,11 +127,11 @@ needing a squint. It was checked for vacuousness by reverting the string
 scanner fix: the suite went red with exit 1, and came back green only when the
 fix was restored.
 
-**39 of 44 fixtures compile clean**, up from 1 (the empty program) when the
+**41 of 46 fixtures compile clean**, up from 1 (the empty program) when the
 direct harness was first built.
 
 ```
-compile matrix: 44 passed, 0 failed (of 44)
+compile matrix: 46 passed, 0 failed (of 46)
 ```
 
 Compiling: `t01` minimal · `t04` var+assign+`writeln` · `t06` two args ·
@@ -153,7 +154,9 @@ inside a procedure, so both address paths run ·
 `t40` `readln` into a global *and* a local `string` ·
 `t41` `s := t` between two strings, with the truncation both ways ·
 `t42` `var a, b, c : integer` and `procedure p (x, y : integer ; z : integer)`,
-each name landing on its own slot.
+each name landing on its own slot ·
+`t44` `s[i]` read and write on a global and a local string ·
+`t45` `readln (s[i])` storing one character at position `i`.
 
 Failing, all deliberately: `t14` `array [1..5] of integer` at its point of use
 and `t25` a string literal assigned to an *integer* (`i := 'ab'`, the reverse
@@ -165,9 +168,9 @@ refuses the whole class because the runtime has no boolean reader at all; and
 "Memory overflow", rather than a corrupt image; and
 `uierror` is a deliberate syntax error (41) used by the UI test.
 
-`run_com_tests.sh` additionally links **all 39 that compile** to a real `.COM`
+`run_com_tests.sh` additionally links **all 41 that compile** to a real `.COM`
 and re-verifies the bytes with an independent Python checker that *measures*
-the layout instead of restating it: `independent .COM check: 39 checked, 0
+the layout instead of restating it: `independent .COM check: 41 checked, 0
 failed`. That last part was itself a bug fix — see "the two restated
 constants" below.
 
@@ -211,7 +214,7 @@ proves each one can go red.
 | `check_runtime.py` + `runtime.golden` | the built runtime's code region (585 bytes, code 0..584) sweeps cleanly through FCML, every entry and all branch targets land on an instruction boundary, and the whole disassembly is byte-for-byte the committed golden | whether the golden is *right* |
 | `check_framedisp.py` | `[BP+off]` uses disp8 iff `off <= 127`, for locals (negative) and far parameters (>127) | which of the two encodings was chosen, if the other also works |
 | `run_com_exec.py` | the **emitted image executes** and prints exactly the expected bytes | semantics the fixture never exercises |
-| `run_exec86.py` | the same image runs in a **second, independent 8086** and agrees with qemu byte for byte, 39/39 | the `.out` file it shares with `run_com_exec.py` — a wrong expectation fails both at once |
+| `run_exec86.py` | the same image runs in a **second, independent 8086** and agrees with qemu byte for byte, 41/41 | the `.out` file it shares with `run_com_exec.py` — a wrong expectation fails both at once |
 | `rt_exec.py` | the **runtime entries themselves** are called directly, one qemu boot per call, 50 cases plus a pre-flight, 51/51 — plus a `check_bp_contract` pre-flight over the built blob: an entry that borrows BP must open with `55 8B EC` (exact) and contain a `5D` (a screen, because `5D` is also a displacement byte) | whether the *caller's* half of a contract is right, where the only witness is a case that happens to make two calls in a row |
 
 Two of these deserve the detail, because the reason they exist is the reason
@@ -284,7 +287,7 @@ and `tests/check_comimage.py` asserting there is only one copy of each.
 ### Execution under qemu — `tests/run_com_exec.py`
 
 The check that cannot be written as a byte comparison, so also the one that
-finds the most: 39 fixtures are compiled to `.COM`, put on a floppy, booted,
+finds the most: 41 fixtures are compiled to `.COM`, put on a floppy, booted,
 and their serial output compared to a committed `.out` file **exactly** — CRLF
 included — plus the exit code passed to `INT 21h AH=4Ch`.
 
@@ -344,7 +347,7 @@ compiler: **does the 8086 have this instruction at all?**
             value conditions  : = x6  <> x2  < x5  >= x3  <= x2  > x16
             branch conditions : IF / REPEAT x9  CASE x2  FOR downto x1  FOR to x3
             runtime: 616 bytes, 330 swept, 0 0F-prefixed
-            program code: 39 of 39 fixtures swept end to end, 4603 bytes
+            program code: 41 of 41 fixtures swept end to end, 4963 bytes
             t33_cmpops: 13 comparisons matched against their source operators, in order
             clause H: 8 of 8 fixtures matched the branch conditions read off their source
 ```
@@ -587,7 +590,7 @@ checker.
 compile to `.COM` images, are booted on a floppy by a 512-byte hand-assembled
 boot sector, and are compared **byte for byte** against the exact output the
 fixture demands — `tests/run_com_exec.py`, wired into `run_all.sh`, 21/21 at
-that tag and 39/39 now.
+that tag and 41/41 now.
 
 Everything below is about establishing what *can* be believed, because the
 first attempt at this used an emulator that was wrong, and a wrong oracle is
@@ -1172,7 +1175,7 @@ Details that are deliberate, not incidental:
   prove the *path* exists (compile → poke → run → report → no file written),
   which it does with eight assertions. The other 37 are covered by
   `run_exec86.py`, which calls the same interpreter directly.
-- **Every fixture that compiles is now also run.** 39 of 44 execute; the 5 that
+- **Every fixture that compiles is now also run.** 41 of 46 execute; the 5 that
   do not are `t14` and `t25` (`ENoLib`, by design), `t38` (a boolean read,
   refused with TP3's error 66, by design), `t43` (a program whose code exceeds
   the 4 KiB window, refused with error 98, by design) and `uierror` (a
@@ -1990,10 +1993,12 @@ independently-scanned inventory at all.
 
 ## Next steps
 
-1. The rest of the string language. `v-TP3-STRVAR` covers `s := 'literal'`
-   and `writeln(s)`; `v-TP3-STRREAD-ASSIGN` covers `readln(s)`/`read(s)` (TP3's
-   `xrdstr`) and `s := t` between two string variables. Still `ENoLib`:
-   `s [i]` indexing and the `+` / comparison operators.
+1. The rest of the string language. `v-TP3-STRVAR`/`v-TP3-STRREAD-ASSIGN` cover
+   string variables, `s := 'literal'`, `writeln (s)`, `readln (s)`/`read (s)`
+   and `s := t`; `v-TP3-STRINDEX` covers `s[i]` with a **constant** index —
+   read, write and `readln (s[i])`. Still `ENoLib`: a *variable* index `s [i]`
+   (the runtime address arithmetic TPSRC9 `indstr`/`ialoop` does), and the `+`
+   / comparison operators (`cmpstr`/`xconcat`, TPSRC2).
 2. Nested procedures / recursion, `var` parameters (the `SEG:OFF` push from
    RESUME-TP3.md §3.11), range/index checks (`TU_RANGE_CHECK`,
    `TU_INDEX_CHECK`), typed constants (RESUME-TP3.md §3.14), `array` at its

TP3-COMPILER.md

@@ -146,6 +146,30 @@ than 4 KiB of inline string bytes and must report `ERR 98`; with the gate
 disabled the same fixture compiles `OK` -- the corrupt image -- which is the
 proof the test is not vacuous.
 
+## String subscripts: `s[i]`, and a latent `boff` clobber
+
+A `string [n]` is a length byte at `s[0]` followed by the characters, so
+`s[i]` is the single byte at `off+i`.  `ParseSub` folds a constant index into
+`boff` (the same field an array uses) and drops `r.cls` to `TChar`, and then:
+
+- `LoadAtom` loads one byte (`A0 off` / `8A 46 disp`) and zeroes `AH`;
+- `Assignment` stores one byte when the symbol is a `TString` but `r.cls` is
+  `TChar` (the element) rather than `TString` (the whole string);
+- `IoCall` already classified on `arg.cls`, so `writeln (s[i])` picked `wrchar`;
+- the reader now classifies `read (s[i])` the same way and pushes `off+boff`.
+
+The bug this uncovered had been in the tree the whole time: `ParseAtom` ended
+its variable arm with
+
+    ParseVar (r) ;
+    r.boff := 0 ;       (* discard the subscript ParseSub just folded in *)
+
+`ParseVar` already sets `boff := 0` before calling `ParseSub`, so the reset was
+pure loss — `s[i]` (and any constant array subscript) addressed `off+0`, the
+length byte.  Nothing caught it because arrays cannot be declared yet, so no
+fixture had ever exercised the fold.  `t44_strindex` reads `off+0` when the
+reset is restored (wrong output, exit 0) and the right bytes when it is gone.
+
 ## Tests
 
 Several harnesses live in `shell/tests/` -- in the repo on purpose, because
@@ -196,13 +220,13 @@ byte is a perfectly well-formed instruction that does something else, so no
 amount of inspecting the compiler's intent will catch it.  This harness closes
 that gap the only way it can be closed -- by running the thing.
 
-For each of 39 fixtures it compiles a `.COM`, writes a 1.44 MB floppy with the
+For each of 41 fixtures it compiles a `.COM`, writes a 1.44 MB floppy with the
 image in it, boots `qemu-system-i386 -fda disk.img -serial out.txt`, and
 compares `out.txt` to the committed `tests/fixtures/tNN.out` **exactly** --
 CRLF included -- plus the exit code the program hands to `INT 21h AH=4Ch`.
 
 ```
-cd shell && python3 tests/run_com_exec.py            # all 39
+cd shell && python3 tests/run_com_exec.py            # all 41
 cd shell && python3 tests/run_com_exec.py --show     # print the serial file
 cd shell && python3 tests/run_com_exec.py --rebless  # rewrite the .out files
 ```
@@ -231,7 +255,7 @@ partial read or a wrong sector count would have been visible, this was not.
 window at `C000h`; and the copy is executed code, so it is the last thing
 that touches the image and no BIOS call follows it.
 
-**39 of 44 fixtures execute.**  5 do not: `t14` and `t25` (`ENoLib`, by
+**41 of 46 fixtures execute.**  5 do not: `t14` and `t25` (`ENoLib`, by
 design), `t38` (a boolean read, refused with TP3's error 66, by design),
 `t43` (a program whose code exceeds the 4 KiB window, refused with error 98,
 by design) and `uierror` (a deliberate syntax error).  The nine control-flow fixtures (`t08`

shell/Compiler.mod

@@ -1648,6 +1648,7 @@ PROCEDURE LoadAtom (VAR r : ERes) ;
    a counted string where a 16-bit word is expected.  Reporting it here rather
    than in the parser means writeln('hi') still works - IoCall handles a
    literal before it ever calls LoadAtom. *)
+VAR emsz : CARDINAL ;
 BEGIN
    IF r.cls = TString THEN
       Err (ENoLib) ;                    (* string value used as a number *)
@@ -1658,13 +1659,20 @@ BEGIN
       EmMovAxi (W16 (r.imm)) ;
       r.kind := 2
    ELSIF r.kind = 1 THEN
-      IF symtab [r.idx].size > 2 THEN
+      IF (symtab [r.idx].size > 2) AND NOT
+         ((r.cls = TChar) AND (symtab [r.idx].cls = TString)) THEN
          Err (ENoLib)
       ELSE
+         (* s[i] indexes one byte of a string, not the string, so its width is
+            one whatever the variable's declared width is. *)
+         emsz := symtab [r.idx].size ;
+         IF (r.cls = TChar) AND (symtab [r.idx].cls = TString) THEN
+            emsz := 1
+         END ;
          EmLoadVar (symtab [r.idx].local,
                     (symtab [r.idx].off + r.boff) MOD 10000H,
-                    symtab [r.idx].size) ;
-         IF symtab [r.idx].size = 1 THEN
+                    emsz) ;
+         IF emsz = 1 THEN
             EmMovAh0 ()
          END ;
          r.kind := 2
@@ -1692,6 +1700,21 @@ BEGIN
          IF symtab [r.idx].cls = TArray THEN
             r.boff := W16 (VAL (LONGINT, r.boff)
                            + t.imm * VAL (LONGINT, symtab [r.idx].elem))
+         ELSIF symtab [r.idx].cls = TString THEN
+            (* s[i]: a STRING is a length byte at s[0] followed by the
+               characters (TPSRC8's `string' type), so index i selects the one
+               byte at off+i.  A constant index folds into boff exactly as an
+               array's does, and r.cls drops to TChar so every consumer below
+               - LoadAtom, Assignment, IoCall - sees a one-byte value instead
+               of the whole string.  A second subscript on a character is not
+               a type.  A variable index reaches here only after failing the
+               `t.kind # 0' test above and is ENoLib, same as an array's. *)
+            IF r.cls = TChar THEN
+               Err (ESimpType) ;
+               RETURN
+            END ;
+            r.boff := W16 (VAL (LONGINT, r.boff) + t.imm) ;
+            r.cls := TChar
          ELSE
             Err (ESimpType) ;
             RETURN
@@ -2145,8 +2168,13 @@ BEGIN
       r.cls := symtab [idx].cls ;
       RETURN
    ELSIF (symtab [idx].tag = KVar) OR (symtab [idx].tag = KType) THEN
+      (* ParseVar initialises boff to 0 and then ParseSub folds any constant
+         subscript into it.  Do NOT reset boff here: doing so after ParseVar
+         returns threw away the subscript, so s[i] read/wrote off+0 (the
+         length byte) while still reporting itself as a char.  This was
+         latent for arrays because an array cannot be declared yet, so no
+         fixture had ever exercised the fold. *)
       ParseVar (r) ;
-      r.boff := 0 ;
       RETURN
    ELSE
       Err (EUnknown)
@@ -2301,7 +2329,7 @@ BEGIN
       Err (EUnknown) ;
       RETURN
    END ;
-   IF symtab [r.idx].cls = TString THEN
+   IF (symtab [r.idx].cls = TString) AND (r.cls = TString) THEN
       (* s := <string literal>.  TPSRC8 asn2 sends a string through the scalar
          store path, and estcpl's string arm emits `MOV CL,max_len' (max_len =
          varsize-1) then `CALL strstore', which copies the source - length byte
@@ -2356,7 +2384,18 @@ BEGIN
       RETURN
    END ;
    IF symtab [r.idx].size > 2 THEN
-      Err (ENoLib) ;
+      IF (r.cls # TChar) OR (symtab [r.idx].cls # TString) THEN
+         Err (ENoLib) ;
+         RETURN
+      END ;
+      (* s[i] := c : one byte at off+boff.  The whole-string path above was
+         skipped because r.cls is TChar, not TString, so this is the element
+         store; a plain char variable has size 1 and takes the scalar path
+         below instead. *)
+      ParseExpr (src) ;
+      LoadAtom (src) ;
+      EmStoreVar (symtab [r.idx].local,
+                  (symtab [r.idx].off + r.boff) MOD 10000H, 1) ;
       RETURN
    END ;
    ParseExpr (src) ;
@@ -2444,7 +2483,17 @@ BEGIN
                   RETURN
                END ;
                acls := symtab [arg.idx].cls ;
-               EmPushVarAddr (symtab [arg.idx].local, symtab [arg.idx].off) ;
+               (* s[i] is a CHAR variable, so read(s[i]) must read one byte at
+                  off+boff, not the whole string at off.  arg.cls is TChar
+                  (set by ParseSub) while the symbol's own class is still
+                  TString, which is exactly how the element is told from the
+                  whole string; the address push has to carry boff.  For a
+                  plain variable arg.boff is 0, so nothing changes. *)
+               IF (acls = TString) AND (arg.cls = TChar) THEN
+                  acls := TChar
+               END ;
+               EmPushVarAddr (symtab [arg.idx].local,
+                              (symtab [arg.idx].off + arg.boff) MOD 10000H) ;
                IF acls = TString THEN
                   (* readln(s)/read(s) of a string variable: TPSRC8 prdstr
                      pushes the address, emits MOV CL,max_len and CALL

shell/tests/fixtures/expected.tsv

@@ -307,3 +307,20 @@ uierror	ERR	41	331
 #                   writeln cases below embeds 240 bytes inline (wrtinl), so the
 #                   overflow lands inside one of them at pos 4978.
 t43_codewindow	ERR	98	4978
+#   t44_strindex    s[i] read and write on a string, as a CHAR.  A STRING is a
+#                   length byte at s[0] then the characters, so s[i] is the one
+#                   byte at off+i; a constant index folds into boff exactly as
+#                   an array's would, and r.cls drops to TChar so LoadAtom,
+#                   Assignment and IoCall all treat it as one byte.  Only a
+#                   constant index is accepted (a variable index is ENoLib, as
+#                   it already was for an array).  data=16 is the global
+#                   string[10] (11 bytes), the char c (1) and two @@T temps.
+#                   The fix also had to drop ParseAtom's `r.boff := 0` after
+#                   ParseVar, which had been discarding every subscript.
+t44_strindex	OK	258	16
+#   t45_strindex_read  readln (s[i]) reads one character into position i; the
+#                   reader classifies by arg.cls (TChar for an indexed string)
+#                   and pushes off+boff.  data=15 is the global string[10]
+#                   (11 bytes) plus two @@T temps.  t45_strindex_read.in feeds
+#                   'X' then 'Y'.
+t45_strindex_read	OK	134	15

shell/tests/fixtures/t44_strindex.out

@@ -0,0 +1,5 @@
+hel
+o
+Hello
+abc
+Hcllo

shell/tests/fixtures/t44_strindex.pas

@@ -0,0 +1,30 @@
+program t44;
+(* String subscripts, s[i], in both directions.
+
+   A STRING is a length byte at s[0] followed by the characters, so s[i]
+   selects the single byte at off+i: reading it yields a CHAR, and assigning to
+   it stores one byte.  Both a global and a local string are exercised, because
+   the address paths differ (absolute vs [BP+off]).  Only a CONSTANT index is
+   covered: a variable index is ENoLib here, exactly as it already is for an
+   array. *)
+var
+  g : string[10] ;
+  c : char ;
+procedure p ;
+var
+  l : string[6] ;
+begin
+  l := 'abc' ;
+  writeln (l[1], l[2], l[3]) ;
+  g[2] := l[3]
+end ;
+begin
+  g := 'hello' ;
+  writeln (g[1], g[2], g[3]) ;
+  c := g[5] ;
+  writeln (c) ;
+  g[1] := 'H' ;
+  writeln (g) ;
+  p ;
+  writeln (g)
+end.

shell/tests/fixtures/t45_strindex_read.in

@@ -0,0 +1,2 @@
+X
+Y

shell/tests/fixtures/t45_strindex_read.out

@@ -0,0 +1,2 @@
+aXcde
+XY

shell/tests/fixtures/t45_strindex_read.pas

@@ -0,0 +1,17 @@
+program t45;
+(* readln (s[i]) : reading into one character of a string.
+
+   s[i] is a CHAR variable, so the read must store one byte at off+i and the
+   address push has to carry the subscript.  Before this, the reader classified
+   by the SYMBOL's class (TString) and pushed off, so readln(s[i]) read a whole
+   line into s from its start instead.  The two reads here land at two
+   different positions, so either one being wrong changes the output. *)
+var
+  s : string[10] ;
+begin
+  s := 'abcde' ;
+  readln (s[2]) ;
+  writeln (s) ;
+  readln (s[1]) ;
+  writeln (s[2], s[1])
+end.