diff options
40 files changed, 1185 insertions, 25 deletions
diff --git a/chapter10/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter10/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index 843ce733..455f4b3a 100644 --- a/chapter10/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter10/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -254,6 +254,13 @@ public class ScopeNode extends Node { // combinations, and replaces the variable with the upcast variant. public Node upcast( Node ctrl, Node pred, boolean invert ) { if( ctrl._type==Type.XCONTROL ) return null; + // A single negation is handled below. For !!p (including p != null), + // also refine the underlying p without changing the Boolean result. + if( pred instanceof NotNode not && not.in(1) instanceof NotNode ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + upcast(ctrl,not.in(1),!invert); + pred.unkeep(); + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : IterPeeps.add(new NotNode(pred).peephole()); diff --git a/chapter10/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter10/src/test/java/com/seaofnodes/simple/Chapter10Test.java index f2e7ca2c..c39c19b3 100644 --- a/chapter10/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter10/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -12,6 +12,46 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point point = new Point; + point.x = 42; + Point? p = null; + if (arg) p = point; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; else return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;" + } ) { + StopNode stop = new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + assertEquals(body,-1L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,0)); + assertEquals(body,42L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,1)); + } + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!point) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { Parser parser = new Parser( diff --git a/chapter11/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter11/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index 843ce733..455f4b3a 100644 --- a/chapter11/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter11/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -254,6 +254,13 @@ public class ScopeNode extends Node { // combinations, and replaces the variable with the upcast variant. public Node upcast( Node ctrl, Node pred, boolean invert ) { if( ctrl._type==Type.XCONTROL ) return null; + // A single negation is handled below. For !!p (including p != null), + // also refine the underlying p without changing the Boolean result. + if( pred instanceof NotNode not && not.in(1) instanceof NotNode ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + upcast(ctrl,not.in(1),!invert); + pred.unkeep(); + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : IterPeeps.add(new NotNode(pred).peephole()); diff --git a/chapter11/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter11/src/test/java/com/seaofnodes/simple/Chapter10Test.java index 7371ff18..6969358d 100644 --- a/chapter11/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter11/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -7,6 +7,46 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point point = new Point; + point.x = 42; + Point? p = null; + if (arg) p = point; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; else return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;" + } ) { + StopNode stop = new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + assertEquals(body,-1L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,0)); + assertEquals(body,42L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,1)); + } + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!point) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { Parser parser = new Parser( diff --git a/chapter12/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter12/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index 843ce733..455f4b3a 100644 --- a/chapter12/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter12/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -254,6 +254,13 @@ public class ScopeNode extends Node { // combinations, and replaces the variable with the upcast variant. public Node upcast( Node ctrl, Node pred, boolean invert ) { if( ctrl._type==Type.XCONTROL ) return null; + // A single negation is handled below. For !!p (including p != null), + // also refine the underlying p without changing the Boolean result. + if( pred instanceof NotNode not && not.in(1) instanceof NotNode ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + upcast(ctrl,not.in(1),!invert); + pred.unkeep(); + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : IterPeeps.add(new NotNode(pred).peephole()); diff --git a/chapter12/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter12/src/test/java/com/seaofnodes/simple/Chapter10Test.java index 4a9521b9..6f8a6b3b 100644 --- a/chapter12/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter12/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -12,6 +12,46 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point point = new Point; + point.x = 42; + Point? p = null; + if (arg) p = point; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; else return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;" + } ) { + StopNode stop = new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + assertEquals(body,-1L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,0)); + assertEquals(body,42L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,1)); + } + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!point) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { Parser parser = new Parser( diff --git a/chapter13/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter13/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index 843ce733..455f4b3a 100644 --- a/chapter13/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter13/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -254,6 +254,13 @@ public class ScopeNode extends Node { // combinations, and replaces the variable with the upcast variant. public Node upcast( Node ctrl, Node pred, boolean invert ) { if( ctrl._type==Type.XCONTROL ) return null; + // A single negation is handled below. For !!p (including p != null), + // also refine the underlying p without changing the Boolean result. + if( pred instanceof NotNode not && not.in(1) instanceof NotNode ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + upcast(ctrl,not.in(1),!invert); + pred.unkeep(); + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : IterPeeps.add(new NotNode(pred).peephole()); diff --git a/chapter13/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter13/src/test/java/com/seaofnodes/simple/Chapter10Test.java index c9f695c9..124cea6d 100644 --- a/chapter13/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter13/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -7,6 +7,46 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point point = new Point; + point.x = 42; + Point? p = null; + if (arg) p = point; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;" + } ) { + StopNode stop = new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + assertEquals(body,-1L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,0)); + assertEquals(body,42L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,1)); + } + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!point) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { Parser parser = new Parser( diff --git a/chapter14/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter14/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index 843ce733..455f4b3a 100644 --- a/chapter14/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter14/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -254,6 +254,13 @@ public class ScopeNode extends Node { // combinations, and replaces the variable with the upcast variant. public Node upcast( Node ctrl, Node pred, boolean invert ) { if( ctrl._type==Type.XCONTROL ) return null; + // A single negation is handled below. For !!p (including p != null), + // also refine the underlying p without changing the Boolean result. + if( pred instanceof NotNode not && not.in(1) instanceof NotNode ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + upcast(ctrl,not.in(1),!invert); + pred.unkeep(); + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : IterPeeps.add(new NotNode(pred).peephole()); diff --git a/chapter14/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter14/src/test/java/com/seaofnodes/simple/Chapter10Test.java index fb324fd9..bfe3b1d3 100644 --- a/chapter14/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter14/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -7,6 +7,46 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point point = new Point; + point.x = 42; + Point? p = null; + if (arg) p = point; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;" + } ) { + StopNode stop = new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + assertEquals(body,-1L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,0)); + assertEquals(body,42L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,1)); + } + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!point) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { Parser parser = new Parser( diff --git a/chapter15/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter15/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index 877ae4a4..9c4c4403 100644 --- a/chapter15/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter15/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -252,6 +252,13 @@ public class ScopeNode extends Node { // combinations, and replaces the variable with the upcast variant. public Node upcast( Node ctrl, Node pred, boolean invert ) { if( ctrl._type==Type.XCONTROL ) return null; + // A single negation is handled below. For !!p (including p != null), + // also refine the underlying p without changing the Boolean result. + if( pred instanceof NotNode not && not.in(1) instanceof NotNode ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + upcast(ctrl,not.in(1),!invert); + pred.unkeep(); + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : IterPeeps.add(new NotNode(pred).peephole()); diff --git a/chapter15/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter15/src/test/java/com/seaofnodes/simple/Chapter10Test.java index def9aaa6..cf817b28 100644 --- a/chapter15/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter15/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -7,6 +7,47 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point?[] points = new Point?[2]; + Point point = new Point; + point.x = 42; + points[arg] = point; + Point? p = points[1]; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;" + } ) { + StopNode stop = new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + assertEquals(body,-1L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,0)); + assertEquals(body,42L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,1)); + } + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!points) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { Parser parser = new Parser( diff --git a/chapter16/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter16/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index 11e54d82..02baeb8a 100644 --- a/chapter16/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter16/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -268,6 +268,13 @@ public class ScopeNode extends ScopeMinNode { // This Scope looks for direct variable uses, or certain simple // combinations, and replaces the variable with the upcast variant. public Node upcast( Node ctrl, Node pred, boolean invert ) { + // A single negation is handled below. For !!p (including p != null), + // also refine the underlying p without changing the Boolean result. + if( pred instanceof NotNode not && not.in(1) instanceof NotNode ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + upcast(ctrl,not.in(1),!invert); + pred.unkeep(); + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : IterPeeps.add(new NotNode(pred).peephole()); diff --git a/chapter16/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter16/src/test/java/com/seaofnodes/simple/Chapter10Test.java index 2a33031d..ccc09dcb 100644 --- a/chapter16/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter16/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -8,6 +8,45 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point?[] !points = new Point?[2]; + points[arg] = new Point { x = 42; }; + Point? p = points[1]; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;" + } ) { + StopNode stop = new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + assertEquals(body,-1L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,0)); + assertEquals(body,42L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,1)); + } + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!points) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { Parser parser = new Parser( diff --git a/chapter17/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter17/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index 86aa7665..e71c669f 100644 --- a/chapter17/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter17/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -287,10 +287,23 @@ public class ScopeNode extends ScopeMinNode { // E.g. "if( ptr ) ptr.field;" is legal because ptr is known not-null. public void addGuards( Node ctrl, Node pred, boolean invert ) { assert ctrl instanceof CFGNode; - _guards.add(ctrl); // Marker to distinguish 0,1,2 guards + _guards.add(ctrl); // Marker between guard sets // add pred & its cast to the normal input list, with special Vars if( ctrl._type == Type.XCONTROL || pred==null || pred.isDead() ) return; // Dead, do not add any guards + _addGuards(ctrl,pred,invert); + } + + private void _addGuards( Node ctrl, Node pred, boolean invert ) { + if( pred==null || pred.isDead() ) + return; + // A single negation is handled below. For !!p (including p != null), + // also refine the underlying p without changing the Boolean result. + if( pred instanceof NotNode not && not.in(1) instanceof NotNode ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + _addGuards(ctrl,not.in(1),!invert); + pred.unkeep(); + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : IterPeeps.add(new NotNode(pred).peephole()); @@ -322,7 +335,7 @@ public class ScopeNode extends ScopeMinNode { // Remove matching pred/cast pairs from this guarded region. public void removeGuards( Node ctrl ) { assert ctrl instanceof CFGNode; - // 0,1 or 2 guards + // Pop the guards up to this region's marker. while( true ) { Node g = _guards.pop(); if( g == ctrl ) break; diff --git a/chapter17/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter17/src/test/java/com/seaofnodes/simple/Chapter10Test.java index d41c83d8..d6f6bcd3 100644 --- a/chapter17/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter17/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -8,6 +8,45 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point?[] !points = new Point?[2]; + points[arg] = new Point { x = 42; }; + Point? p = points[1]; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;" + } ) { + StopNode stop = new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + assertEquals(body,-1L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,0)); + assertEquals(body,42L,com.seaofnodes.simple.evaluator.Evaluator.evaluate(stop,1)); + } + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!points) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new Parser(NULLABLE_POINT_SOURCE+body).parse().iterate(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { Parser parser = new Parser( diff --git a/chapter18/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter18/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index d0d86b79..675cd197 100644 --- a/chapter18/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter18/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -345,10 +345,23 @@ public class ScopeNode extends MemMergeNode { // E.g. "if( ptr ) ptr.field;" is legal because ptr is known not-null. public void addGuards( Node ctrl, Node pred, boolean invert ) { assert ctrl instanceof CFGNode; - _guards.add(ctrl); // Marker to distinguish 0,1,2 guards + _guards.add(ctrl); // Marker between guard sets // add pred & its cast to the normal input list, with special Vars if( pred==null || pred.isDead() ) return; // Dead, do not add any guards + _addGuards(ctrl,pred,invert); + } + + private void _addGuards( Node ctrl, Node pred, boolean invert ) { + if( pred==null || pred.isDead() ) + return; + // A single negation is handled below. For !!p (including p != null), + // also refine the underlying p without changing the Boolean result. + if( pred instanceof NotNode not && not.in(1) instanceof NotNode ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + _addGuards(ctrl,not.in(1),!invert); + pred.unkeep(); + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : IterPeeps.add(new NotNode(pred).peephole()); @@ -380,7 +393,7 @@ public class ScopeNode extends MemMergeNode { // Remove matching pred/cast pairs from this guarded region. public ScopeNode removeGuards( Node ctrl ) { assert ctrl instanceof CFGNode; - // 0,1 or 2 guards + // Pop the guards up to this region's marker. while( true ) { Node g = _guards.pop(); if( g == ctrl ) break; diff --git a/chapter18/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter18/src/test/java/com/seaofnodes/simple/Chapter10Test.java index 02029b3f..42684416 100644 --- a/chapter18/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter18/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -6,6 +6,45 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point?[] !points = new Point?[2]; + points[arg] = new Point { x = 42; }; + Point? p = points[1]; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;" + } ) { + CodeGen code = new CodeGen(NULLABLE_POINT_SOURCE+body).parse().opto().typeCheck(); + assertEquals(body,"-1",Eval2.eval(code,0)); + assertEquals(body,"42",Eval2.eval(code,1)); + } + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!points) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new CodeGen(NULLABLE_POINT_SOURCE+body).parse().opto().typeCheck(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { CodeGen code = new CodeGen( diff --git a/chapter18/src/test/java/com/seaofnodes/simple/Eval2.java b/chapter18/src/test/java/com/seaofnodes/simple/Eval2.java index 2da834e5..50207d8c 100644 --- a/chapter18/src/test/java/com/seaofnodes/simple/Eval2.java +++ b/chapter18/src/test/java/com/seaofnodes/simple/Eval2.java @@ -268,7 +268,7 @@ public abstract class Eval2 { case MulFNode mlf -> d(mlf.in(1)) * d(mlf.in(2)); case MulNode mul -> x(mul.in(1)) * x(mul.in(2)); case NewNode alloc-> alloc(alloc); - case NotNode not -> x(not.in(1)) == 0 ? 1L : 0L; + case NotNode not -> not(not); case OrNode or -> x(or .in(1)) | x(or .in(2)); case ProjNode proj -> proj._type instanceof TypeMem ? "$mem" : val(proj.in(0)); case ReadOnlyNode read -> val(read.in(1)); @@ -285,6 +285,12 @@ public abstract class Eval2 { }; } + private static Object not( NotNode not ) { + Object n = val(not.in(1)); + return ( n==null || (n instanceof Double D && D==0.0) || n instanceof Long L && L==0 ) + ? 1L : 0L; + } + // Fetch without unboxing, searching up-Frame static Object val( Node n ) { return F.get(n); } // Fetch and unbox as primitive long diff --git a/chapter19/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter19/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index 1208ff63..913c2669 100644 --- a/chapter19/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter19/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -345,10 +345,23 @@ public class ScopeNode extends MemMergeNode { // E.g. "if( ptr ) ptr.field;" is legal because ptr is known not-null. public void addGuards( Node ctrl, Node pred, boolean invert ) { assert ctrl instanceof CFGNode; - _guards.add(ctrl); // Marker to distinguish 0,1,2 guards + _guards.add(ctrl); // Marker between guard sets // add pred & its cast to the normal input list, with special Vars if( pred==null || pred.isDead() ) return; // Dead, do not add any guards + _addGuards(ctrl,pred,invert); + } + + private void _addGuards( Node ctrl, Node pred, boolean invert ) { + if( pred==null || pred.isDead() ) + return; + // A single negation is handled below. For !!p (including p != null), + // also refine the underlying p without changing the Boolean result. + if( pred instanceof NotNode not && not.in(1) instanceof NotNode ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + _addGuards(ctrl,not.in(1),!invert); + pred.unkeep(); + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : CodeGen.CODE.add(new NotNode(pred).peephole()); @@ -380,7 +393,7 @@ public class ScopeNode extends MemMergeNode { // Remove matching pred/cast pairs from this guarded region. public ScopeNode removeGuards( Node ctrl ) { assert ctrl instanceof CFGNode; - // 0,1 or 2 guards + // Pop the guards up to this region's marker. while( true ) { Node g = _guards.pop(); if( g == ctrl ) break; diff --git a/chapter19/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter19/src/test/java/com/seaofnodes/simple/Chapter10Test.java index 02029b3f..42684416 100644 --- a/chapter19/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter19/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -6,6 +6,45 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point?[] !points = new Point?[2]; + points[arg] = new Point { x = 42; }; + Point? p = points[1]; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;" + } ) { + CodeGen code = new CodeGen(NULLABLE_POINT_SOURCE+body).parse().opto().typeCheck(); + assertEquals(body,"-1",Eval2.eval(code,0)); + assertEquals(body,"42",Eval2.eval(code,1)); + } + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!points) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new CodeGen(NULLABLE_POINT_SOURCE+body).parse().opto().typeCheck(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { CodeGen code = new CodeGen( diff --git a/chapter19/src/test/java/com/seaofnodes/simple/Eval2.java b/chapter19/src/test/java/com/seaofnodes/simple/Eval2.java index 6ae8427c..24761945 100644 --- a/chapter19/src/test/java/com/seaofnodes/simple/Eval2.java +++ b/chapter19/src/test/java/com/seaofnodes/simple/Eval2.java @@ -268,7 +268,7 @@ public abstract class Eval2 { case MulFNode mlf -> d(mlf.in(1)) * d(mlf.in(2)); case MulNode mul -> x(mul.in(1)) * x(mul.in(2)); case NewNode alloc-> alloc(alloc); - case NotNode not -> x(not.in(1)) == 0 ? 1L : 0L; + case NotNode not -> not(not); case OrNode or -> x(or .in(1)) | x(or .in(2)); case ProjNode proj -> proj._type instanceof TypeMem ? "$mem" : val(proj.in(0)); case ReadOnlyNode read -> val(read.in(1)); @@ -285,6 +285,12 @@ public abstract class Eval2 { }; } + private static Object not( NotNode not ) { + Object n = val(not.in(1)); + return ( n==null || (n instanceof Double D && D==0.0) || n instanceof Long L && L==0 ) + ? 1L : 0L; + } + // Fetch without unboxing, searching up-Frame static Object val( Node n ) { return F.get(n); } // Fetch and unbox as primitive long diff --git a/chapter20/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter20/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index f193e55a..3411756d 100644 --- a/chapter20/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter20/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -346,10 +346,23 @@ public class ScopeNode extends MemMergeNode { // E.g. "if( ptr ) ptr.field;" is legal because ptr is known not-null. public void addGuards( Node ctrl, Node pred, boolean invert ) { assert ctrl instanceof CFGNode; - _guards.add(ctrl); // Marker to distinguish 0,1,2 guards + _guards.add(ctrl); // Marker between guard sets // add pred & its cast to the normal input list, with special Vars if( pred==null || pred.isDead() ) return; // Dead, do not add any guards + _addGuards(ctrl,pred,invert); + } + + private void _addGuards( Node ctrl, Node pred, boolean invert ) { + if( pred==null || pred.isDead() ) + return; + // A single negation is handled below. For !!p (including p != null), + // also refine the underlying p without changing the Boolean result. + if( pred instanceof NotNode not && not.in(1) instanceof NotNode ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + _addGuards(ctrl,not.in(1),!invert); + pred.unkeep(); + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : CodeGen.CODE.add(new NotNode(pred).peephole()); @@ -381,7 +394,7 @@ public class ScopeNode extends MemMergeNode { // Remove matching pred/cast pairs from this guarded region. public ScopeNode removeGuards( Node ctrl ) { assert ctrl instanceof CFGNode; - // 0,1 or 2 guards + // Pop the guards up to this region's marker. while( true ) { Node g = _guards.pop(); if( g == ctrl ) break; diff --git a/chapter20/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter20/src/test/java/com/seaofnodes/simple/Chapter10Test.java index 1253e4a6..e2cbf331 100644 --- a/chapter20/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter20/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -7,6 +7,45 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point?[] !points = new Point?[2]; + points[arg] = new Point { x = 42; }; + Point? p = points[1]; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;" + } ) { + CodeGen code = new CodeGen(NULLABLE_POINT_SOURCE+body).parse().opto().typeCheck(); + assertEquals(body,"-1",Eval2.eval(code,0)); + assertEquals(body,"42",Eval2.eval(code,1)); + } + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!points) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new CodeGen(NULLABLE_POINT_SOURCE+body).parse().opto().typeCheck(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { CodeGen code = new CodeGen( diff --git a/chapter20/src/test/java/com/seaofnodes/simple/Eval2.java b/chapter20/src/test/java/com/seaofnodes/simple/Eval2.java index ecea97dd..3e1954aa 100644 --- a/chapter20/src/test/java/com/seaofnodes/simple/Eval2.java +++ b/chapter20/src/test/java/com/seaofnodes/simple/Eval2.java @@ -270,7 +270,7 @@ public abstract class Eval2 { case MulFNode mlf -> d(mlf.in(1)) * d(mlf.in(2)); case MulNode mul -> x(mul.in(1)) * x(mul.in(2)); case NewNode alloc-> alloc(alloc); - case NotNode not -> x(not.in(1)) == 0 ? 1L : 0L; + case NotNode not -> not(not); case OrNode or -> x(or .in(1)) | x(or .in(2)); case ProjNode proj -> proj._type instanceof TypeMem ? "$mem" : val(proj.in(0)); case ReadOnlyNode read -> val(read.in(1)); @@ -287,6 +287,12 @@ public abstract class Eval2 { }; } + private static Object not( NotNode not ) { + Object n = val(not.in(1)); + return ( n==null || (n instanceof Double D && D==0.0) || n instanceof Long L && L==0 ) + ? 1L : 0L; + } + // Fetch without unboxing, searching up-Frame static Object val( Node n ) { return F.get(n); } // Fetch and unbox as primitive long diff --git a/chapter21/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter21/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index 38a5d968..38343648 100644 --- a/chapter21/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter21/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -346,10 +346,23 @@ public class ScopeNode extends MemMergeNode { // E.g. "if( ptr ) ptr.field;" is legal because ptr is known not-null. public void addGuards( Node ctrl, Node pred, boolean invert ) { assert ctrl instanceof CFGNode; - _guards.add(ctrl); // Marker to distinguish 0,1,2 guards + _guards.add(ctrl); // Marker between guard sets // add pred & its cast to the normal input list, with special Vars if( pred==null || pred.isDead() ) return; // Dead, do not add any guards + _addGuards(ctrl,pred,invert); + } + + private void _addGuards( Node ctrl, Node pred, boolean invert ) { + if( pred==null || pred.isDead() ) + return; + // A single negation is handled below. For !!p (including p != null), + // also refine the underlying p without changing the Boolean result. + if( pred instanceof NotNode not && not.in(1) instanceof NotNode ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + _addGuards(ctrl,not.in(1),!invert); + pred.unkeep(); + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : CodeGen.CODE.add(new NotNode(pred).peephole()); @@ -381,7 +394,7 @@ public class ScopeNode extends MemMergeNode { // Remove matching pred/cast pairs from this guarded region. public ScopeNode removeGuards( Node ctrl ) { assert ctrl instanceof CFGNode; - // 0,1 or 2 guards + // Pop the guards up to this region's marker. while( true ) { Node g = _guards.pop(); if( g == ctrl ) break; diff --git a/chapter21/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter21/src/test/java/com/seaofnodes/simple/Chapter10Test.java index 1253e4a6..e2cbf331 100644 --- a/chapter21/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter21/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -7,6 +7,45 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point?[] !points = new Point?[2]; + points[arg] = new Point { x = 42; }; + Point? p = points[1]; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;" + } ) { + CodeGen code = new CodeGen(NULLABLE_POINT_SOURCE+body).parse().opto().typeCheck(); + assertEquals(body,"-1",Eval2.eval(code,0)); + assertEquals(body,"42",Eval2.eval(code,1)); + } + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!points) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new CodeGen(NULLABLE_POINT_SOURCE+body).parse().opto().typeCheck(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { CodeGen code = new CodeGen( diff --git a/chapter21/src/test/java/com/seaofnodes/simple/Eval2.java b/chapter21/src/test/java/com/seaofnodes/simple/Eval2.java index ecea97dd..3e1954aa 100644 --- a/chapter21/src/test/java/com/seaofnodes/simple/Eval2.java +++ b/chapter21/src/test/java/com/seaofnodes/simple/Eval2.java @@ -270,7 +270,7 @@ public abstract class Eval2 { case MulFNode mlf -> d(mlf.in(1)) * d(mlf.in(2)); case MulNode mul -> x(mul.in(1)) * x(mul.in(2)); case NewNode alloc-> alloc(alloc); - case NotNode not -> x(not.in(1)) == 0 ? 1L : 0L; + case NotNode not -> not(not); case OrNode or -> x(or .in(1)) | x(or .in(2)); case ProjNode proj -> proj._type instanceof TypeMem ? "$mem" : val(proj.in(0)); case ReadOnlyNode read -> val(read.in(1)); @@ -287,6 +287,12 @@ public abstract class Eval2 { }; } + private static Object not( NotNode not ) { + Object n = val(not.in(1)); + return ( n==null || (n instanceof Double D && D==0.0) || n instanceof Long L && L==0 ) + ? 1L : 0L; + } + // Fetch without unboxing, searching up-Frame static Object val( Node n ) { return F.get(n); } // Fetch and unbox as primitive long diff --git a/chapter22/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter22/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index 3127e491..828fa4d6 100644 --- a/chapter22/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter22/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -350,10 +350,23 @@ public class ScopeNode extends MemMergeNode { // E.g. "if( ptr ) ptr.field;" is legal because ptr is known not-null. public void addGuards( Node ctrl, Node pred, boolean invert ) { assert ctrl instanceof CFGNode; - _guards.add(ctrl); // Marker to distinguish 0,1,2 guards + _guards.add(ctrl); // Marker between guard sets // add pred & its cast to the normal input list, with special Vars if( pred==null || pred.isDead() ) return; // Dead, do not add any guards + _addGuards(ctrl,pred,invert); + } + + private void _addGuards( Node ctrl, Node pred, boolean invert ) { + if( pred==null || pred.isDead() ) + return; + // A single negation is handled below. For !!p (including p != null), + // also refine the underlying p without changing the Boolean result. + if( pred instanceof NotNode not && not.in(1) instanceof NotNode ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + _addGuards(ctrl,not.in(1),!invert); + pred.unkeep(); + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : CodeGen.CODE.add(new NotNode(pred).peephole()); @@ -385,7 +398,7 @@ public class ScopeNode extends MemMergeNode { // Remove matching pred/cast pairs from this guarded region. public ScopeNode removeGuards( Node ctrl ) { assert ctrl instanceof CFGNode; - // 0,1 or 2 guards + // Pop the guards up to this region's marker. while( true ) { Node g = _guards.pop(); if( g == ctrl ) break; diff --git a/chapter22/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter22/src/test/java/com/seaofnodes/simple/Chapter10Test.java index 712704db..743d5d85 100644 --- a/chapter22/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter22/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -7,6 +7,45 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point?[] !points = new Point?[2]; + points[arg] = new Point { x = 42; }; + Point? p = points[1]; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;" + } ) { + CodeGen code = new CodeGen(NULLABLE_POINT_SOURCE+body).parse().opto().typeCheck(); + assertEquals(body,"-1",Eval2.eval(code,0)); + assertEquals(body,"42",Eval2.eval(code,1)); + } + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!points) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new CodeGen(NULLABLE_POINT_SOURCE+body).parse().opto().typeCheck(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { CodeGen code = new CodeGen( diff --git a/chapter22/src/test/java/com/seaofnodes/simple/Eval2.java b/chapter22/src/test/java/com/seaofnodes/simple/Eval2.java index 86f0e9db..8303b2c8 100644 --- a/chapter22/src/test/java/com/seaofnodes/simple/Eval2.java +++ b/chapter22/src/test/java/com/seaofnodes/simple/Eval2.java @@ -269,7 +269,7 @@ public abstract class Eval2 { case MulFNode mlf -> d(mlf.in(1)) * d(mlf.in(2)); case MulNode mul -> x(mul.in(1)) * x(mul.in(2)); case NewNode alloc-> alloc(alloc); - case NotNode not -> x(not.in(1)) == 0 ? 1L : 0L; + case NotNode not -> not(not); case OrNode or -> x(or .in(1)) | x(or .in(2)); case ProjNode proj -> proj._type instanceof TypeMem ? "$mem" : val(proj.in(0)); case ReadOnlyNode read -> val(read.in(1)); @@ -286,6 +286,12 @@ public abstract class Eval2 { }; } + private static Object not( NotNode not ) { + Object n = val(not.in(1)); + return ( n==null || (n instanceof Double D && D==0.0) || n instanceof Long L && L==0 ) + ? 1L : 0L; + } + // Fetch without unboxing, searching up-Frame static Object val( Node n ) { return F.get(n); } // Fetch and unbox as primitive long diff --git a/chapter23/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter23/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index 36746a81..d8c07a92 100644 --- a/chapter23/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter23/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -385,10 +385,38 @@ public class ScopeNode extends MemMergeNode { // E.g. "if( ptr ) ptr.field;" is legal because ptr is known not-null. public void addGuards( Node ctrl, Node pred, boolean invert ) { assert ctrl instanceof CFGNode; - _guards.add(ctrl); // Marker to distinguish 0,1,2 guards + _guards.add(ctrl); // Marker between guard sets // add pred & its cast to the normal input list, with special Vars if( pred==null || pred.isDead() ) return; // Dead, do not add any guards + _addGuards(ctrl,pred,invert); + } + + private void _addGuards( Node ctrl, Node pred, boolean invert ) { + if( pred==null || pred.isDead() ) + return; + if( !availableAt(pred,(CFGNode)ctrl,new BitSet()) ) + return; + // A single negation is handled below. For !!p (including p != null) + // or a negated short-circuit Phi, also discover the underlying facts. + if( pred instanceof NotNode not && + (not.in(1) instanceof NotNode || not.in(1) instanceof PhiNode) ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + _addGuards(ctrl,not.in(1),!invert); + pred.unkeep(); + } + // A false || or true && proves both operands. Only guard values + // available at this control, not ones confined to the RHS branch. + if( pred instanceof PhiNode phi && phi.nIns()==3 && phi.region() instanceof RegionNode r && r.nIns()==3 ) + for( int i=1; i<3; i++ ) + if( r.in(i) instanceof CProjNode prj && prj.ctrl() instanceof IfNode iff ) { + Node skip = shortCircuitSkippedPred(phi.in(i),iff.pred(),prj,invert); + if( skip != null ) { + _addGuards(ctrl,skip,invert); + _addGuards(ctrl,phi.in(3-i),invert); + break; + } + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : CodeGen.CODE.add(new NotNode(pred).peephole()); @@ -406,6 +434,35 @@ public class ScopeNode extends MemMergeNode { } } + // A Phi's value is available at its Region. Other data nodes require all + // their inputs, including pinned control, to dominate the proposed guard. + private static boolean availableAt(Node n, CFGNode ctrl, BitSet visit) { + if( n==null || n instanceof ConstantNode || visit.get(n._nid) ) return true; + visit.set(n._nid); + if( n instanceof PhiNode phi ) n = phi.region(); + if( n instanceof CFGNode cfg ) { + for( CFGNode dom=ctrl; dom!=null; dom=dom.idom() ) + if( dom==cfg ) return true; + return false; + } + for( Node def : n._inputs ) + if( !availableAt(def,ctrl,visit) ) return false; + return true; + } + + private static Node shortCircuitSkippedPred( Node val, Node pred, CProjNode prj, boolean invert ) { + if( val == null ) return null; + if( val == pred ) + return (prj._idx==0)==invert ? val : null; + if( val instanceof NotNode not && not.in(1)==pred ) + return (prj._idx==1)==invert ? val : null; + if( val._type == TypeInteger.TRUE && invert ) + return prj._idx==0 ? pred : CodeGen.CODE.add(new NotNode(pred).peephole()); + if( val._type == TypeInteger.FALSE && !invert ) + return prj._idx==1 ? pred : CodeGen.CODE.add(new NotNode(pred).peephole()); + return null; + } + private void _addGuard(Type guard, Node ctrl, Node pred) { Type tcast = guard.join(pred._type); if( tcast != pred._type && !tcast.isHigh() ) { @@ -420,7 +477,7 @@ public class ScopeNode extends MemMergeNode { // Remove matching pred/cast pairs from this guarded region. public ScopeNode removeGuards( Node ctrl ) { assert ctrl instanceof CFGNode; - // 0,1 or 2 guards + // Pop the guards up to this region's marker. while( true ) { Node g = _guards.pop(); if( g == ctrl ) break; diff --git a/chapter23/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter23/src/test/java/com/seaofnodes/simple/Chapter10Test.java index ec363239..c1b5e3a8 100644 --- a/chapter23/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter23/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -7,6 +7,61 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point?[] !points = new Point?[2]; + points[arg] = new Point { x = 42; }; + Point? p = points[1]; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;", + "if (!(p == null || arg == 0)) return p.x; return -1;" + } ) { + CodeGen code = new CodeGen(NULLABLE_POINT_SOURCE+body).parse().opto().typeCheck(); + assertEquals(body,"-1",Eval2.eval(code,0)); + assertEquals(body,"42",Eval2.eval(code,1)); + } + } + + @Test + public void testShortCircuitGuardScheduling() { + // The call result exists only on the RHS path, not above the merge. + CodeGen code = new CodeGen(""" + val f = { int n -> (n+1)&7; }; + int x = 0; + if (!!(arg && (x=f(arg)))) return x; + return -1; + """).driver(CodeGen.Phase.TypeCheck); + assertEquals("-1",Eval2.eval(code,0)); + assertEquals("2",Eval2.eval(code,1)); + assertEquals("-1",Eval2.eval(code,7)); + code.driver(CodeGen.Phase.LocalSched); + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!points) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new CodeGen(NULLABLE_POINT_SOURCE+body).parse().opto().typeCheck(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { CodeGen code = new CodeGen( diff --git a/chapter23/src/test/java/com/seaofnodes/simple/Eval2.java b/chapter23/src/test/java/com/seaofnodes/simple/Eval2.java index 094b5143..1254c0d3 100644 --- a/chapter23/src/test/java/com/seaofnodes/simple/Eval2.java +++ b/chapter23/src/test/java/com/seaofnodes/simple/Eval2.java @@ -271,7 +271,7 @@ public abstract class Eval2 { case MulFNode mlf -> d(mlf.in(1)) * d(mlf.in(2)); case MulNode mul -> x(mul.in(1)) * x(mul.in(2)); case NewNode alloc-> alloc(alloc); - case NotNode not -> x(not.in(1)) == 0 ? 1L : 0L; + case NotNode not -> not(not); case OrNode or -> x(or .in(1)) | x(or .in(2)); case ProjNode proj -> proj._type instanceof TypeMem ? "$mem" : val(proj.in(0)); case ReadOnlyNode read -> val(read.in(1)); @@ -288,6 +288,12 @@ public abstract class Eval2 { }; } + private static Object not( NotNode not ) { + Object n = val(not.in(1)); + return ( n==null || (n instanceof Double D && D==0.0) || n instanceof Long L && L==0 ) + ? 1L : 0L; + } + // Fetch without unboxing, searching up-Frame static Object val( Node n ) { return F.get(n); } // Fetch and unbox as primitive long diff --git a/chapter24/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter24/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index 36746a81..d8c07a92 100644 --- a/chapter24/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter24/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -385,10 +385,38 @@ public class ScopeNode extends MemMergeNode { // E.g. "if( ptr ) ptr.field;" is legal because ptr is known not-null. public void addGuards( Node ctrl, Node pred, boolean invert ) { assert ctrl instanceof CFGNode; - _guards.add(ctrl); // Marker to distinguish 0,1,2 guards + _guards.add(ctrl); // Marker between guard sets // add pred & its cast to the normal input list, with special Vars if( pred==null || pred.isDead() ) return; // Dead, do not add any guards + _addGuards(ctrl,pred,invert); + } + + private void _addGuards( Node ctrl, Node pred, boolean invert ) { + if( pred==null || pred.isDead() ) + return; + if( !availableAt(pred,(CFGNode)ctrl,new BitSet()) ) + return; + // A single negation is handled below. For !!p (including p != null) + // or a negated short-circuit Phi, also discover the underlying facts. + if( pred instanceof NotNode not && + (not.in(1) instanceof NotNode || not.in(1) instanceof PhiNode) ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + _addGuards(ctrl,not.in(1),!invert); + pred.unkeep(); + } + // A false || or true && proves both operands. Only guard values + // available at this control, not ones confined to the RHS branch. + if( pred instanceof PhiNode phi && phi.nIns()==3 && phi.region() instanceof RegionNode r && r.nIns()==3 ) + for( int i=1; i<3; i++ ) + if( r.in(i) instanceof CProjNode prj && prj.ctrl() instanceof IfNode iff ) { + Node skip = shortCircuitSkippedPred(phi.in(i),iff.pred(),prj,invert); + if( skip != null ) { + _addGuards(ctrl,skip,invert); + _addGuards(ctrl,phi.in(3-i),invert); + break; + } + } // Invert the If conditional if( invert ) pred = pred instanceof NotNode not ? not.in(1) : CodeGen.CODE.add(new NotNode(pred).peephole()); @@ -406,6 +434,35 @@ public class ScopeNode extends MemMergeNode { } } + // A Phi's value is available at its Region. Other data nodes require all + // their inputs, including pinned control, to dominate the proposed guard. + private static boolean availableAt(Node n, CFGNode ctrl, BitSet visit) { + if( n==null || n instanceof ConstantNode || visit.get(n._nid) ) return true; + visit.set(n._nid); + if( n instanceof PhiNode phi ) n = phi.region(); + if( n instanceof CFGNode cfg ) { + for( CFGNode dom=ctrl; dom!=null; dom=dom.idom() ) + if( dom==cfg ) return true; + return false; + } + for( Node def : n._inputs ) + if( !availableAt(def,ctrl,visit) ) return false; + return true; + } + + private static Node shortCircuitSkippedPred( Node val, Node pred, CProjNode prj, boolean invert ) { + if( val == null ) return null; + if( val == pred ) + return (prj._idx==0)==invert ? val : null; + if( val instanceof NotNode not && not.in(1)==pred ) + return (prj._idx==1)==invert ? val : null; + if( val._type == TypeInteger.TRUE && invert ) + return prj._idx==0 ? pred : CodeGen.CODE.add(new NotNode(pred).peephole()); + if( val._type == TypeInteger.FALSE && !invert ) + return prj._idx==1 ? pred : CodeGen.CODE.add(new NotNode(pred).peephole()); + return null; + } + private void _addGuard(Type guard, Node ctrl, Node pred) { Type tcast = guard.join(pred._type); if( tcast != pred._type && !tcast.isHigh() ) { @@ -420,7 +477,7 @@ public class ScopeNode extends MemMergeNode { // Remove matching pred/cast pairs from this guarded region. public ScopeNode removeGuards( Node ctrl ) { assert ctrl instanceof CFGNode; - // 0,1 or 2 guards + // Pop the guards up to this region's marker. while( true ) { Node g = _guards.pop(); if( g == ctrl ) break; diff --git a/chapter24/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter24/src/test/java/com/seaofnodes/simple/Chapter10Test.java index ec363239..c1b5e3a8 100644 --- a/chapter24/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter24/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -7,6 +7,61 @@ import static org.junit.Assert.fail; public class Chapter10Test { + // Issue #246: null-check guards start in Chapter 10; arrays arrive in Chapter 15. + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; }; + Point?[] !points = new Point?[2]; + points[arg] = new Point { x = 42; }; + Point? p = points[1]; + """; + + @Test + public void testNullGuards() { + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; return p.x;", + "int b = !!p; if (b) return p.x + b - 1; return -1;", + "if (!(p == null || arg == 0)) return p.x; return -1;" + } ) { + CodeGen code = new CodeGen(NULLABLE_POINT_SOURCE+body).parse().opto().typeCheck(); + assertEquals(body,"-1",Eval2.eval(code,0)); + assertEquals(body,"42",Eval2.eval(code,1)); + } + } + + @Test + public void testShortCircuitGuardScheduling() { + // The call result exists only on the RHS path, not above the merge. + CodeGen code = new CodeGen(""" + val f = { int n -> (n+1)&7; }; + int x = 0; + if (!!(arg && (x=f(arg)))) return x; + return -1; + """).driver(CodeGen.Phase.TypeCheck); + assertEquals("-1",Eval2.eval(code,0)); + assertEquals("2",Eval2.eval(code,1)); + assertEquals("-1",Eval2.eval(code,7)); + code.driver(CodeGen.Phase.LocalSched); + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!points) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new CodeGen(NULLABLE_POINT_SOURCE+body).parse().opto().typeCheck(); + fail(body); + } catch( RuntimeException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { CodeGen code = new CodeGen( diff --git a/chapter24/src/test/java/com/seaofnodes/simple/Eval2.java b/chapter24/src/test/java/com/seaofnodes/simple/Eval2.java index 5994e7e1..770f8351 100644 --- a/chapter24/src/test/java/com/seaofnodes/simple/Eval2.java +++ b/chapter24/src/test/java/com/seaofnodes/simple/Eval2.java @@ -271,7 +271,7 @@ public abstract class Eval2 { case MulFNode mlf -> d(mlf.in(1)) * d(mlf.in(2)); case MulNode mul -> x(mul.in(1)) * x(mul.in(2)); case NewNode alloc-> alloc(alloc); - case NotNode not -> x(not.in(1)) == 0 ? 1L : 0L; + case NotNode not -> not(not); case OrNode or -> x(or .in(1)) | x(or .in(2)); case ProjNode proj -> proj._type instanceof TypeMem ? "$mem" : val(proj.in(0)); case ReadOnlyNode read -> val(read.in(1)); @@ -288,6 +288,12 @@ public abstract class Eval2 { }; } + private static Object not( NotNode not ) { + Object n = val(not.in(1)); + return ( n==null || (n instanceof Double D && D==0.0) || n instanceof Long L && L==0 ) + ? 1L : 0L; + } + // Fetch without unboxing, searching up-Frame static Object val( Node n ) { return F.get(n); } // Fetch and unbox as primitive long diff --git a/chapter25/src/main/java/com/seaofnodes/simple/node/ScopeNode.java b/chapter25/src/main/java/com/seaofnodes/simple/node/ScopeNode.java index 7fb84394..527d3395 100644 --- a/chapter25/src/main/java/com/seaofnodes/simple/node/ScopeNode.java +++ b/chapter25/src/main/java/com/seaofnodes/simple/node/ScopeNode.java @@ -525,7 +525,7 @@ public class ScopeNode extends MemMergeNode { // values to Guard post-test. public void addGuards( CFGNode ctrl, Node pred, boolean invert ) { - _guards.add(ctrl); // Marker to distinguish 0,1,2 guards + _guards.add(ctrl); // Marker between guard sets // add pred & its cast to the normal input list, with special Vars if( pred==null || pred.isDead() ) return; // Dead, do not add any guards @@ -538,6 +538,14 @@ public class ScopeNode extends MemMergeNode { CFGNode early = CFGNode.earlyCFG(pred); if( early != null && !early.dominates(ctrl,null) ) return; + // A single negation is handled below. For !!p (including p != null) + // or a negated short-circuit Phi, also discover the underlying facts. + if( pred instanceof NotNode not && + (not.in(1) instanceof NotNode || not.in(1) instanceof PhiNode) ) { + pred.keep(); // Recursive peepholes may rediscover this Not. + _addGuards(ctrl,not.in(1),!invert); + pred.unkeep(); + } // Short-circuit logic is represented by a Phi. For `a || b` being // false, or `a && b` being true, both individual operands have the // same proven value as the whole expression. The "skipped RHS" path @@ -594,7 +602,7 @@ public class ScopeNode extends MemMergeNode { // Remove matching pred/cast pairs from this guarded region. public void removeGuards( Node ctrl ) { assert ctrl instanceof CFGNode; - // 0,1 or 2 guards + // Pop the guards up to this region's marker. while( true ) { Node g = _guards.pop(); if( g == ctrl ) break; diff --git a/chapter25/src/test/java/com/seaofnodes/simple/Chapter10Test.java b/chapter25/src/test/java/com/seaofnodes/simple/Chapter10Test.java index 0d40d871..a96da02c 100644 --- a/chapter25/src/test/java/com/seaofnodes/simple/Chapter10Test.java +++ b/chapter25/src/test/java/com/seaofnodes/simple/Chapter10Test.java @@ -7,6 +7,64 @@ import static org.junit.Assert.fail; public class Chapter10Test { + private static final String NULLABLE_POINT_SOURCE = """ + struct Point { int x; new Point = { int v -> x = v; }; }; + Point?[] !points = new Point?[2]; + points[arg] = new Point(42); + Point? p = points[1]; + """; + + @Test + public void testNullGuards() { + // Issue #246: != null lowers to !!p. Follow every negation, on either arm. + for( String body : new String[] { + "if (p != null) return p.x; return -1;", + "if (null != p) return p.x; return -1;", + "if (!!!!p) return p.x; return -1;", + "if (!!!p) return -1; return p.x;", + "val b = !!p; if (b) return p.x + b - 1; return -1;", + "if (!(p == null || arg == 0)) return p.x; return -1;" + } ) + for( long seed : new long[] { 1, 42, 126 } ) { + CodeGen code = new CodeGen(NULLABLE_POINT_SOURCE+body,seed,true) + .driver(CodeGen.Phase.TypeCheck); + assertEquals(body,"-1",Eval2.eval(code,0)); + assertEquals(body,"42",Eval2.eval(code,1)); + code.driver(CodeGen.Phase.LocalSched); + } + } + + @Test + public void testShortCircuitGuardScheduling() { + // The call result exists only on the RHS path, not above the merge. + CodeGen code = new CodeGen(""" + val f = { int n -> (n+1)&7; }; + int x = 0; + if (!!(arg && (x=f(arg)))) return x; + return -1; + """).driver(CodeGen.Phase.TypeCheck); + assertEquals("-1",Eval2.eval(code,0)); + assertEquals("2",Eval2.eval(code,1)); + assertEquals("-1",Eval2.eval(code,7)); + code.driver(CodeGen.Phase.LocalSched); + } + + @Test + public void testNullGuardErrors() { + for( String body : new String[] { + "return p.x;", + "if (!!points) return p.x; return -1;", + "if (!!p) { int x = p.x; } return p.x;" + } ) { + try { + new CodeGen(NULLABLE_POINT_SOURCE+body).driver(CodeGen.Phase.TypeCheck); + fail(body); + } catch( Parser.ParseException e ) { + assertEquals(body,"Might be null accessing 'x'",e.getMessage()); + } + } + } + @Test public void testFuzzer() { CodeGen code = new CodeGen( diff --git a/docs/chapter-backports.md b/docs/chapter-backports.md index 190e5351..1828b16b 100644 --- a/docs/chapter-backports.md +++ b/docs/chapter-backports.md @@ -23,6 +23,7 @@ the correction into the chapter's representation, not the entire modern file. | B10 | Preserve widening state in integer nonzero refinement | **Audit** first chapter with both widening and `nonZero`; no later than 24 | Chapter 25 `TypeInteger.nonZero` preserves `_widen`. Check lattice laws and loop refinement. Exclude TypeScalar, storage-type, and serialization changes. | | B11 | Diagnose return types using the optimized return expression | 18; 19 already has the correction | `ReturnNode.err()` uses `expr()._type` instead of the parse-time `mt` aggregate. Chapter 18 rejects `struct S { u8 x; }; return new S; return 0;` with a mixed integer/reference error; Chapter 19 accepts it. Also test genuinely reachable incompatible returns. This is a candidate, not yet an applied or isolated-patch-verified fix. | | B12 | Encode the actual destination register for two-address immediate multiply | 21 | `MulIX86` inherits `ImmX86.encoding`, whose ModRM.reg is fixed to zero and whose REX.R is clear. With source/destination both `rcx`, multiplying by 11 emits `48 6b c1 0b` (destination `rax`) instead of `48 6b c9 0b`. With both `r9`, it emits `49 6b c1 0b` instead of `4d 6b c9 0b`. Give multiply its proper register fields while preserving Chapter 21's two-address allocation contract; do not change the opcode-extension fields used by other `ImmX86` subclasses. Later chapters use a separate multiply encoder. | +| B14 | Check leaf type kinds before cyclic equality | 23-24; 25 already checks | `Type.cycle_eq` calls leaf `eq` without checking `_type`. Intern a struct with a `Type.BOTTOM` field, then same-named structs with float constants 1.0 and 2.0; meeting the latter throws ClassCastException in `TypeFloat.eq`. Reproduced independently in 23 and 24; the adapted probe passes in 25. Add a deterministic TypeTest before backporting the type-kind check. This is separate from B13; no type implementation changed. | After the first two reviews, batch only corrections with established independence and regressions. Keep one logical correction per commit across affected chapters. @@ -76,6 +77,192 @@ starts, so run `make lib` separately before `make tests`. ## Validation record +### Issue #246: Chapter 25 reproduction and local correction + +On 2026-09-19, rebuilt Chapter 25 with +`make build/classes/main/.mtag build/classes/test/.ttag` and ran the following +through `new CodeGen(src).driver(CodeGen.Phase.TypeCheck)` with Java assertions +enabled. Before the correction it failed in `MemOpNode.err()` with +`Might be null accessing 'x'`. +The original issue's `new Point { x = 42; }` syntax is no longer accepted; +only object construction was adapted to Chapter 25's constructor syntax. + +```text +struct Point { int x; new Point = { int v -> x = v; }; }; +Point?[] !points = new Point?[2]; +points[arg] = new Point(42); +Point? p = points[1]; +if (p != null) + return p.x; +return -1; +``` + +That failure prevented evaluation of either expected runtime result. + +Debugging isolated the first bad phase to parsing. `Parser.parseEquality` +builds `!(p == null)`, and `BoolNode.EQ.idealize` rewrites the inner comparison +to `!p`. `ScopeNode._addGuards` records nonzero `!!p` and zero `!p` on the +true arm, but does not descend again to record non-null `p`. Meanwhile, +`CProjNode.idealize` strips the negations from the control test. The resulting +graph therefore tests `p` directly without refining the pointer used by `p.x`. + +With default seed 126, the original's `LoadNode#150` uses nullable +`ReadOnlyNode#126` directly after Parse, Iter, and Opto. Changing only the +condition to `p` produces `GuardNode#132` on that same pointer, consumed by +`LoadNode#135`, and returns `-1` and `42` as expected. Explicit `!!p` fails; +`if (p == null) return -1; return p.x;` passes with both expected results. +The original failure and successful `if (p)` control were also checked with +seeds 1, 42, and 123, with assertions enabled. + +The local correction recurses into a Not operand when it is another Not or a +short-circuit Phi, flipping the proven truth. Existing single-negation handling +remains in place; its guards need not be duplicated. Each recursive call still +checks dominance/schedulability, and the predicate is kept alive while recursive +peepholes run. Boolean expression values retain their original meaning. + +`Chapter10Test.testNullGuards` covers both comparison orders, +nested negations, early returns, a reused Boolean value, and negated +short-circuit logic. Each case checks both runtime outcomes and scheduling with +seeds 1, 42, and 126. `testNullGuardErrors` verifies rejection of +unguarded access, a check on a different pointer, and a guard used beyond its +branch. The positive regression failed before the compiler change. + +Final `make tests` passed all 427 tests with assertions enabled: 362 raw0, +32 raw1, 7 standalone, 18 system, 1 fixed-seed fuzzer wrapper, and 7 remaining +tests. The build regenerated `sys.o`. Log: `chapter25/build/issue246-tests.log`. +A broader intermediate version that duplicated single-negation handling exposed +a Dijkstra SCCP assertion; it was not retained. The original compiler passed +Dijkstra in a comparison run with freshly rebuilt `sys.o`, and the final +correction passes it too. No assertions or existing test expectations changed. + +Separate pre-existing diagnostic concern: using the same array setup with +`if (p == null) return p.x; return -1;` was accepted before the correction, +despite dereferencing null on the taken arm. This remains outside B13's fix. + +### B13: earliest chapter and regression placement + +Null-check refinement begins in Chapter 10, alongside nullable struct pointers; +it is documented in `chapter10/README.md` and implemented by +`ScopeNode.upcast`. Chapter 9 has no corresponding pointer guards. The reduced +failure already reproduces in Chapter 10: + +```text +struct Point { int x; }; +Point point = new Point; +point.x = 42; +Point? p = null; +if (arg) p = point; +if (p != null) return p.x; +return -1; +``` + +The same program with `if (p)` passes. The array version reproduces in Chapter +15, where arrays first appear; its `if (p)` control also passes. + +Substantial guard implementation differences before the backport: + +| Chapters | Representation and behavior | +|---|---| +| 10-16 | `ScopeNode.upcast` refines only values present directly in local scope inputs. A non-null pointer uses `CastNode(TypeMemPtr.VOIDPTR, ...)`; a zero/null fact replaces a local with a constant. There is no expression-guard table and no general nonzero integer refinement. | +| 13 onward | The parser also refines the false arm without an explicit `else`, allowing a fact to survive an early return. Chapters 10-12 require an explicit `else` for that test. | +| 17 | `addGuards`/`removeGuards` introduce a scoped predicate/cast table and `upcastGuard` applies facts to later matching expressions, including loads. Refinements use `nonZero`/`makeZero` and type-bearing CastNodes. | +| 18-24 | The same scoped Cast architecture; `_addGuard` factors the two cases and rejects high joins. Chapter 23 adds short-circuit syntax, but these scopes do not recursively decompose its Phis as Chapter 25 does. | +| 25 | GuardNode stores zero/nonzero intent and derives the value family from its input, supporting incomplete types. Scope guard discovery checks dominance, decomposes short-circuit Phis, and now follows nested negations. | + +The tests have been moved from Chapter 25's `Chapter25Test.java` to +`Chapter10Test.java` and forward-ported into that same test class in every +snapshot from 10 through 25. Chapters 10-14 use the reduced pointer setup; +15-24 use arrays with the allocation syntax available there; 25 retains the +constructor-based array setup, seed rotation, and scheduling checks. Boolean +temporaries use `int` in earlier snapshots, early-return coverage uses an +explicit `else` before Chapter 13, and short-circuit coverage starts at 23. +The tests are active and assert successful guarded access, not the old error. + +The compiler backport now retains each chapter's existing guard representation +and adds negation traversal there, as detailed below. Importing Chapter 25's +GuardNode/incomplete-type architecture remains a separate design change. + +Before adding the tests, `make -k tests` passed in all snapshots 10-24. +Log: `chapter25/build/issue246-review/baseline.log`. + +After moving/porting the tests, ran `make -k tests` across 10-25: + +- Chapters 10-23 each fail only `Chapter10Test.testNullGuards`, with + `Might be null accessing 'x'`. The new rejection test and all existing tests + in those suites pass. These were the exposed regressions before the compiler + backport recorded below. +- Chapter 24's isolated `Chapter10Test` runs 26 tests with only the same B13 + failure. Its full suite also exposes a type-equality failure, starting at + `Chapter16Test.testSquare`: `TypeFloat.eq` receives a plain `Type` through + `Type.cycle_eq`, causing a ClassCastException and later cascading failures + (197 failures total, including B13). A forced `make -B tests CHAPTERS=chapter24` + reproduces this, so a rebuild does not resolve it. No type implementation + was changed. This additional failure must be reviewed separately. +- Chapter 25 passes all 427 tests after relocation: 364 raw0, 32 raw1, + 7 standalone, 16 system, 1 fuzzer wrapper, and 7 remaining tests. + +Logs: `chapter25/build/issue246-review/ported-tests.log`, +`chapter25/build/issue246-review/chapter24-focused.log`, and +`chapter25/build/issue246-review/chapter24-rebuilt.log`. + +### B13: completed compiler backport + +Applied locally on 2026-09-19 to every snapshot from Chapter 10 through 25; +nothing pushed. The same paired-negation traversal starts in Chapter 10 and +continues forward, with representation-specific adaptations: + +- 10-16 recurse within `upcast`, retaining local-binding casts and constants. + No dominance machinery, expression-guard table, or modern type architecture + is introduced. +- 17-22 separate the public guard-set marker from recursive `_addGuards`. + Nested negations add facts within the existing scoped CastNode table; + recursive calls do not add extra scope markers. +- 23-24 additionally use Chapter 25's short-circuit Phi decomposition, since + short-circuit syntax starts in 23. A small `availableAt` helper walks data + inputs and the existing CFG immediate-dominator chain to reject RHS-only + values unavailable at the proposed guard. Phi availability is checked at + its Region. This avoids importing 25's folding-aware `earlyCFG` machinery. +- 25 retains its existing GuardNode, dominance, and short-circuit mechanisms; + only the nested-negation discovery is added. + +All versions retain the original single-negation handling and keep the +predicate alive during recursive peepholes. The added guards do not change +the Boolean expression's value. + +The reused-Boolean regression also exposed `Eval2` casting a pointer to Long +when evaluating Not in 18-24. Those test evaluators now share Chapter 25's +null/integer/float-aware Not helper. Compiler type implementations are unchanged. + +`Chapter10Test` remains the home of the forward-ported regression in every +snapshot 10-25. In 23-25, `testShortCircuitGuardScheduling` additionally checks +an RHS-only call result through local scheduling and evaluates both branch +outcomes and a zero result. Chapter 25's pointer cases retain seed rotation. + +Each full `make tests` target passes after the correction: + +| Chapters | Ordinary tests per chapter | Additional suites | +|---|---|---| +| 10-17 | 149, 164, 166, 181, 201, 212, 230, 282 | Each complete chapter target passed. | +| 18-22 | 309, 350, 359, 374, 386 | Each also passed its fixed-seed fuzzer wrapper. | +| 23-24 | 403, 425 | Each also passed its fixed-seed fuzzer wrapper. | +| 25 | 428 total | 365 raw0, 32 raw1, 7 standalone, 16 system, 1 fuzzer wrapper, 7 remaining. | + +Commands were `make tests CHAPTERS=chapter10`, then `make -k tests` with +`CHAPTERS='chapter11 ... chapter22'`, `CHAPTERS='chapter18 ... chapter24'`, +and finally `CHAPTERS='chapter23 chapter24 chapter25'` (the ellipses here +abbreviate explicitly enumerated chapter names). Logs under +`chapter25/build/issue246-review/`: `chapter10-fixed.log`, +`chapters11-22-fixed.log` (11-17 passed; 18-22 exposed the evaluator issue), +`chapters18-24-fixed.log`, and `chapters23-25-final.log`. + +The separate cyclic-equality failure is not fixed by B13's green suites. +It has a deterministic standalone reproducer in 23 and 24 and is queued as +B14. The probe interns `Probe { x: BOTTOM }`, `Probe { x: 1.0 }`, and +`Probe { x: 2.0 }`, then meets the latter two. It throws in `TypeFloat.eq` +through `Type.cycle_eq`; Chapter 25's equivalent probe succeeds. This records +the previously order-sensitive full-suite failure without expanding the guard +fix into a lattice change. + ### Initial setup baseline On 2026-09-17, using Windows/Cygwin and OpenJDK 22.0.2: |