Преглед изворни кода

fix(generics): lower each unit in its own module scope

Two instantiations of one generic with arguments from different families
failed on the second.  Root cause was in Lowering, not the formal
binding: after EndUnit, Lower re-emitted each unit in globScope, so
MaterializeGlobals copied the unit's types there; a generic instance
leaked its NodeRec and the second instance's 'POINTER TO NodeRec'
resolved the first instance's record via ForwardType.

- SymTab.EnterLowerScope/LeaveLowerScope; Lower re-emits each unit in
  its own module scope (also around the ExitCode probe in EndSession)
- Lower.ReenterImports re-materialises a nested module's FROM-imports
  in the enclosing scope (its scope is popped before lowering)
- regression: compiler/tests/g_twice{,i,c,p} (GTwiceProg, 42)
- docs: analysis_gaps.md 10 (trail), STATUS, next-session handoff

Gates: suite 240/240, fixpoint OK (3,821,110 bytes), corpus 405/612,
0 crashes.
Eric Streit пре 3 дана
родитељ
комит
dc3b3c847b

+ 3 - 0
compiler/run_tests.sh

@@ -316,6 +316,9 @@ expect_run_files GCBoxProg 42 \
   g_gcbox.def g_gcbox.mod g_gcint.def g_gcint.mod g_gcprog.mod
 expect_run_files GStkProg 42 \
   g_gstk.def g_gstk.mod g_gstki.def g_gstki.mod g_gstkp.mod
+expect_run_files GTwiceProg 42 \
+  g_twice.def g_twice.mod g_twice_i.def g_twice_i.mod \
+  g_twice_c.def g_twice_c.mod g_twice_p.mod
 expect_run_files WrapProg 42 \
   ../runtime/syslib/SysShim.def ../runtime/syslib/SysShim.mod \
   ../stdlib/iochan.def ../stdlib/iochan.mod \

+ 57 - 1
compiler/src/Lower.mod

@@ -2405,6 +2405,51 @@ PROCEDURE ReenterGlobals (seq: AST.Node);
     END
   END ReenterGlobals;
 
+PROCEDURE ReenterImports (seq: AST.Node);
+(* A nested module's FROM-imports live in its (now popped) scope.  Copy
+   them into the enclosing scope so lowering the nested module's
+   declarations and body can still resolve the imported names. *)
+  VAR i, j: CARDINAL;
+    c, cn: AST.Node;
+    mod, nm: SymTab.Name;
+  BEGIN
+    IF seq = AST.NoNode THEN RETURN END;
+    i := 0;
+    WHILE i < AST.NChild(seq) DO
+      c := AST.Child(seq, i);
+      IF i = AST.MaxChild - 1 THEN ReenterImports(c)
+      ELSIF AST.Kind(c) = AST.NkDeclSeq THEN ReenterImports(c)
+      ELSIF (AST.Kind(c) = AST.NkImport) AND (AST.NChild(c) >= 1) THEN
+        Name(AST.Child(c, 0), mod);
+        IF SymTab.Equal(mod, "libc") THEN
+          j := 1;
+          WHILE j < AST.NChild(c) DO
+            cn := AST.Child(c, j);
+            IF AST.Kind(cn) = AST.NkIdent THEN
+              Name(cn, nm);
+              IF SymTab.DeclareCProc(nm) THEN NoteCProc(nm) END
+            END;
+            INC(j)
+          END
+        ELSIF SymTab.ModKnown(mod) THEN
+          j := 1;
+          WHILE j < AST.NChild(c) DO
+            cn := AST.Child(c, j);
+            IF AST.Kind(cn) = AST.NkIdent THEN
+              Name(cn, nm);
+              (* plain IMPORT lists modules; only FROM adds names *)
+              IF SymTab.SymKind(nm) # SymTab.KindModule THEN
+                IF NOT SymTab.ImportFrom(mod, nm) THEN END
+              END
+            END;
+            INC(j)
+          END
+        END
+      END;
+      INC(i)
+    END
+  END ReenterImports;
+
 PROCEDURE EmitVarNames (c: AST.Node);
 (* Emit DeclVar for each name of an NkVarDecl; the names are a chunked
    sequence (AstAppend), so follow NkBlock continuations. *)
