# TP3-comp — Turbo Pascal 3 clone in GNU Modula-2 (ISO)

**Status:** compiler-core (`Compiler.mod`) + shell compiled clean with
`gm2 -fiso -Wall`, and the ISO shell `tpshell` builds and runs (ESC-key
driven, TP3-style DOS screen redraws).  This is the working *implementation*
companion — the human overview is `TP3-COMPILER.md` history summary and
`RESUME-TP3.md`.

## Build (proven, two-phase — the ONLY recipe that beats GNU gm2's
pass-3 "too many errors / too many identifiers" cap on compiler-sized
whole-program programs)

```
gm2 -fiso -c Compiler.mod          # phase A: small per-module pass
gm2 -fiso -fgen-module-list=modules.lst -o /dev/null \
    Compiler.mod Term.o TextBuf.o Posix.o Editor.o  # phase B1: build list
gm2 -fiso -fuse-module-list=modules.lst -o tpshell \
    Shell.mod Compiler.mod Term.o TextBuf.o Posix.o Editor.o  # phase B2: link
```
(Equivalent make target `tpshell` in `shell/Makefile`.) The single `gm2 -o`
whole-program pass 3 silently caps identifier/error counts — see
`build_tpshell.sh`.)

## What was changed inside `Compiler.mod` (all ISO-legal, gm2-verifiable)

1. **CHAR vs hex-int comparisons** (lines ~829, ~1086): Modula-2 `CHAR` is
   not the ZType; `ch = 09H` → `ORD (ch) = 09H` (and likewise 0DH/0AH/0CH).
2. **Return-value discard** (ISO forbids drop in statements; gm2 treats as
   error and has no `-Wno-`): added `DropCh/DropB/DropC` discard helpers
   and wrapped ~39 statement call sites of `GetCh`, `MatchKey`, `NewSym`,
   `EmCall`, `EmJmpNear` (no output reground clutter; TP3 TP5 `cascadepos`
   `waitkey` semantics preserved via the editor's ESC wait).
3. **`AND`/`OR`/`NOT` on W16-folded CARDINAL** (lines ~1233/1246/1454) are
   illegal on ZType in gm2 → added `BitAnd/BitOr/BitNot` (BITSET `*`/`+`/`-`
   fold, VAL-projected back to LONGINT) and rewrote the operands.
4. **`Compile` error-return params** aligned to `Compiler.def`
   (`errNo`/`errPos`, was `eNo`/`ePos`) — gm2 ISO rejects a disparate name
   in the proper procedure.
5. **`HALT` -> `HALT (0)`** in `Shell.mod`.  A bare `HALT` (no operand)
   aborts under `gm2 -fiso`: a minimal repro exits 134 (SIGABRT) with a
   bare `HALT` but 0 with `HALT (0)`.  Quitting the shell died on SIGABRT.
6. **Module-level `errNo` renamed `errNum`.**  `Compile`'s formal `errNo`
   shadowed the module variable, so `errNo := errNo` was a *self*-assignment
   and every error code reached the caller as 0.

The compiler's whole-program 8086-emit, TEXT-equivalent, pastres and
errexit flow are otherwise unchanged (mirrors TP3 TPSRC file, ConvertTP3).

## Lexer/parser bug class: "Alpha test without Skip"

The single largest source of wrong behaviour.  `Skip ()` is what folds
blanks, `{ }` and `(* *)` comments, and `CR`; neither `PeekKw` nor
`MatchKey` leaves the cursor past it (`PeekKw` save/restores `srcPos`,
`MatchKey` stops right after the word it matched).  So a routine entered
*immediately after a keyword* sees the CR/blank that follows that keyword,
and any

```
IF NOT Alpha (CurCh ()) THEN Err (EUnknown) ; RETURN
```

fires with `EUnknown` (41) pointing at a blank.  That is why every program
used to fail on `program`'s program name, and why `writeln(...)` failed on
its `(`.

Fixed by adding the missing `Skip ()` at each entry point (it is idempotent,
so it is safe even where a `Skip` already ran):

| routine | entered after |
|---|---|
| program header | `PROGRAM` |
| `DefVar` | `VAR` |
| `DefConst` | `CONST` |
| `DefType` | `TYPE` |
| `ParseType` | `:` / `=` |
| `ProcFunc` (name) | `PROCEDURE` / `FUNCTION` |
| `ProcFunc` (parameter name) | `(`, or a var-parameter's `VAR` |
| `Statmnt`, `TkFor` branch | `FOR` |
| `Statmnt`, `TkGoto` branch | `GOTO` |

`DefLabelPart`, `MatchKey`, `MatchDelim` and `MatchAssign` already had theirs.

Three related defects fixed alongside:

- `ProcFunc`'s parameter loop used `MatchKey (tok) AND (tok = TkVar)` to
  detect a var-parameter.  `MatchKey` **consumes** the word it reads, so
  using it as a lookahead ate the parameter's own name.  Replaced with
  `PeekKw` (non-consuming), consuming with `DropB (MatchKey (tok))` only on
  a `VAR` hit.
- `ProcFunc` called `ParseType` straight after the parameter name without
  consuming the `:` of `name : type`; added `ExpectDelim (':', ENoSemi)`.
- `ParseLabelStmt` consumed `n :` but never parsed the statement the label
  is attached to, so `Compound` then demanded a `;` that does not exist in
  `1: x := 1`.  `Statmnt` now parses the labelled statement.

## Tests

Several harnesses live in `shell/tests/` -- in the repo on purpose, because
`/tmp` is wiped between sessions and an earlier /tmp-only harness was lost
with it.  `shell/tests/run_all.sh` runs them all; its header says which is
which.  `shell/SUMMARY.md` (at the repo root) is the state-of-the-project
document.

### `run_compile_tests.sh` -- compiler front end, no pty, instant

`tests/CompileTest.mod` links `Compiler` + `TextBuf` + `Posix` and reads
fixture paths from **stdin**, one per line.  For each it does exactly what
the shell's `LoadWorkFile` does (LF -> CR normalisation, `^Z` ends the
text), calls `Compile`, and prints a one-line verdict plus a source excerpt
with a caret under the error position.  It rebuilds `Compiler.o` when the
source is newer, so an edit is picked up automatically.

