Another round of extra _phase changes removed

Also removes loading more code to resolve cyclic class dependencies, and instead rejects some of the more exciting cases

8 files changed, 103 insertions, 157 deletions

diff --git a/chapter28b/README.md b/chapter28b/README.md
index 008f34d2..f313dcf3 100644
--- a/chapter28b/README.md
+++ b/chapter28b/README.md
@@ -191,9 +191,10 @@ already known to be initialized.
 The compiler must also reject cycles in which class initializers depend on one
 another.  Simple keeps *function summaries* that list calls and classes that
 may need initialization, so the compiler can check for cycles before loading
-bodies.  If a possible cycle appears, it loads all remaining bodies, optimizes,
-and checks again.  A constant returned by a some other code outside the cycle
-can make a branch disappear and remove the cycle.
+bodies.  After ordinary optimization, any remaining possible cycle is an
+error; the compiler does not load more bodies to try to disprove it.  Inlining
+and other optimizations can remove a dependency, so changing optimization
+decisions can change whether a program passes this check.
 
 All required object files must be found before execution.  Native code is
 linked before the program starts.  Eval2 can wait until a call needs a body
@@ -219,7 +220,8 @@ the modular `sys` objects used by the library tests. Programs can import
 inputs, and changed library objects using controlled timestamps. It also
 compares native execution after an edit with a clean build.
 `SelectiveLoadingTest` checks summarized calls, loading for inlining, native
-link dependencies, and refinement of a potential initialization cycle.
+link dependencies, and rejection of a possible initialization cycle without
+loading more bodies.
 `make release` builds the launcher jar and standard library; `make view`
 starts the shared graph viewer.
 
diff --git a/chapter28b/src/main/java/com/seaofnodes/simple/codegen/ClassInitDependencies.java b/chapter28b/src/main/java/com/seaofnodes/simple/codegen/ClassInitDependencies.java
index d95f55f2..9a544685 100644
--- a/chapter28b/src/main/java/com/seaofnodes/simple/codegen/ClassInitDependencies.java
+++ b/chapter28b/src/main/java/com/seaofnodes/simple/codegen/ClassInitDependencies.java
@@ -150,16 +150,6 @@ public class ClassInitDependencies {
         return null;
     }
 
-    // A condition-producing helper outside the SCC can eliminate a cycle.
-    // Before reporting one, restore the full-IR precision of the original check.
-    // Called only after check() has found a cycle.
-    Ary<CompUnit> cycleImports() {
-        Ary<CompUnit> units = new Ary<>(CompUnit.class);
-        for( FunNode fun : _code._linker )
-            if( fun!=null && fun.summary()!=null && units.find(fun._compunit)==-1 ) units.add(fun._compunit);
-        return units;
-    }
-
     // Every internal edge of an SCC belongs to a cycle. Print all of them:
     // stack order is not a path, and enumerating all cycle paths can explode.
     // This bookkeeping and formatting are only needed on an error.
diff --git a/chapter28b/src/main/java/com/seaofnodes/simple/codegen/CodeGen.java b/chapter28b/src/main/java/com/seaofnodes/simple/codegen/CodeGen.java
index ddcdba3c..e56068e7 100644
--- a/chapter28b/src/main/java/com/seaofnodes/simple/codegen/CodeGen.java
+++ b/chapter28b/src/main/java/com/seaofnodes/simple/codegen/CodeGen.java
@@ -523,28 +523,9 @@ public class CodeGen {
     }
 
     // TypeCheck saves its successful analysis for Serialize. Loading bodies
-    // or entering another graph-changing phase discards it; no stale edges
-    // survive a later Iter/FinalizeIR/TypeCheck sequence.
+    // or entering another graph-changing phase discards it.
     ClassInitDependencies _classInitDependencies;
 
-    ClassInitDependencies resolveInitCycles() {
-        while( true ) {
-            ClassInitDependencies deps = new ClassInitDependencies(this);
-            String cycle = deps.check();
-            if( cycle==null ) return deps;
-            Ary<CompUnit> units = deps.cycleImports();
-            if( units.isEmpty() ) throw Parser.error(cycle,null);
-            for( CompUnit cu : units ) ParseAll.loadCompUnit(this,cu);
-            Phase prior = _phase;
-            try {
-                _phase=Phase.IterPostOpto;
-                _iter.iterate(this);
-                _phase=Phase.FinalizeIR;
-                FinalizeIR.run(this);
-            } finally { _phase=prior; }
-        }
-    }
-
     public CodeGen finalizeIR() { return phase(Phase.FinalizeIR, _phase==Phase.IterPostOpto, () -> FinalizeIR.run(this)); }
 
     // Cross-phase `keep()`s belong here!  We have a few nodes which we can