@@ -2482,6 +2527,7 @@ PROCEDURE LowerDecls (seq: AST.Node);
         END
       ELSIF AST.Kind(c) = AST.NkUnit THEN
         Name(AST.Child(c, 0), nm);
+        ReenterImports(AST.Child(c, 3));
         ReenterGlobals(AST.Child(c, 1));
         LowerDecls(AST.Child(c, 1));
         IF AST.Child(c, 2) # AST.NoNode THEN
@@ -2661,18 +2707,28 @@ PROCEDURE BeginUnit;
   END BeginUnit;
 
 PROCEDURE LowerUnitNow (unit: AST.Node; isProg: BOOLEAN);
+  VAR nm: SymTab.Name; scoped: BOOLEAN;
   BEGIN
     IF unit = AST.NoNode THEN RETURN END;
     QbeGen.RestoreSession;
     QbeGen.SetNoEmit(FALSE);
+    (* Lower the unit in its own module scope, so type lookups resolve
+       locally and nothing needs to be copied into globScope. *)
+    Name(AST.Child(unit, 0), nm);
+    scoped := SymTab.EnterLowerScope(nm);
     LowerOneUnit(unit, isProg);
+    IF scoped THEN SymTab.LeaveLowerScope END;
     QbeGen.SetNoEmit(TRUE)
   END LowerUnitNow;
 
 PROCEDURE EndSession;
+  VAR scoped: BOOLEAN;
   BEGIN
     QbeGen.SetNoEmit(FALSE);
