# 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`.

### `run_com_exec.py` -- the emitted image, on a CPU

The byte-level checks above all stop at "the compiler produced what it
intended to produce", which is where this project's bugs live: a wrong ModRM
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 21 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 21
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
```

`--rebless` exists because 21 golden files will eventually need re-basing, and
re-basing by hand is how a red test is quietly made green.  The rule this
project follows instead: **re-baseline deliberately, never to turn a red test
green.**  If a fixture's expected output changes, the reason goes in
`expected.tsv` or in `SUMMARY.md` first.

`shell/tests/exec/bootcom.s` is the boot sector: 512 bytes, reads the `.COM`
off the same floppy with `INT 13h`, sets `DS=ES=0`, `SS=2000h SP=2004h`, builds
the three GDT descriptors a `.COM` expects, installs an `INT 21h` shim for
`AH=02h/09h/08h/4Ch` to the serial port, and `JMP 0000:0100`.  It is
reusable, and `rt_exec.py` is meant to reuse it rather than grow a second boot
path.

**21 of 33 fixtures execute.**  3 do not compile, and 9 compile without a
`.out` file -- and those 9 are the control-flow fixtures (`t08` const, `t09`
if, `t10` while, `t11` for, `t12` repeat, `t13` procedure, `t15` label, `t27`
five locals, `t28` 70 parameters).  They have byte-level checks and no
behavioural check.  That is the next gap to close, and it is a gap in the
*tests*, not in the compiler.

What execution found that no byte-level check could: the `FOR` loop was
post-tested, `EXIT` targeted the increment, procedure bodies executed as part
of the main body, and the procedure-skip jump's patch slot could not be
distinguished from "no jump" because slot 0 is legitimate.  All four had
plausible byte counts.  See `SUMMARY.md` for the full list.

### 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 while nothing executed the image, which is exactly
why it survived so long.  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 -- and `t10_while` is one of the nine
fixtures that *still* do not execute, so the claim is byte-level only.

## Codegen bug class: a patch slot of 0 is not "no patch"

`EmJmpNear (target)` does not emit a branch -- it emits one and returns the
**index of a patch slot** in the image's fixup list, because the target is not
known yet when the branch is emitted.  `Compile` stores that index in
`overProc` and patches it later with `SetPatTgt (overProc, pc)`.

The first version used `overProc := 0` as the "there was no such jump"
sentinel, and **patch slot 0 is a perfectly valid slot**.  So

```
IF overProc # 0 THEN SetPatTgt (overProc, pc) END ;
```

skipped the patch for any program whose procedure-skip jump happened to be the
*first* patch in the image.  The jump kept its placeholder target of 0, landed
at image offset 0 -- which is the entry `JMP` -- and looped forever.  `t31_procparam`
did not fail, it **hung**.

The fix is a separate `hasProc : BOOLEAN`, and the rule generalises: **a
plausible placeholder is indistinguishable from a real value.**  `0` as "no
target", `-1` as "unbounded", `0` as "flag not set" are all correct until the
first real value is 0.  Reach for a `BOOLEAN`; it has no collision to have.

## Codegen: procedure bodies are emitted inside the caller's main body

The compiler emits a procedure's body *between* the caller program's prologue
and its own main body, so a jump over it is mandatory -- without it the main
body runs straight into the procedure's code.  `DeclaresProc ()` answers "does
this program declare any procedure at all?" with a save/restore-`srcPos`
lookahead that steps over `:` and `;` and stops at `srcLen`; `Compile` then
emits `EmJmpNear (0)` after the prolog when the answer is TRUE, and patches it
once `DefPart` has emitted the bodies.

This is why four fixture code sizes grew by exactly 3 bytes (the `E9` plus its
rel16): `t13_proc` 53->56, `t27_localvar` 122->125, `t28_farparam` 123->126,
`t31_procparam` 66->69.  A size change in a pinned matrix is a claim that has to
be justified, and each of these is written into `expected.tsv`.

`DeclaresProc` reads lookahead characters with

```
ch : CHAR ; ch := GetCh ()
```

rather than a bare `GetCh ()`, because `gm2 -fiso` rejects an ignored function
result -- a rule that has bitten this project in several unrelated places.


## 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`.

## gm2 / ISO trap: `AND` is not short-circuit, and a lookahead must not match

Two rules that produced the same class of bug:

- `IF MatchKey (tok) AND (tok = TkVar) THEN` -- `MatchKey` **consumes** the
  word it recognises, and `AND` evaluates both sides.  So this is not a
  lookahead, it is a parse that eats the very token being tested for.  The
  shape that works is `PeekKw (tok)` (which save/restores `srcPos`) for the
  question, and `DropB (MatchKey (tok))` only on the branch that commits.
  This is the "Alpha test without `Skip`" family above, one level up: there,
  the cursor was not past whitespace; here, it was past the token.
- `IF <stmt>` may not be the last thing in a compound statement.  It is a
  compile error under `-fiso`, and the error points at the `END`, not at the
  `IF`.

## gm2 trap: an implementation module must not re-declare its own `.def` CONST

`Runtime.def` declares `LoadBias = 100H` because `Compiler.mod` needs it too.
`Runtime.mod` must then **not** declare it again -- duplicating a CONST that
the definition module already exported is an error, and the message points at
the duplicate, not at the `.def`, so it reads like a redeclaration problem
rather than "this already exists upstream".

## `gm2 -fiso` rejects a dropped function result

`GetCh ()` as a statement, or any call whose result is discarded, is an error.
Hence the `DropCh`/`DropB`/`DropC` helpers throughout, and hence
`ch : CHAR ; ch := GetCh ()` in `DeclaresProc`.  There are ~39 such call sites
and every one of them is a place where a reader might "simplify" the `DropB`
away.

*Generated: 2026-09-30*