diff --git a/chapter28b/src/main/java/com/seaofnodes/simple/codegen/Serialize.java b/chapter28b/src/main/java/com/seaofnodes/simple/codegen/Serialize.java
index 3e4adf5f..32027cb5 100644
--- a/chapter28b/src/main/java/com/seaofnodes/simple/codegen/Serialize.java
+++ b/chapter28b/src/main/java/com/seaofnodes/simple/codegen/Serialize.java
@@ -450,113 +450,76 @@ public class Serialize {
         String[] strs = tread.strs();
         GlobalBits fileAliases = tread.fileAliases();
 
-        CodeGen.Phase prior = code._phase;
-        code._phase=CodeGen.Phase.Parse; // Constructors must tolerate an incomplete graph.
-        try {
-            // D - Read the nodes
-
-            // Number of nodes
-            int num = bais.packed4();
-            // Packed array of nodes
-            Ary<Node> nodes = new Ary<>(Node.class);
-            CompUnit cu = null;
-            for( int i = 0; i<num; i++ ) {
-                Node.Tag tag = Node.Tag.VALS[bais.packed1()];
-                Type t = types[bais.packed2()];
-                Node n = tag.make(bais,strs,types,fileAliases,aliases);
-                n._type = t;
-                nodes.push(n);
-                // A little extra for FunNodes
-                if( n instanceof StartCUNode startcu ) {
-                    cu = code._compunits.get(startcu._fname==null ? (code._srcName==null ? "Test" : code._srcName) : startcu._fname);
-                    assert cu != null;
-                }
-                if( n instanceof StopCUNode stopcu ) {
-                    cu = null;
-                }
-                if( n instanceof FunNode fun ) {
-                    assert cu != null;
-                    fun._compunit = cu;
-                    if( fun._name==null || fun._name.isEmpty() ) fun._name=fidxs.symbol(fun.sig().fidx());
-                }
-                if( n instanceof ExternNode ext && ext._con instanceof TypeFunPtr tfp && tfp.isConstant() ) {
-                    String old = code.externFunc(tfp.fidx());
-                    if( old == null )
-                        code.externFunc(tfp.fidx(), ext._extern);
-                    else
-                        assert old.equals(ext._extern);
-                }
+        // D - Read the nodes
+
+        // Number of nodes
+        int num = bais.packed4();
+        // Packed array of nodes
+        Ary<Node> nodes = new Ary<>(Node.class);
+        CompUnit cu = null;
+        for( int i = 0; i<num; i++ ) {
+            Node.Tag tag = Node.Tag.VALS[bais.packed1()];
+            Type t = types[bais.packed2()];
+            Node n = tag.make(bais,strs,types,fileAliases,aliases);
+            n._type = t;
+            nodes.push(n);
+            // A little extra for FunNodes
+            if( n instanceof StartCUNode startcu ) {
+                cu = code._compunits.get(startcu._fname==null ? (code._srcName==null ? "Test" : code._srcName) : startcu._fname);
+                assert cu != null;
+            }
+            if( n instanceof StopCUNode stopcu ) {
+                cu = null;
+            }
+            if( n instanceof FunNode fun ) {
+                assert cu != null;
+                fun._compunit = cu;
+                if( fun._name==null || fun._name.isEmpty() ) fun._name=fidxs.symbol(fun.sig().fidx());
+            }
+            if( n instanceof ExternNode ext && ext._con instanceof TypeFunPtr tfp && tfp.isConstant() ) {
+                String old = code.externFunc(tfp.fidx());
+                if( old == null )
+                    code.externFunc(tfp.fidx(), ext._extern);
+                else
+                    assert old.equals(ext._extern);
             }
+        }
 
-            // Node edges
-            for( Node n : nodes ) {
-                int len = n.nIns();
-                for( int i = 0; i<len; i++ ) {
-                    int idx = bais.packed2();
-                    Node def = idx==0 ? null : nodes.at(idx-1);
-                    // MemMerge precise aliases are encoded by their input slot.
-                    // Remap populated file-local slots into this compilation's
-                    // local alias numbering; null holes carry no identity.
-                    int local = i;
-                    if( n instanceof MemMergeNode && i >= GlobalBits.RESERVED ) {
-                        if( def == null )  continue;
-                        local = aliases.map(fileAliases,i);
-                    }
-                    n.setDefX(local,def);
-                    // Call's output[0] is always the CallEnd
-                    if( n instanceof CallEndNode cend && i==0 ) {
-                        var outs = cend.call()._outputs;
-                        outs.swap(0,outs._len-1);
-                    }
+        // Node edges
+        for( Node n : nodes ) {
+            int len = n.nIns();
+            for( int i = 0; i<len; i++ ) {
+                int idx = bais.packed2();
+                Node def = idx==0 ? null : nodes.at(idx-1);
+                // MemMerge precise aliases are encoded by their input slot.
+                // Remap populated file-local slots into this compilation's
+                // local alias numbering; null holes carry no identity.
+                int local = i;
+                if( n instanceof MemMergeNode && i >= GlobalBits.RESERVED ) {
+                    if( def == null )  continue;
+                    local = aliases.map(fileAliases,i);
                 }
-                // A little extra for FunNodes
-                if( n instanceof FunNode fun ) {
-                    // Return is hooked like an edge
-                    ReturnNode ret = (ReturnNode)nodes.at(bais.packed2()-1);
-                    fun.setRet(ret);
-                    ret._fun = fun;
+                n.setDefX(local,def);
+                // Call's output[0] is always the CallEnd
+                if( n instanceof CallEndNode cend && i==0 ) {
+                    var outs = cend.call()._outputs;
+                    outs.swap(0,outs._len-1);
                 }
             }
-            // Add edge from Start to Stop
-            StartNode start = (StartNode)nodes.at(0);
-            StopNode stop = (StopNode)nodes.last();
-            start.setDef(1,stop);
-
-            // A body materialized after SCCP joins a different analysis context.
-            // Its saved types are reset and checked by the ensuing fixed-point solve.
-            assert prior!=CodeGen.Phase.Parse || check(code,nodes);
-            elf._nodes = nodes;
-        } finally { code._phase=prior; }
-    }
-
-    // Check loaded types and nodes are consistent
-    private static boolean check( CodeGen code, Ary<Node> nodes ) {
-        // Loaded nodes are complete, unlike nodes still being built by the
-        // parser.  Recompute them with post-parse semantics.  The compilation-
-        // unit Stop is the exception: its escape type depends on the complete
-        // program's public interface, which ParseAll freezes only after every
-        // compilation unit has been loaded.
-        boolean good = true;
-        CodeGen.Phase phase = code._phase;
-        code._phase = CodeGen.Phase.Opto;
-        try {
-            for( Node n : nodes ) {
-                if( n instanceof StopCUNode && !code.publicInterfaceFrozen() )
-                    continue;
-                // Cross-object calls are relinked only after every body has
-                // loaded. Their saved return types cannot be recomputed yet.
-                if( n instanceof CallEndNode end && end.call().fptr()._type instanceof TypeFunPtr fp &&
-                    end.nIns()<=fp.nfcns() ) continue;
-                Type nval = n.compute();
-                if( nval != n._type ) {
-                    System.err.println("Node "+n+" read in as "+n._type+" but computes as "+nval);
-                    good = false;
-                }
+            // A little extra for FunNodes
+            if( n instanceof FunNode fun ) {
+                // Return is hooked like an edge
+                ReturnNode ret = (ReturnNode)nodes.at(bais.packed2()-1);
+                fun.setRet(ret);
+                ret._fun = fun;
             }
-        } finally {
-            code._phase = phase;
         }
-        return good;
+        // Add edge from Start to Stop
+        StartNode start = (StartNode)nodes.at(0);
+        StopNode stop = (StopNode)nodes.last();
+        start.setDef(1,stop);
+
+        elf._nodes = nodes;
     }
 
     // --------------------------------------------------
diff --git a/chapter28b/src/main/java/com/seaofnodes/simple/codegen/TypeCheck.java b/chapter28b/src/main/java/com/seaofnodes/simple/codegen/TypeCheck.java
index 78794096..a5eaa5ca 100644
--- a/chapter28b/src/main/java/com/seaofnodes/simple/codegen/TypeCheck.java
+++ b/chapter28b/src/main/java/com/seaofnodes/simple/codegen/TypeCheck.java
@@ -10,8 +10,12 @@ import com.seaofnodes.simple.util.Ary;
 abstract class TypeCheck {
     static void run(CodeGen code) {
         // Check for class-init cycles first; such a cycle can prevent other
-        // things from resolving types. Serialize can reuse the final graph.
-        code._classInitDependencies = code.resolveInitCycles();
+        // things from resolving types. Use the graph and summaries left by
+        // optimization; do not load more bodies to disprove a possible cycle.
+        ClassInitDependencies deps = new ClassInitDependencies(code);
+        String cycle = deps.check();
+        if( cycle!=null ) throw Parser.error(cycle,null);
+        code._classInitDependencies = deps; // Serialize reuses this checked graph.
 
         Ary<Node> errs = new Ary<>(Node.class);
         code._stop.walk( n -> {
diff --git a/chapter28b/src/main/java/com/seaofnodes/simple/node/StopNode.java b/chapter28b/src/main/java/com/seaofnodes/simple/node/StopNode.java
index a7e52563..6d453324 100644
--- a/chapter28b/src/main/java/com/seaofnodes/simple/node/StopNode.java
+++ b/chapter28b/src/main/java/com/seaofnodes/simple/node/StopNode.java
@@ -15,7 +15,8 @@ public class StopNode extends CFGNode {
 
     public StopNode() {
         super();
-        _type = compute();
+        // Edges may still be missing, including when loading a StopCU.
+        _type = TypeTuple.STATE;
     }
     public StopNode(StopNode stop) { super(stop); }
     @Override public Tag serialTag() { return Tag.Stop; }
diff --git a/chapter28b/src/test/java/com/seaofnodes/simple/ClassInitDependenciesTest.java b/chapter28b/src/test/java/com/seaofnodes/simple/ClassInitDependenciesTest.java
index 08648187..38b4d8a2 100644
--- a/chapter28b/src/test/java/com/seaofnodes/simple/ClassInitDependenciesTest.java
+++ b/chapter28b/src/test/java/com/seaofnodes/simple/ClassInitDependenciesTest.java
@@ -248,25 +248,25 @@ public class ClassInitDependenciesTest {
         Path dir=tempDir("import-"), stubs=tempDir("import-");
         object(stubs,stubs,"A","int value=arg;");
         object(stubs,stubs,"B","int value=arg;");
-        // Each object is independently acyclic. Combining their real bodies
+        // Each object is independently acyclic. Combining their summaries
         // must recheck the graph rather than trust either object's local order.
         String asrc="int value=arg;\nvalue=B.value;";
         CodeGen acode=module(dir,stubs,"A",Map.of("A",asrc));
         String afile=acode._compunits.get("A").sourceFile();
         module(dir,stubs,"B",Map.of("B","int value=arg; value=A.value;"));
         for( String entry : new String[]{"A","B"} ) {
+            CodeGen code = new CodeGen(null,dir.toString(),new Ary<>(new String[]{dir.toString()}),
+                                      "Main","return "+entry+".value;",123,true,TypeInteger.BOT);
             try {
-                new CodeGen(null,dir.toString(),new Ary<>(new String[]{dir.toString()}),
-                            "Main","return "+entry+".value;",123,true,TypeInteger.BOT).driver(CodeGen.Phase.TypeCheck);
+                code.driver(CodeGen.Phase.TypeCheck);
                 fail("Expected imported initialization cycle");
             } catch( Parser.ParseException ex ) {
                 assertCycle(ex.getMessage(),"A -> B -> A");
-                assertTrue(ex.getMessage(),ex.getMessage().contains("value=B.value;\n    -------^"));
-                assertTrue(ex.getMessage(),ex.getMessage().contains("\n"+afile+":2:8: error: cyclic class initialization\n"));
+                assertNull(code.getElf(code._compunits.get("A")._obj)._nodes);
+                assertNull(code.getElf(code._compunits.get("B")._obj)._nodes);
             }
         }
-        // The imported object must still report its saved path and offset
-        // when the original source is no longer available.
+        // The summary reports the dependency chain even without source files.
         Files.delete(Path.of(afile));
         try {
             new CodeGen(null,dir.toString(),new Ary<>(new String[]{dir.toString()}),
@@ -274,7 +274,6 @@ public class ClassInitDependenciesTest {
             fail("Expected imported initialization cycle");
         } catch( Parser.ParseException ex ) {
             assertCycle(ex.getMessage(),"A -> B -> A");
-            assertTrue(ex.getMessage(),ex.getMessage().contains("\n"+afile+": error: cyclic class initialization (source unavailable at byte offset "+asrc.indexOf('.')+")"));
         }
     }
 
diff --git a/chapter28b/src/test/java/com/seaofnodes/simple/SelectiveLoadingTest.java b/chapter28b/src/test/java/com/seaofnodes/simple/SelectiveLoadingTest.java
index d8fad3d5..d56d5c89 100644
--- a/chapter28b/src/test/java/com/seaofnodes/simple/SelectiveLoadingTest.java
+++ b/chapter28b/src/test/java/com/seaofnodes/simple/SelectiveLoadingTest.java
@@ -160,29 +160,35 @@ public class SelectiveLoadingTest {
         }
     }
 
-    @Test public void testCycleRefinementLoadsConditionHelper() throws Exception {
+    @Test public void testPossibleCycleDoesNotLoadConditionHelper() throws Exception {
         Path base = Path.of("build/objs/selective");
         Files.createDirectories(base);
         Path root = Files.createTempDirectory(base,"cycle-");
-        Path stubs = Files.createDirectory(root.resolve("stubs"));
+        Path src = Files.createDirectory(root.resolve("src"));
         Path lib = Files.createDirectory(root.resolve("lib"));
-        object(stubs,null,"A","int value=arg;");
-        object(stubs,null,"B","int value=arg;");
         StringBuilder helper = new StringBuilder("val test={bool n -> ");
         // Large parse, small final body: it will not request speculative loading.
         for( int i = 0; i<80; i++ ) helper.append("val x").append(i).append("=n+0;");
         helper.append("return n;};");
         object(lib,null,"Predicate",helper.toString());
-        Ary<String> inputs = new Ary<>(new String[]{stubs.toString(),lib.toString()});
-        object(lib,inputs,"A","int value=1; if(Predicate.test(false)) value=B.value;");
-        object(lib,inputs,"B","int value=A.value;");
-        CodeGen code = new CodeGen(null,root.toString(),new Ary<>(new String[]{lib.toString()}),
-            "Main","return A.value;",123,true,TypeInteger.BOT).driver(CodeGen.Phase.FinalizeIR);
+        // All sources and objects agree. The summary cannot express test(n)==n.
+        write(src,"Root","val entry=A;");
+        write(src,"Root/A","int value=1; if(Predicate.test(false)) value=B.value;");
+        write(src,"Root/B","int value=A.value;");
+        CodeGen code = new CodeGen(src.toString(),root.toString(),new Ary<>(new String[]{lib.toString()}),
+            "Root",null,123,true,TypeInteger.BOT).driver(CodeGen.Phase.FinalizeIR);
         assertNotNull(function(code,"test").summary());
         assertNotNull("Summaries alone report a potential cycle",new ClassInitDependencies(code).check());
-        code.driver(CodeGen.Phase.TypeCheck);
-        assertNull("The helper is outside A/B's apparent cycle",function(code,"test").summary());
-        assertEquals("1",Eval2.eval(code,0));
+        try {
+            code.driver(CodeGen.Phase.TypeCheck);
+            fail("Reject the possible cycle without loading the condition helper");
+        } catch( Parser.ParseException ex ) {
+            assertTrue(ex.getMessage(),ex.getMessage().startsWith("Cyclic class initialization:"));
+            assertTrue(ex.getMessage(),ex.getMessage().contains("Root.A -> Root.B"));
+            assertTrue(ex.getMessage(),ex.getMessage().contains("Root.B -> Root.A"));
+        }
+        assertNotNull(function(code,"test").summary());
+        assertNull(code.getElf(code._compunits.get("Predicate")._obj)._nodes);
     }
 
     private static void object(Path dir, Ary<String> paths, String name, String source) {