```
cd shell && tests/run_compile_tests.sh            # all fixtures
cd shell && tests/run_compile_tests.sh /some/dir  # another fixture set
```

**The fixture matrix is not written here.**  It used to be, and it went stale
the moment it was copied -- a second copy of a table is a table that lies.
The live one is `shell/tests/fixtures/expected.tsv`, which the runner *asserts*
against: per fixture it pins the verdict plus the code size plus the data size
(or the error number plus the error position), and the rationale for every
re-baselining is written in the file rather than in a commit message.  Read that
for what compiles; this document for how.

The two fixtures that fail are `t14` (`array [1..5] of integer` at its point of
use) and `t25` (a string literal used as a value, `s := 'hi'`), both
`ENoLib` (102) -- the original's own "not implemented" path, and pinned on
purpose so that implementing them is a deliberate change to a test rather than
an accident.  `uierror` is a deliberate syntax error used by `uitest.py`.

### Hex-dumping the emitted image

A line reading `@dump` on stdin switches on a hex dump of the emitted 8086
code (via the new `Compiler.CodeByteAt`), which is how the `rel16` off-by-2
below was found -- sizes alone could never have shown it:

```
cd shell
printf '@dump\ntests/fixtures/t19_int1.pas\n' | ./compiletest
```

```
0000:  01 00 02 00 10 00 00 00 00 00 10 00 00 00 00 00
0010:  E8 F5 FF 8B EC B8 01 00 50 E8 04 00 83 C4 02 E8
0020:  1E 00 33 C0 E8 E9 FF
```

which reads: header words (CS=1, DS=0x10 = 256/16, 16 open files),
`CALL 8H` (initmem), `MOV BP,SP`, `MOV AX,1`, `PUSH AX`,
`CALL 20H` (WrInt), `ADD SP,2`, `CALL 40H` (WrLn), `XOR AX,AX`,
`CALL 10H` (ProgEnd).

### `uitest.py` -- shell/editor behaviour that only exists interactively

Drives a real pty (`ptyharness.py` supplies read-until-quiet, key sending
and a small VT100 emulator) and asserts the TP3 **compile-error jump**:
`W` load -> `C` compile -> `ESC` -> the editor opens with the cursor exactly
on the error position -> `Ctrl-K D` back to the menu -> `Q` exit 0.
10/10 pass on `uierror.pas`, and the reported error is 41 -- i.e. the
`errNo` self-assignment fix is now proven through the real UI.  The
fixture carries a deliberate *syntax* error (`x := 1 + ;` -> error 41 at
line 9, column 12) rather than a missing library feature, on purpose: the
latter move as the compiler grows, and a UI test whose expectations drift
with it stops being a test.

The jump mirrors original TP3 `kcwait` + `editor2`
(`Resources/turbopascal3source/TP3/TPSRC5:333-336` and `:919`):
`waitesc; BX:=txerrpos; DEC BX; JMP editor2`, where `editor2` then does
`ADD BX,txbeg; INC BX` -- the `DEC`/`INC` cancel, so the net position is
`txbeg+txerrpos` = our 0-based `errPos`.  It is armed as a sticky position
(`Editor.GotoOffset`) instead of by changing `Run`'s signature, so
`Editor.def` stays additive.

Note `LoadWorkFile` ends with a `Pause`, so a driver must send one filler
key after the path; skipping it desynchronises every later keypress.

## Standard procedures (the builtin table)

