Handle double-negation preserving not-null knowledge

2 files changed, 68 insertions, 2 deletions

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(