diff options
10 files changed, 342 insertions, 83 deletions
diff --git a/chapter25/initialization.md b/chapter25/initialization.md index 30b1589c..9d2c7250 100644 --- a/chapter25/initialization.md +++ b/chapter25/initialization.md @@ -27,11 +27,11 @@ Use one runtime bit in the first, single-threaded implementation. - Proven static-data accesses require no dynamic work. This does not authorize a called function to observe unfinished dynamic state in an ancestor or any other class. -- Initialization requirements form a DAG across the complete linked program, - including module boundaries. Count only requirements surviving Opto and its - call-graph analysis. Reject cycles behind conditionals that remain unresolved; - eliminated branches and call targets contribute no edges. Ordinary recursive - functions are still legal. +- After Opto, build the combined function-call and class-initialization graph, + including module boundaries. Reject an SCC only when it contains more than + one `<clinit>`. Ordinary recursion, initializer self-edges, and helper cycles + involving only one initializer are legal. Unresolved conditional branches + contribute edges; eliminated branches and call targets do not. - Initialization order follows actual execution and parent requirements, not parser discovery order, worklist seed, or filesystem enumeration order. @@ -42,9 +42,20 @@ provides the reverse dependency. An initializer may read its own previously initialized fields under the existing definite-initialization rules. That is not an ordinary external active touch. -Do not infer permission for arbitrary callbacks or same-class helper calls from -the fact that their owner is currently being initialized. Such helpers need a -proof that they do not observe unfinished state; otherwise reject their use. +Same-class helper calls are allowed by the cycle checker, whether or not they +inline. Ordinary field-initialization and type checks remain separate. + +The cycle diagnostic lists every internal edge of the first rejected SCC, +including helper calls. Every listed edge participates in a cycle; this reports +all cyclic connections in that component without enumerating individual paths. +On failure, rescan the component's functions to choose one surviving touch or +call site for each edge. `SourceError.format(CompUnit, byteOffset, message)` prints +`file:line:character: error: message` starting in column one, followed by the +source line and a separate caret line. Lexer snapshots retain their +original CompUnit through inlining; Call and CLInit serialization retain the +offset and source identity/path. Imported source is read only on error. Parent +requirements are labeled explicitly rather than assigned a fabricated position. +This is a focused diagnostic utility, not a migration of all parser errors. ## Ownership and identity diff --git a/chapter25/src/main/java/com/seaofnodes/simple/Parser.java b/chapter25/src/main/java/com/seaofnodes/simple/Parser.java index 331ff05d..f0c4567f 100644 --- a/chapter25/src/main/java/com/seaofnodes/simple/Parser.java +++ b/chapter25/src/main/java/com/seaofnodes/simple/Parser.java @@ -4,6 +4,7 @@ import com.seaofnodes.simple.codegen.*; import com.seaofnodes.simple.node.*; import com.seaofnodes.simple.type.*; import com.seaofnodes.simple.util.Ary; +import com.seaofnodes.simple.util.BAOS; import com.seaofnodes.simple.node.ScopeNode.Kind; import static com.seaofnodes.simple.util.Utils.TODO; @@ -168,7 +169,7 @@ public class Parser { public Ary<FRefNode> parse( CompUnit ref ) { assert _scope == null && _breakScope == null && _continueScope == null && _returnScope == null; assert _frefs == null; - _lexer = new Lexer(ref._src); + _lexer = new Lexer(ref); _frefs = new Ary<>(FRefNode.class); // Starting Scope has control, memory, initial arguments _scope = new ScopeNode(); @@ -2231,6 +2232,8 @@ public class Parser { return parsePostfix(expr); } private Node parsePostfix(Node expr) { + _lexer.skipWhiteSpace(); + Lexer site = loc(); String name; if( match(".") ) name = requireId(); else if( match("#") ) name = "#"; @@ -2240,7 +2243,7 @@ public class Parser { else if( match("(") ) return parsePostfix(require(functionCall(expr,defaultSelf()),")")); else return expr; // No postfix - return parsePostfixName(expr,name,0); + return parsePostfixName(expr,name,0,site); } private Node defaultSelf() { @@ -2261,6 +2264,10 @@ public class Parser { // pre is +/-1 for a prefix update, otherwise parse a postfix operation. private Node parsePostfixName(Node expr, String name, int pre) { + return parsePostfixName(expr,name,pre,loc()); + } + + private Node parsePostfixName(Node expr, String name, int pre, Lexer site) { // Keep expr across possible updates expr.keep(); @@ -2308,7 +2315,7 @@ public class Parser { Kind.Func func = (Kind.Func)_scope._kinds.at(_scope.enclosingFuncOrDecl()); if( (ts==null || ts==TypeStruct.BOT || ts.isHigh() || startsClzPrefix(ts._name)) && !(ts!=null && (ts._name+".<clinit>").equals(func._name)) ) - ctrl(new CLInitNode(loc(),ctrl(),expr).peephole()); + ctrl(new CLInitNode(site,ctrl(),expr).peephole()); // With no field, we will update the bulk memory int alias = fld==null ? 1 : fld._alias; @@ -2473,6 +2480,8 @@ public class Parser { * </pre> */ private Node functionCall(Node fcn, Node self) { + Lexer site = loc(); + site._position--; // Opening '('; arguments can contain nested calls. if( fcn._type == Type.NIL ) throw error("Calling a null function pointer"); fcn.keep(); // Keep while parsing args @@ -2495,10 +2504,14 @@ public class Parser { // Unkeep them all for( Node arg : args ) arg.unkeep(); - return functionCall(args); + return functionCall(args,site); } private Node functionCall( Ary<Node> args ) { + return functionCall(args,loc()); + } + + private Node functionCall( Ary<Node> args, Lexer site ) { // Dead into the call? Skip all the node gen if( ctrl()._type == Type.XCONTROL ) { for( Node arg : args ) @@ -2508,7 +2521,7 @@ public class Parser { } // Into the call - CallNode call = (CallNode)new CallNode(loc(), args.asAry()).peephole(); + CallNode call = (CallNode)new CallNode(site, args.asAry()).peephole(); // Post-call setup CallEndNode cend = (CallEndNode)new CallEndNode(call,TypeRPC.constant(_code.rpc(_ref._cname))).peephole(); call.peephole(); // Rerun peeps after CallEnd, allows early inlining @@ -2694,6 +2707,7 @@ public class Parser { // Input buffer; an array of text bytes read from a file or a string private final byte[] _input; + public final CompUnit _cu; // Original source, retained across inlining. // Tracks current position in input buffer private int _position = 0; @@ -2704,12 +2718,11 @@ public class Parser { /** True when this location precedes {@code that} in the same source. */ public boolean before(Lexer that) { - return that != null && _input == that._input && _position < that._position; + return that != null && (_cu!=null ? _cu==that._cu : _input==that._input) && _position < that._position; } public String position() { return _line_number+":"+(_position-_line_start+1); } - // Numeric location for diagnostics that must survive without source text. - public long lineColumn() { return ((long)_line_number<<32) | (_position-_line_start+1); } + public int pos() { return _position; } /** * Record the source text for lexing @@ -2723,6 +2736,45 @@ public class Parser { */ public Lexer(byte[] buf) { _input = buf; + _cu = null; + } + + private Lexer(CompUnit cu) { + _input = cu._src.getBytes(); + _cu = cu; + } + + // Imported location: source text is only needed when reporting an error. + private Lexer(CompUnit cu, int pos) { + _input = null; + _cu = cu; + _position = pos; + } + + public static void gather(Lexer loc, HashMap<String,Integer> strs) { + if( loc==null || loc._cu==null ) return; + Serialize.gather(strs,loc._cu._cname); + String file=loc._cu.sourceFile(); + if( file!=null ) Serialize.gather(strs,file); + } + + public static void packed(Lexer loc, BAOS out, HashMap<String,Integer> strs) { + if( loc==null || loc._cu==null ) { out.packed4(0); return; } + out.packed4(loc._position+1); + out.packed4(strs.get(loc._cu._cname)); + String file=loc._cu.sourceFile(); + out.packed4(file==null ? 0 : strs.get(file)); + } + + public static Lexer unpack(BAOS in, String[] strs) { + int pos=in.packed4(); + if( pos==0 ) return null; + String owner=strs[in.packed4()]; + String file=strs[in.packed4()]; + CompUnit cu=CodeGen.CODE._compunits.get(owner.replace('.','/')); + assert cu!=null : owner; + if( !file.isEmpty() ) cu._sourceFile=file; + return new Lexer(cu,pos-1); } /** @@ -2730,6 +2782,7 @@ public class Parser { */ private Lexer(Lexer l) { _input = l._input; + _cu = l._cu; _position = l._position; _line_number = l._line_number; _line_start = l._line_start; @@ -2738,7 +2791,7 @@ public class Parser { // Very handy in the debugger, shows the unparsed program @Override public String toString() { - return new String(_input, _position, _input.length - _position); + return _input==null ? _cu._cname+"@"+_position : new String(_input, _position, _input.length - _position); } // True if at EOF diff --git a/chapter25/src/main/java/com/seaofnodes/simple/codegen/ClassInitDependencies.java b/chapter25/src/main/java/com/seaofnodes/simple/codegen/ClassInitDependencies.java index 0150219e..df3be2d0 100644 --- a/chapter25/src/main/java/com/seaofnodes/simple/codegen/ClassInitDependencies.java +++ b/chapter25/src/main/java/com/seaofnodes/simple/codegen/ClassInitDependencies.java @@ -1,5 +1,6 @@ package com.seaofnodes.simple.codegen; +import com.seaofnodes.simple.Parser; import com.seaofnodes.simple.node.*; import com.seaofnodes.simple.type.*; import com.seaofnodes.simple.util.*; @@ -15,13 +16,11 @@ import java.util.*; // CompUnits from CLInitNodes, and the set of fcn call targets in f and the // class parent. These are all reported as fidxs (ints). // -// (2) Walk the _edges graph using Tarjans SCC. Stop at cycles containing a -// <clint> (and continue with cycles NOT containing a <clinit>, which is -// ordinary recursive functions). +// (2) Walk the _edges graph using Tarjans SCC. Reject cycles containing more +// than one <clinit>. // // (3) Report on <clinit> cycles // -// public class ClassInitDependencies { private final CodeGen _code; // Function FIDX -> callees, touched initializers, and parent initializer. @@ -50,12 +49,13 @@ public class ClassInitDependencies { for( FunNode fun : code._linker ) if( fun!=null && !fun.isDead() ) // Visit function and collect direct class touches and callees - _edgess[fun.sig().fidx()] = walk(code,fun,work,iset,visit); + _edgess[fun.sig().fidx()] = walk(code,fun,work,iset,visit,null); visit.clear(); } // Return the outgoing init edge set for this function - private static int[] walk( CodeGen code, FunNode fun, Ary<CFGNode> work, IntHashMap iset, BitSet visit) { + private static int[] walk( CodeGen code, FunNode fun, Ary<CFGNode> work, IntHashMap iset, BitSet visit, + Ary<Parser.Lexer> sites) { // Walk function body, looking for outgoing init edges assert iset .isEmpty(); // int hash set for de-dup assert visit.isEmpty(); // needs a visit set, because loops @@ -79,14 +79,17 @@ public class ClassInitDependencies { if( cfg instanceof CLInitNode init ) { FunNode clz = code.link(init.cu()); // <clinit> referred to iset.put(clz.sig().fidx(),1); // Add to edge set + if( sites!=null && sites.atX(clz.sig().fidx())==null ) sites.setX(clz.sig().fidx(),init._loc); } // Calls union all their fptr fidxs into the callee set if( cfg instanceof CallNode call && call.fptr()._type instanceof TypeFunPtr fp ) { assert !XInt.isHigh(fp.fidxs()) && fp.nfcns()>0; // Call is alive with valid targets // Union call targets into the callee set - for( int dstx = XInt.next(fp.fidxs(),0); dstx>=0; dstx=XInt.next(fp.fidxs(),dstx) ) + for( int dstx = XInt.next(fp.fidxs(),0); dstx>=0; dstx=XInt.next(fp.fidxs(),dstx) ) { iset.put(dstx,1); + if( sites!=null && sites.atX(dstx)==null ) sites.setX(dstx,call._loc); + } } // Forward recursion to walk the whole function @@ -106,36 +109,61 @@ public class ClassInitDependencies { return edges; } - // Return an error string, or null for an acyclic class-init graph. + // Return an error string, or null if no cycle contains multiple clinits. public String check() { + Arrays.fill(_pres,0); + _sp = 0; _pres[0] = 1; // Preorders stored in pres[0] and start at 1 - BitSet visit = _code.visit(); - int idx = 0; - for( int fidx = 1; fidx < _edgess.length; fidx++ ) { - if( _pres[fidx] == 0 ) // Already visited - idx = scc(fidx); - if( idx != -1 ) break; // Found a clz cycle - } - visit.clear(); + int idx = -1; + for( int fidx = 1; fidx < _edgess.length; fidx++ ) + if( _pres[fidx] == 0 && // Already visited + (idx = scc(fidx)) != -1 ) // And found a cycle + return report(idx); // Report cycle + return null; + } - // Error report for a clz heading a cycle - if( idx != -1 ) { - SB sb = new SB().p("Cyclic class initialization: "); - String first = null; - for( int i=idx; i<_sp; i++ ) { - FunNode fun = _code.link(_stk[i]); - if( fun.isClz() ) { - if( first==null ) first = fun._compunit._cname; - sb.p(fun._compunit._cname).p(" -> "); - } - } - sb.p(first); - visit.clear(); - return sb.toString(); + // 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. + private String report(int idx) { + BitSet members = new BitSet(); + for( int i=idx; i<_sp; i++ ) members.set(_stk[i]); + SB sb = new SB().p("Cyclic class initialization:"); + // Recover one surviving source site per edge only after finding an + // error. A representative touch is enough; runtime first-touch order + // need not be statically known. Lexer snapshots retain the source CU + // even when the node has been inlined into a different function. + Ary<CFGNode> work = new Ary<>(CFGNode.class); + IntHashMap iset = new IntHashMap(0); + BitSet visit = new BitSet(); + Ary<Parser.Lexer> sites = new Ary<>(Parser.Lexer.class); + for( int i=idx; i<_sp; i++ ) { + int src = _stk[i]; + FunNode fun = _code.link(src); + Arrays.fill(sites._es,null); + sites.clear(); + walk(_code,fun,work,iset,visit,sites); + if( _edgess[src]!=null ) + for( int dst : _edgess[src] ) + if( members.get(dst) ) { + sb.p("\n ").p(name(src)).p(" -> ").p(name(dst)); + Parser.Lexer loc = sites.atX(dst); + if( loc!=null && loc._cu!=null ) + sb.p('\n').p(SourceError.format(loc._cu,loc.pos(),"cyclic class initialization")); + else if( fun.isClz() && fun._compunit._par==_code.link(dst)._compunit ) + sb.p("\n parent initialization requirement"); + } } + return sb.toString(); + } - return null; // No errors + private String name(int fidx) { + FunNode fun = _code.link(fidx); + String owner=fun._compunit._cname; + if( fun.isClz() ) return owner; + String label=fun.label(); + return (label.startsWith(owner+".") ? label : owner+"."+label)+" [fidx "+fidx+"]"; } // Tarjan SCC @@ -174,10 +202,12 @@ public class ClassInitDependencies { private boolean onStk(int e) { return find(e) != -1; } private boolean hasClzCycle(int idx) { - if( idx+1 == _sp ) return false; // Cycle of 1 does not count - for( int i=idx; i<_sp; i++ ) - if( _code.link(_stk[i]).isClz() ) + int clinits = 0; + for( int i=idx; i<_sp; i++ ) { + FunNode fun = _code.link(_stk[i]); + if( fun!=null && fun.isClz() && ++clinits > 1 ) return true; + } return false; } diff --git a/chapter25/src/main/java/com/seaofnodes/simple/codegen/CompUnit.java b/chapter25/src/main/java/com/seaofnodes/simple/codegen/CompUnit.java index 544ca27e..128c4136 100644 --- a/chapter25/src/main/java/com/seaofnodes/simple/codegen/CompUnit.java +++ b/chapter25/src/main/java/com/seaofnodes/simple/codegen/CompUnit.java @@ -24,6 +24,13 @@ public class CompUnit { final File _smp; // Simple source file, e.g. module_root/A/B/C.smp public final File _obj; // ELF output file, e.g. build/A/B/C.o final public String _src; // Source code, from file or test case + // Original source path from an imported diagnostic location. Does not + // affect discovery/rebuild decisions, which use _smp and _src. + public String _sourceFile; + + public String sourceFile() { + return _smp!=null ? _smp.getAbsolutePath() : _sourceFile; + } public TypeStruct _clz; // The one TypeStruct being published diff --git a/chapter25/src/main/java/com/seaofnodes/simple/codegen/SourceError.java b/chapter25/src/main/java/com/seaofnodes/simple/codegen/SourceError.java new file mode 100644 index 00000000..f2f46146 --- /dev/null +++ b/chapter25/src/main/java/com/seaofnodes/simple/codegen/SourceError.java @@ -0,0 +1,49 @@ +package com.seaofnodes.simple.codegen; + +import com.seaofnodes.simple.util.SB; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +/** Format one parser byte offset. Source is read and text built only on error. */ +public abstract class SourceError { + public static String format(CompUnit cu, int pos, String message) { + String file=cu.sourceFile(); + String name=file==null ? cu._fname+".smp" : file; + byte[] source; + try { + source=cu._src!=null ? cu._src.getBytes(StandardCharsets.UTF_8) + : file==null ? null : Files.readAllBytes(Path.of(file)); + } catch( IOException ex ) { source=null; } + if( source==null || pos<0 || pos>source.length ) + return name+": error: "+message+" (source unavailable at byte offset "+pos+")"; + + int line=1, start=0; + for( int i=0; i<pos; i++ ) + if( source[i]=='\n' ) { line++; start=i+1; } + int end=pos; + while( end<source.length && source[end]!='\n' ) end++; + if( end>start && source[end-1]=='\r' ) end--; + int point=Math.min(pos,end); + String before=new String(source,start,point-start,StandardCharsets.UTF_8); + String text=new String(source,start,end-start,StandardCharsets.UTF_8); + int column=before.codePointCount(0,before.length())+1; + SB sb=new SB().p(name).p(':').p(line).p(':').p(column).p(": error: ").p(message).p("\n "); + int width=0, caret=0; + for( int i=0; i<text.length(); ) { + if( i==before.length() ) caret=width; + int ch=text.codePointAt(i); + if( ch=='\t' ) { + int spaces=8-(width%8); + for( int j=0; j<spaces; j++ ) sb.p(' '); + width+=spaces; + } else { sb.p(new String(Character.toChars(ch))); width++; } + i+=Character.charCount(ch); + } + if( before.length()==text.length() ) caret=width; + sb.p("\n "); + for( int i=0; i<caret; i++ ) sb.p('-'); + return sb.p('^').toString(); + } +} diff --git a/chapter25/src/main/java/com/seaofnodes/simple/node/CLInitNode.java b/chapter25/src/main/java/com/seaofnodes/simple/node/CLInitNode.java index 5131049e..c9aeb874 100644 --- a/chapter25/src/main/java/com/seaofnodes/simple/node/CLInitNode.java +++ b/chapter25/src/main/java/com/seaofnodes/simple/node/CLInitNode.java @@ -22,10 +22,12 @@ public class CLInitNode extends CFGNode { _loc = loc; } - CLInitNode(BAOS bais) { super(null,null); } + CLInitNode(BAOS bais, String[] strs) { super(null,null); _loc=Parser.Lexer.unpack(bais,strs); } @Override public Tag serialTag() { return Tag.CLInit; } @Override public void packed( BAOS baos, HashMap<String,Integer> strs, HashMap<Type,Integer> types, IdentityHashMap<Node, Integer> anodes ) { + Parser.Lexer.packed(_loc,baos,strs); } + @Override public void gather(HashMap<String,Integer> strs) { Parser.Lexer.gather(_loc,strs); } public CLInitNode(CLInitNode init) { super(init); diff --git a/chapter25/src/main/java/com/seaofnodes/simple/node/CallNode.java b/chapter25/src/main/java/com/seaofnodes/simple/node/CallNode.java index 38117f23..b4bb5e72 100644 --- a/chapter25/src/main/java/com/seaofnodes/simple/node/CallNode.java +++ b/chapter25/src/main/java/com/seaofnodes/simple/node/CallNode.java @@ -22,16 +22,17 @@ public class CallNode extends CFGNode { // Source location for late reported errors public final Parser.Lexer _loc; - public CallNode(Parser.Lexer loc, Node... nodes) { this(loc,loc==null ? 0 : loc.lineColumn(),nodes); } - private CallNode(Parser.Lexer loc, long position, Node... nodes) { super(nodes); _loc=loc; } + public CallNode(Parser.Lexer loc, Node... nodes) { super(nodes); _loc=loc; } public CallNode(CallNode call) { super(call); _loc = call._loc; } @Override public Tag serialTag() { return Tag.Call; } public void packed( BAOS baos, HashMap<String,Integer> strs, HashMap<Type,Integer> types, IdentityHashMap<Node, Integer> anodes ) { baos.packed1(nIns()); + Parser.Lexer.packed(_loc,baos,strs); } - static Node make( BAOS bais ) { + @Override public void gather(HashMap<String,Integer> strs) { Parser.Lexer.gather(_loc,strs); } + static Node make( BAOS bais, String[] strs ) { int nIns = bais.packed1(); - return new CallNode(null,new Node[nIns]); + return new CallNode(Parser.Lexer.unpack(bais,strs),new Node[nIns]); } @Override protected ExprPrinter<Node> _print1(ExprPrinter<Node> p) { diff --git a/chapter25/src/main/java/com/seaofnodes/simple/node/Node.java b/chapter25/src/main/java/com/seaofnodes/simple/node/Node.java index 796965b5..77b43ba8 100644 --- a/chapter25/src/main/java/com/seaofnodes/simple/node/Node.java +++ b/chapter25/src/main/java/com/seaofnodes/simple/node/Node.java @@ -112,9 +112,9 @@ public abstract class Node extends BaseNode<Node> implements Cloneable { case LE -> new BoolNode.LE (null,null,(byte)bais.packed1()); case ULT -> new BoolNode.ULT(null,null,(byte)bais.packed1()); - case Call -> CallNode.make(bais); + case Call -> CallNode.make(bais,strs); case CallEnd-> CallEndNode.make(bais ,types); - case CLInit -> new CLInitNode(bais); + case CLInit -> new CLInitNode(bais,strs); case CProj -> CProjNode.make(bais,strs); case Con -> ConstantNode.make(bais ,types); case Escape-> EscapeNode.make(bais ,types); diff --git a/chapter25/src/test/java/com/seaofnodes/simple/ClassInitDependenciesTest.java b/chapter25/src/test/java/com/seaofnodes/simple/ClassInitDependenciesTest.java index 1dec3312..e3af8cce 100644 --- a/chapter25/src/test/java/com/seaofnodes/simple/ClassInitDependenciesTest.java +++ b/chapter25/src/test/java/com/seaofnodes/simple/ClassInitDependenciesTest.java @@ -12,7 +12,10 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.BitSet; import java.util.Comparator; +import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TestRule; @@ -66,18 +69,34 @@ public class ClassInitDependenciesTest { } } - // The cycle can start at any member, but must follow dependency direction. + // The report lists internal SCC edges, including ordinary helper calls. + private HashMap<String,HashSet<String>> reportEdges(String message) { + String[] lines=message.split("\n"); + assertEquals(message,"Cyclic class initialization:",lines[0]); + HashMap<String,HashSet<String>> edges=new HashMap<>(); + for( int i=1; i<lines.length; i++ ) { + if( !lines[i].startsWith(" ") || lines[i].startsWith(" ") ) continue; // Diagnostic/source/caret. + String[] edge=lines[i].trim().split(" -> "); + assertEquals(message,2,edge.length); + edges.computeIfAbsent(edge[0],key -> new HashSet<>()).add(edge[1]); + } + return edges; + } + + // Follow helpers in the diagnostic, stopping at the next class initializer. + private boolean reportedTouch(HashMap<String,HashSet<String>> edges, String from, String to, HashSet<String> seen) { + if( !seen.add(from) || !edges.containsKey(from) ) return false; + for( String dst : edges.get(from) ) + if( dst.equals(to) || (dst.contains(" [fidx ") && reportedTouch(edges,dst,to,seen)) ) return true; + return false; + } + private void assertCycle(String message, String expected) { - String prefix="Cyclic class initialization: "; - assertTrue(message,message.startsWith(prefix)); - String actual=message.substring(prefix.length()).trim(); + HashMap<String,HashSet<String>> edges=reportEdges(message); String[] path=expected.split(" -> "); - for( int start=0; start<path.length-1; start++ ) { - ArrayList<String> rotated=new ArrayList<>(); - for( int i=0; i<path.length; i++ ) rotated.add(path[(start+i)%(path.length-1)]); - if( actual.equals(String.join(" -> ",rotated)) ) return; - } - fail("Expected cycle "+expected+" (up to rotation), got "+actual); + for( int i=1; i<path.length; i++ ) + assertTrue("Missing "+path[i-1]+" -> "+path[i]+" in:\n"+message, + reportedTouch(edges,path[i-1],path[i],new HashSet<>())); } // Follow ordinary helpers, stopping at the next initializer. This checks @@ -204,8 +223,10 @@ public class ClassInitDependenciesTest { object(stubs,stubs,"B","int value=arg;"); // Each object is independently acyclic. Combining their real bodies // must recheck the graph rather than trust either object's local order. - object(dir,stubs,"A","int value=arg;\nvalue=B.value;"); - object(dir,stubs,"B","int value=arg; value=A.value;"); + 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"} ) { try { new CodeGen(null,dir.toString(),new Ary<>(new String[]{dir.toString()}), @@ -213,8 +234,21 @@ public class ClassInitDependenciesTest { 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")); } } + // The imported object must still report its saved path and offset + // when the original source is no longer available. + Files.delete(Path.of(afile)); + try { + new CodeGen(null,dir.toString(),new Ary<>(new String[]{dir.toString()}), + "Main","return A.value;",123,true,TypeInteger.BOT).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("\n"+afile+": error: cyclic class initialization (source unavailable at byte offset "+asrc.indexOf('.')+")")); + } } @Test public void testImportedDiamondAndHelpers() throws Exception { @@ -276,31 +310,88 @@ public class ClassInitDependenciesTest { @Test public void testCycleReportDirection() throws Exception { cycle("A",Map.of("A","int value=B.value;", "B","int value=C.value;", "C","int value=A.value;"),"A -> B -> C -> A"); + for( long seed : new long[]{1,123,987654321} ) { + // Stack order A,B,C is not a cycle: B has no edge to C. All four + // internal edges must be shown; edges to Root/D must be omitted. + try { + compile("A",Map.of("A","int value=B.value+C.value+D.value;", + "B","int value=A.value;", "C","int value=A.value;", + "D","int value=arg;"),seed); + fail("Expected initialization cycle"); + } catch( Parser.ParseException ex ) { + assertEquals(Map.of("Root.A",Set.of("Root.B","Root.C"), + "Root.B",Set.of("Root.A"), "Root.C",Set.of("Root.A")), + reportEdges(ex.getMessage())); + } + } } - @Test public void testOverlappingFunctionAndClassCycles() { - // Fixed function graph: Opto/inlining must not erase the overlap that - // exercises SCC traversal. f -> g -> f is legal; f -> A -> g -> f isn't. - CodeGen code=new CodeGen("return 0;").driver(CodeGen.Phase.Opto); + @Test public void testCycleSourceLocations() throws Exception { + for( String name : new String[]{"read", "read_noInline"} ) { + try { + compile("A",Map.of("A","int value=Helper."+name+"();", + "Helper","val "+name+"={ ->\n B.value;\n};", + "B","int value=A.value;"),123); + fail("Expected initialization cycle"); + } catch( Parser.ParseException ex ) { + String message=ex.getMessage(); + assertCycle(message,"Root.A -> Root.B -> Root.A"); + // Inlining must keep Helper's source identity, not substitute A. + assertTrue(message,message.contains("Helper.smp:2:4")); + assertTrue(message,message.contains(" B.value;\n ---^")); + if( name.endsWith("noInline") ) { + assertTrue(message,message.contains("A.smp:1:31")); + assertTrue(message,message.contains("int value=Helper.read_noInline();")); + } + } + } + } + + @Test public void testSourceError() { + String src="// π\r\n\treturn 1;\r\n"; + CodeGen code=new CodeGen(src).driver(CodeGen.Phase.Parse); CompUnit cu=code._compunits.get("Test"); - code._linker.clear(); - String[] names={"f", "g", "A.<clinit>"}; + int pos=src.substring(0,src.indexOf('1')).getBytes(java.nio.charset.StandardCharsets.UTF_8).length; + assertEquals("Test.smp:2:9: error: example\n return 1;\n ---------------^",SourceError.format(cu,pos,"example")); + assertEquals("Test.smp:3:1: error: example\n \n ^",SourceError.format(cu,src.getBytes(java.nio.charset.StandardCharsets.UTF_8).length,"example")); + } + + @Test public void testSelfInitialization() throws Exception { + for( String name : new String[]{"read","read_noInline"} ) + for( long seed : new long[]{1,123,987654321} ) + compile("A",Map.of("A","int value=arg; val "+name+"={ -> A.value; }; int result="+name+"();"),seed); + } + + @Test public void testOverlappingFunctionAndClassCycles() throws Exception { + // Fixed function graph: Opto/inlining must not erase the overlap that + // exercises SCC traversal. One clinit is legal, two in the SCC are not. + CodeGen code=compile("A",Map.of("A","val b=B; int value=arg;", "B","int value=arg;"),123); + int f=code._linker._len, g=f+1, a=f+2, b=f+3; + String[] names={"f", "g", "A.<clinit>", "B.<clinit>"}; for( int i=0; i<names.length; i++ ) { - TypeFunPtr sig=TypeFunPtr.make1((byte)2,false,new Type[0],TypeInteger.BOT,i+2); + TypeFunPtr sig=TypeFunPtr.make1((byte)2,false,new Type[0],TypeInteger.BOT,i+f); + CompUnit cu=code._compunits.get(i==3 ? "Root/B" : "Root/A"); FunNode fun=new FunNode(null,sig,names[i],cu); fun._type=Type.CONTROL; - code._linker.setX(i+2,fun); + code._linker.setX(i+f,fun); } ClassInitDependencies deps=new ClassInitDependencies(code); - deps._edgess[2]=new int[]{3}; // f -> g - deps._edgess[3]=new int[]{2}; // g -> f + deps._edgess[f]=new int[]{g}; // f -> g + deps._edgess[g]=new int[]{f}; // g -> f assertNull("Ordinary mutual recursion is legal",deps.check()); - deps._edgess[4]=new int[]{3}; // A -> g - deps._edgess[2]=new int[]{3,4}; + deps._edgess[a]=new int[]{a}; // A -> A + assertNull("A singleton initializer self-edge is legal",deps.check()); + deps._edgess[a]=new int[]{g}; // A -> g + deps._edgess[f]=new int[]{g,a}; + assertNull("A cycle containing only one initializer is legal",deps.check()); + deps._edgess[a]=new int[]{b}; // A -> B + deps._edgess[b]=new int[]{g}; // B -> g -> f -> A String forward=deps.check(); - deps._edgess[2]=new int[]{4,3}; + deps._edgess[f]=new int[]{a,g}; String reverse=deps.check(); assertTrue("Both successor orders must reject the class cycle: "+forward+", "+reverse, forward!=null && reverse!=null); + assertCycle(forward,"Root.A -> Root.B -> Root.A"); + assertCycle(reverse,"Root.A -> Root.B -> Root.A"); } } diff --git a/skills/chapter25-codex-notes.md b/skills/chapter25-codex-notes.md index b9b30ab5..f76c6fd6 100644 --- a/skills/chapter25-codex-notes.md +++ b/skills/chapter25-codex-notes.md @@ -29,6 +29,8 @@ For shared viewer work, read [the graph viewer notes](graph-viewer-codex-notes.m ## Collaboration preferences +- For densely keyed integer maps, use indexed `Ary<T>` storage rather than + `HashMap<Integer,T>`. - Build with Make alone; Maven must never be required. Cliff prefers few tests: avoid tests for every small viewer edit or rapidly changing display experiment. Use focused existing tests and disposable probes when they answer a real risk. @@ -72,6 +74,19 @@ For shared viewer work, read [the graph viewer notes](graph-viewer-codex-notes.m ## Mutability and chapter boundaries +- Class-init cycle validation (2026-10-14): reject an SCC only when it contains + more than one `<clinit>`. A self-edge or helper recursion with one initializer + is legal, regardless of inlining. Error reports list every internal edge of + the first rejected SCC, including helper calls; stack order is not a path. +- Class-cycle diagnostics use `SourceError.format(CompUnit, byteOffset, message)`. + Start `file:line:column: error: message` in column one for Emacs compilation + buffers; put the source and caret on separate following lines. With no source, + report the path and saved byte offset, without inventing line/column numbers. + Lexer snapshots keep their original CompUnit through inlining and Call/CLInit + serialization; recover representative sites by rescanning only on rejection. + Source text is assumed available. Imported source paths are diagnostic data, + not `_smp` rebuild/discovery state. General error-message migration is deferred. + - Constructors start in 16, independent binding/access permissions in 17a, syntax sugar and inference in 17b. Historical Chapter 17 references mean 17b. - `!Point` is writable access, `~Point` deep read-only access; `!p` allows |