`Inittur` defines six predefined *types* (INTEGER/BYTE/CHAR/BOOLEAN/REAL/
STRING), TRUE/FALSE and two temporaries, plus now five **procedures** via
`DefBuiltins`: WRITE, WRITELN, READ, READLN, HALT.  Before this, `WRITELN`
was simply absent from the symbol table, `Statmnt`'s identifier branch
failed its `Search`, and every program that printed anything died with
`EUnknown` (41) on the `(` after the call name.

They are tagged `KBuiltin`, not `KProc`, because they are not called
generically.  TP3 (TPSRC8 `pwriteln` / `pwrloop` / `prdtyped`) does **not**
hand the runtime a descriptor: it inspects each argument's class and emits a
*different call per type*, so the formatting is fixed at compile time and the
runtime only ever sees a value.  `IoCall` mirrors that:

```
writeln(1)      ->  MOV AX,1 ; PUSH AX ; CALL 20H ; ADD SP,2 ; CALL 40H
writeln('a')    ->  MOV AX,'a' ; PUSH AX ; CALL 28H ; ADD SP,2 ; CALL 40H
writeln(1,'a')  ->  ... CALL 20H ... CALL 28H ... CALL 40H
readln(x)       ->  LEA AX,[0104] ; PUSH AX ; CALL 48H ; ADD SP,2 ; CALL 60H
```

`TU_WrInt/Char/Bool/Real`, `TU_WrLn`, `TU_RdInt/Char/Bool`, `TU_RdLn` and
`TU_Halt` are new image-base entry constants continuing the existing `TU_*`
space (`TU_InitMem=8H`, `TU_ProgEnd=10H`, `TU_StackChk=18H`).  READ/READLN
push the *address* of the variable (new `EmPushVarAddr`: `8D 46 disp` /
`8D 06 off`) so the runtime can store; a non-variable argument is `ETypeErr`
(56), as in TP3.

  *Superseded.*  Both the `TU_*` offsets and the `8D 46 disp` encoding changed
  after this was written.  The runtime is now prepended to the image and the
  `TU_*` values are assigned from `Runtime.RT_Entry` rather than written down;
  and `EmPushVarAddr` for locals was emitting the wrong base register
  (`8D 46` is `[SI+disp8]`, not `[BP+disp8]`) while truncating the
  displacement to a byte.  `EmBpDisp` now owns that choice -- disp8 iff
  `off <= 127`, else disp16 -- and is shared by `EmLoadVar`, `EmStoreVar` and
  `EmPushVarAddr`.  See `SUMMARY.md`, "Bugs found by actually running code"
  item 4, and `shell/tests/check_framedisp.py`.

A **single-character** literal needed care.  `RdConst` reports `'a'` as
`TScalar` (its char code 97) and only longer literals as `TString`.  That is
right for `c := 'a'` but would make `writeln('a')` print **97**, so
`ERes` grew a `chr` flag, set in `ParseAtom` and honoured in `IoCall`, which
then picks the char entry.  Without it `writeln('a')` compiled cleanly and
printed the wrong thing -- a green test lying, which is worse than a failure.

Still pending, and deliberately reported rather than faked: the **runtime
blob** itself (`CmdRun`, the interpreter, and the linker that rebases these
entry offsets by the runtime's size) and the **string** runtime, so
multi-character literals still raise `ENoLib` (102).  `ENoLib` is also the
path for real/set/record/file and for any type wider than 2 bytes.

## Codegen bug: every direct CALL/JMP was 2 bytes long

Found by dumping the emitted image, not by size.  `EmCall`, `EmJmpNear` and
`EmJcc` computed their `rel16` as

```
rel := (target + 10000H - pc) MOD 10000H
```

but at that point `pc` already points *past the opcode and at the
displacement field* -- the instruction does not end until `pc + 2`, and
x86 measures `rel16` from the end of the instruction.  So every
resolved-immediately branch and jump landed 2 bytes past its target.
`ResolvePatches`, used for the *forward* patched path, already had it right
(`target - (place + 2)`), which is why forward gotos looked fine and
backward ones did not.  All three now use `pc + 2`.

This was silently harmless so far only because nothing executes the image
yet.  Verified on `t10_while`: the `JZ` forward patch lands on the
instruction after the loop, and the `JMP` back-edge now lands exactly on the
loop head instead of 2 bytes into it.


## gm2 pitfall: `EXIT` inside the program-header `WHILE` crashes pass 3

Adding an `EXIT` to the program-header parameter loop (to guard against
non-advancing input on malformed input such as `program p(1;)`) ICEs gm2:

```
internal compiler error: Abandon
  ... ExitStatement -> PopExit -> M2StackWord_PopWord -> invalidloc
```

Extracting the loop into its own procedure did **not** help.  So the header
fix is deliberately exactly one added `Skip ()` and no `EXIT`.  If that loop
must be hardened, use a `BOOLEAN` "advanced" flag, never `EXIT`.

*Generated: 2026-09-26*