-    QbeGen.EndModule(curUnitName)
+    (* EndModule probes the program module's ExitCode symbol. *)
+    scoped := SymTab.EnterLowerScope(curUnitName);
+    QbeGen.EndModule(curUnitName);
+    IF scoped THEN SymTab.LeaveLowerScope END
   END EndSession;
 
 BEGIN

+ 3 - 0
compiler/src/SymTab.def

@@ -245,6 +245,9 @@ PROCEDURE BeginProg (name: ARRAY OF CHAR): BOOLEAN;
 
 PROCEDURE EndUnit;
 (* Returns to the global scope. *)
+PROCEDURE EnterLowerScope (name: ARRAY OF CHAR): BOOLEAN;
+(* Make a unit's module scope current while Lower re-emits it. *)
+PROCEDURE LeaveLowerScope;
 
 
 

+ 18 - 0
compiler/src/SymTab.mod

@@ -2432,6 +2432,24 @@ PROCEDURE EndUnit;
     curUnit := -1
   END EndUnit;
 
+PROCEDURE EnterLowerScope (name: ARRAY OF CHAR): BOOLEAN;
+(* Make a unit's module scope current while Lower re-emits it.  Without
+   this, Lower runs in globScope and MaterializeGlobals copies the
+   unit's types there — which leaks them across units (and across
+   generic instantiations of the same generic). *)
+  VAR idx: INTEGER;
+  BEGIN
+    idx := FindMod(name);
+    IF idx = -1 THEN RETURN FALSE END;
+    curMod := idx; curScope := modScopes[idx];
+    RETURN TRUE
+  END EnterLowerScope;
+
+PROCEDURE LeaveLowerScope;
+  BEGIN
+    curScope := globScope; curMod := -1
+  END LeaveLowerScope;
+
 PROCEDURE ModKnown (name: ARRAY OF CHAR): BOOLEAN;
   BEGIN
     RETURN FindMod(name) # -1

+ 12 - 0
compiler/tests/g_twice.def

@@ -0,0 +1,12 @@
+GENERIC DEFINITION MODULE GTwice (T : TYPE);
+(* One generic instantiated twice with different scalar families.  The
+   forward pointer base (NodeRec declared after Node) used to make the
+   second instance resolve the first instance's record. *)
+TYPE
+  Node = POINTER TO NodeRec;
+  NodeRec = RECORD val : T; next : Node END;
+  Stack = RECORD top : Node; depth : INTEGER END;
+PROCEDURE Init (VAR s : Stack);
+PROCEDURE Push (VAR s : Stack; v : T);
+PROCEDURE Pop (VAR s : Stack; VAR v : T) : BOOLEAN;
+END GTwice.

+ 18 - 0
compiler/tests/g_twice.mod

@@ -0,0 +1,18 @@
+GENERIC IMPLEMENTATION MODULE GTwice;
+PROCEDURE Init (VAR s : Stack);
+BEGIN s.top := NIL; s.depth := 0 END Init;
+
+PROCEDURE Push (VAR s : Stack; v : T);
+VAR n : Node;
+BEGIN NEW(n); n^.val := v; n^.next := s.top;
+  s.top := n; s.depth := s.depth + 1
+END Push;
+
+PROCEDURE Pop (VAR s : Stack; VAR v : T) : BOOLEAN;
+VAR n : Node;
+BEGIN
+  IF s.top = NIL THEN RETURN FALSE END;
+  n := s.top; v := n^.val; s.top := n^.next;
+  s.depth := s.depth - 1; RETURN TRUE
+END Pop;
+END GTwice.

+ 2 - 0
compiler/tests/g_twice_c.def

@@ -0,0 +1,2 @@
+DEFINITION MODULE GTwiceChar = GTwice(CHAR);
+END GTwiceChar.

+ 2 - 0
compiler/tests/g_twice_c.mod

@@ -0,0 +1,2 @@
+IMPLEMENTATION MODULE GTwiceChar = GTwice(CHAR);
+END GTwiceChar.

+ 2 - 0
compiler/tests/g_twice_i.def

@@ -0,0 +1,2 @@
+DEFINITION MODULE GTwiceInt = GTwice(INTEGER);
+END GTwiceInt.

+ 2 - 0
compiler/tests/g_twice_i.mod

@@ -0,0 +1,2 @@
+IMPLEMENTATION MODULE GTwiceInt = GTwice(INTEGER);
+END GTwiceInt.

+ 17 - 0
compiler/tests/g_twice_p.mod

@@ -0,0 +1,17 @@
+MODULE GTwiceProg;
+(* Two instantiations of one generic, different scalar families:
+   INTEGER (40 points) and CHAR (2 points) -> exit code 42. *)
+IMPORT GTwiceInt, GTwiceChar;
+VAR
+  is : GTwiceInt.Stack;
+  cs : GTwiceChar.Stack;
+  i : INTEGER;
+  c : CHAR;
+  ExitCode : INTEGER;
+BEGIN
+  ExitCode := 0;
+  GTwiceInt.Init(is); GTwiceInt.Push(is, 7);
+  IF GTwiceInt.Pop(is, i) AND (i = 7) THEN INC(ExitCode, 40) END;
+  GTwiceChar.Init(cs); GTwiceChar.Push(cs, "Z");
+  IF GTwiceChar.Pop(cs, c) AND (c = "Z") THEN INC(ExitCode, 2) END
+END GTwiceProg.

+ 5 - 5
docs/STATUS.md

@@ -1,17 +1,17 @@
-# m2compiler-V3 — status (2026-10-18)
+# m2compiler-V3 — status (2026-10-19)
 
 This answers "where are we now?".
 
 ## Numbers
 
-- Suite **239/239** (`compiler/run_tests.sh`).
+- Suite **240/240** (`compiler/run_tests.sh`).
 - Self-hosting **FIXPOINT OK** — stage2 and stage3 images are
-  byte-identical (3,812,771 bytes).
+  byte-identical (3,821,110 bytes).
 - Corpus **405 / 612** sources compile with 0 errors, **0 crashes**
   (`tools/v3-corpus/corpus.sh` over the gm2 testsuite); the rest are
   genuine language gaps, **gm2 leniencies V3 deliberately rejects**, or
   unresolved imports.
-- Branch **`resolve-check`**; tree clean.
+- Branch **`master`**; tree clean.
 
 ## Architecture (current)
 
@@ -121,7 +121,7 @@ is the 10 gm2-leniency files now correctly rejected).
 ```sh
 cd compiler
 ./build.sh          # M2.atg -> scanner/parser/driver -> ./M2
-./run_tests.sh      # 239/239
+./run_tests.sh      # 240/240
 cd ..
 ./bootstrap/fixpoint.sh   # FIXPOINT OK (byte-identical stage2/stage3)
 ```

+ 38 - 12
docs/analysis_gaps.md

@@ -287,22 +287,48 @@ cases (`subrange4`/`7`/`11`, `shift3`/`4`, `constsize2`, `setoverflow`,
 `setconst3`) are **not standard Modula-2** — only a `POINTER`'s base type
 may be declared later.
 
-## 10. Finding while building `showcase24` (generics)
+## 10. Finding while building `showcase24` (generics) — FIXED
 
-A **generic module may not be instantiated twice with type arguments
+A **generic module could not be instantiated twice with type arguments
 from different families** in one session.  With
 
 ```mod
 GENERIC DEFINITION MODULE GStack (T : TYPE);
-TYPE NodeRec = RECORD val : T; next : Node END; ...
+TYPE Node = POINTER TO NodeRec;
+     NodeRec = RECORD val : T; next : Node END; ...
 ```
 
-`CharStack = GStack(CHAR)` compiles **alone**, and `GStack(INTEGER)`
-works; but `GStack(INTEGER)` **plus** `GStack(CHAR)` fails on the second
-instantiation (`incompatible assignment` at `n^.val := v`).  The
-`INTEGER`+`CARDINAL` pair happens to pass (both `ClInt`, so the wrong
-binding still type-checks), which suggests the instantiations **share
-the formal `T`'s binding** instead of getting independent substitutions.
-Repro: `/tmp`-style five-file session (generic def/mod, two
-instantiations, a program) — see `showcase24` (which therefore uses one
-`GStack` instantiation plus the two-parameter `GPair(K,V)`).
+`CharStack = GStack(CHAR)` compiled **alone**, and `GStack(INTEGER)`
+worked; but `GStack(INTEGER)` **plus** `GStack(CHAR)` failed on the
+second instantiation (`incompatible assignment` at `n^.val := v`).  The
+`INTEGER`+`CARDINAL` pair happened to pass (both `ClInt`, so the wrong
+binding still type-checked).
+
+**Root cause (2026-10-19).**  Not the formal `T` binding: each instance
+re-binds `T` in its own module scope correctly.  The fault was in
+**lowering**, not parsing.  After a unit's `END`, `SymTab.EndUnit` resets
+`curScope` to `globScope`, and `Lower` re-emits the unit from the AST in
+that scope.  To make the unit's names resolvable there,
+`Lower.MaterializeGlobals` copied the unit's **types** into `globScope`.
+For a generic instance that leaked `<Instance>.NodeRec` into the global
+scope.  When the second instance's definition was re-parsed, its `TYPE
+Node = POINTER TO NodeRec` hit `ForwardType`, whose `Find("NodeRec")`
+walked up to `globScope` and found the **first** instance's record — so
+`Node` (and every `n^.val` through it) resolved to the first argument,
+while the procedure parameters used the second.  A forward pointer base
+is what made the leak observable (direct records worked).
+
+**Fix.**  `Lower` now re-emits each unit in its **own module scope**
+(`SymTab.EnterLowerScope`/`LeaveLowerScope` around `LowerOneUnit`, and
+around the `ExitCode` probe in `EndSession`), so nothing needs to be
+copied into `globScope` and no type leaks between units.  Because the
+nested-module scope is popped before lowering, a nested module's
+FROM-imports are re-materialised in the enclosing scope by the new
+`Lower.ReenterImports` (mirroring the existing `ReenterGlobals`).
+
+Regression: `compiler/tests/g_twice{,i,c,p}` — one `GTwice(T)` generic
+instantiated as `GTwiceInt = GTwice(INTEGER)` **and** `GTwiceChar =
+GTwice(CHAR)`, both exercised at run time (`GTwiceProg`, exit code 42).
+`showcase24` can now use two instantiations if wanted.
+Repro during diagnosis: a five-file session (generic def/mod, two
+instantiations, a program).

+ 92 - 0
docs/handoff-2026-10-19-next.md

@@ -0,0 +1,92 @@
+# Next session — plan
+
+**Branch state (2026-10-19):** work is on `master`, tree clean.
+
+**Baseline:** suite **240/240**, fixpoint **OK (3,821,110 bytes)**,
+corpus **405/612, 0 crashes**.
+
+**Policy:** follow **Modula-2**, not gm2 degradation — declaration
+before use (only a `POINTER`'s base may forward-reference).  In
+`docs/gaps/`, the `decl-order` and `gm2-ext` classes are **not targets**;
+the 44-file "undeclared identifier" bucket is inflated by them.
+
+---
+
+## P0 — generics: two instantiations of one generic  ✅ DONE
+
+Fixed on 2026-10-19.  The bug was **not** `EnterTypeParamBound` (each
+instance re-binds `T` correctly).  It was in **Lowering**: after
+`EndUnit`, `Lower` re-emitted each unit in `globScope`, so
+`Lower.MaterializeGlobals` copied the unit's types into `globScope`; a
+generic instance leaked its `NodeRec` there, and the *second* instance's
+`POINTER TO NodeRec` picked it up via `ForwardType` (a forward pointer
+base made it visible; direct records did not).  See
+`docs/analysis_gaps.md` §10 for the full trail.
+
+- `SymTab.EnterLowerScope`/`LeaveLowerScope` + `Lower` re-emits each
+  unit in its own module scope (also around the `ExitCode` probe in
+  `Lower.EndSession`).
+- `Lower.ReenterImports` re-materialises a nested module's FROM-imports
+  in the enclosing scope (the nested scope is popped before lowering).
+- Regression: `compiler/tests/g_twice{,i,c,p}` (`GTwiceProg`, 42).
+
+*Follow-up:* `showcase24` can now use two instantiations of one generic
+(simplify it, or add a demo) — optional.
+
+## P1 — quick, bounded corpus wins
+
+- **`gap22` `__FILE__`/`__LINE__`** (`packed`, +1).
+- **`gap09` scalar actual → `ARRAY OF BYTE`** (`aochar`,
+  `arrayofbyte`, `bytearray`, +~3).
+
+## P2 — structural
+
+- **`gap12`** variant records: empty arms, `ELSE`, untagged inner
+  `CASE` (5 files).
+- **`gap11`** bare procedure value as a statement.
+- **`gap10`** `VAR`-param type leniency (decide whether to adopt gm2's
+  same-size rule — non-standard).
+- **`gap13` remainder**: de-reserve the *standard* builtins (`ORD`,
+  `CHR`, `VAL`, …) — contextual-keyword parser refactor (`ORDL` is done).
+- **`gap18`** set-typed `CASE` labels.
+
+## P3 — conformance remainder (architectural)
+
+Forward references to module-level **variables/procedures** from a
+procedure body still work (the two-phase parse defers bodies).  Strict
+declaration-before-use would require order-aware body checking; high
+risk, touches self-hosting.  The declaration part is already strict.
+
+## P4 — latent correctness
+
+The **size-sensitive V3 miscompile** is still unroot-caused.  Note: the
+2026-10-19 Lowering change moved code around, so re-check whether the
+old symptom ("removing `ASTDUMP` breaks the fixpoint") still reproduces
+before bisecting.  Dedicated session; a real correctness landmine.
+
+## P5 — housekeeping
+
+- Consolidate `docs/summary_*.md` (see `docs/INDEX.md`).
+- (Merge of `resolve-check` → `master`: **done**.)
+
+---
+
+## Gates for every step
+
+`compiler/build.sh` → `compiler/run_tests.sh` (must stay 100%) →
+`bootstrap/fixpoint.sh` (**byte-identical**) → `tools/v3-corpus/corpus.sh`
+(record OK/CRASH + note) → update `docs/gaps/` + `STATUS`/`README` →
+commit with a `v3-*` tag.
+
+## Resume
+
+```sh
+cd .../m2compiler-V3
+git checkout master
+cd compiler && ./build.sh && ./run_tests.sh   # 240/240
+cd .. && TMP="$PWD/.scratch/fixpoint" ./bootstrap/fixpoint.sh
+tools/v3-corpus/corpus.sh                     # 405 OK, 0 crash
+```
+
+**Recommendation:** P1 (two bounded wins) next, then reserve a separate
+session for P4.