From 8234877eebd32d81960a7f434fdf29150a7ed505 Mon Sep 17 00:00:00 2001
From: jespergravgaard <jesper@balmangravgaard.dk>
Date: Mon, 15 Apr 2019 10:44:18 +0200
Subject: [PATCH] Fixed error where classic for() failed when init was empty.
 Closes #163

---
 .../Pass0GenerateStatementSequence.java       |  15 +-
 .../dk/camelot64/kickc/test/TestPrograms.java |  10 +
 src/test/kc/for-empty-init.kc                 |   9 +
 src/test/kc/for-ranged-novar.kc               |   9 +
 src/test/ref/for-empty-init.asm               |  15 +
 src/test/ref/for-empty-init.cfg               |  21 ++
 src/test/ref/for-empty-init.log               | 306 ++++++++++++++++++
 src/test/ref/for-empty-init.sym               |  13 +
 8 files changed, 393 insertions(+), 5 deletions(-)
 create mode 100644 src/test/kc/for-empty-init.kc
 create mode 100644 src/test/kc/for-ranged-novar.kc
 create mode 100644 src/test/ref/for-empty-init.asm
 create mode 100644 src/test/ref/for-empty-init.cfg
 create mode 100644 src/test/ref/for-empty-init.log
 create mode 100644 src/test/ref/for-empty-init.sym

diff --git a/src/main/java/dk/camelot64/kickc/passes/Pass0GenerateStatementSequence.java b/src/main/java/dk/camelot64/kickc/passes/Pass0GenerateStatementSequence.java
index 4f57ea77f..2c6523ab7 100644
--- a/src/main/java/dk/camelot64/kickc/passes/Pass0GenerateStatementSequence.java
+++ b/src/main/java/dk/camelot64/kickc/passes/Pass0GenerateStatementSequence.java
@@ -768,11 +768,13 @@ public class Pass0GenerateStatementSequence extends KickCBaseVisitor<Object> {
       loopStack.push(new Loop(blockScope));
       KickCParser.StmtForContext stmtForCtx = (KickCParser.StmtForContext) ctx.getParent();
       KickCParser.ForDeclContext forDeclCtx = (KickCParser.ForDeclContext) stmtForCtx.forDeclaration();
-      // Create and assign declared loop variable
-      Variable lValue = getForVariable(forDeclCtx);
-      KickCParser.ExprContext initializer = forDeclCtx.expr();
-      if(initializer != null) {
-         addInitialAssignment(initializer, lValue, Comment.NO_COMMENTS);
+      if(forDeclCtx!=null) {
+         // Create and assign declared loop variable
+         Variable lValue = getForVariable(forDeclCtx);
+         KickCParser.ExprContext initializer = forDeclCtx.expr();
+         if(initializer != null) {
+            addInitialAssignment(initializer, lValue, Comment.NO_COMMENTS);
+         }
       }
       // Add label
       Label repeatLabel = getCurrentScope().addLabelIntermediate();
@@ -808,6 +810,9 @@ public class Pass0GenerateStatementSequence extends KickCBaseVisitor<Object> {
       loopStack.push(new Loop(blockScope));
       KickCParser.StmtForContext stmtForCtx = (KickCParser.StmtForContext) ctx.getParent();
       KickCParser.ForDeclContext forDeclCtx = (KickCParser.ForDeclContext) stmtForCtx.forDeclaration();
+      if(forDeclCtx==null) {
+         throw new CompileError("Ranged for() must have iteration variable.", new StatementSource(ctx));
+      }
       // Create declared loop variable
       Variable lValue = getForVariable(forDeclCtx);
       KickCParser.ExprContext rangeFirstCtx = ctx.expr(0);
diff --git a/src/test/java/dk/camelot64/kickc/test/TestPrograms.java b/src/test/java/dk/camelot64/kickc/test/TestPrograms.java
index 1cf51db80..3b6f07013 100644
--- a/src/test/java/dk/camelot64/kickc/test/TestPrograms.java
+++ b/src/test/java/dk/camelot64/kickc/test/TestPrograms.java
@@ -37,6 +37,16 @@ public class TestPrograms {
    //   compileAndCompare("pointer-cast-3");
    //}
 
+   @Test
+   public void testForRangedNoVar() throws IOException, URISyntaxException {
+      assertError("for-ranged-novar", "Ranged for() must have iteration variable");
+   }
+
+   @Test
+   public void testForEmptyInit() throws IOException, URISyntaxException {
+      compileAndCompare("for-empty-init");
+   }
+
    @Test
    public void testForEmptyIncrement() throws IOException, URISyntaxException {
       compileAndCompare("for-empty-increment");
diff --git a/src/test/kc/for-empty-init.kc b/src/test/kc/for-empty-init.kc
new file mode 100644
index 000000000..d76167b2f
--- /dev/null
+++ b/src/test/kc/for-empty-init.kc
@@ -0,0 +1,9 @@
+// Tests that for()-loops can have empty inits
+
+void main() {
+    const byte* SCREEN = $400;
+    unsigned char i=0;
+    for(;i<10;i++) {
+        SCREEN[i] = i;
+    }
+}
\ No newline at end of file
diff --git a/src/test/kc/for-ranged-novar.kc b/src/test/kc/for-ranged-novar.kc
new file mode 100644
index 000000000..9fc490dd8
--- /dev/null
+++ b/src/test/kc/for-ranged-novar.kc
@@ -0,0 +1,9 @@
+// Tests that ranged for()-loops must have an iterator variable
+
+void main() {
+    const byte* SCREEN = $400;
+
+    for( : 0..10) {
+        SCREEN[i] = i;
+    }
+}
\ No newline at end of file
diff --git a/src/test/ref/for-empty-init.asm b/src/test/ref/for-empty-init.asm
new file mode 100644
index 000000000..cd3e1d3ed
--- /dev/null
+++ b/src/test/ref/for-empty-init.asm
@@ -0,0 +1,15 @@
+// Tests that for()-loops can have empty inits
+.pc = $801 "Basic"
+:BasicUpstart(main)
+.pc = $80d "Program"
+main: {
+    .label SCREEN = $400
+    ldx #0
+  b1:
+    txa
+    sta SCREEN,x
+    inx
+    cpx #$a
+    bcc b1
+    rts
+}
diff --git a/src/test/ref/for-empty-init.cfg b/src/test/ref/for-empty-init.cfg
new file mode 100644
index 000000000..0dffafc27
--- /dev/null
+++ b/src/test/ref/for-empty-init.cfg
@@ -0,0 +1,21 @@
+@begin: scope:[]  from
+  [0] phi()
+  to:@1
+@1: scope:[]  from @begin
+  [1] phi()
+  [2] call main 
+  to:@end
+@end: scope:[]  from @1
+  [3] phi()
+main: scope:[main]  from @1
+  [4] phi()
+  to:main::@1
+main::@1: scope:[main]  from main main::@1
+  [5] (byte) main::i#2 ← phi( main/(byte/signed byte/word/signed word/dword/signed dword) 0 main::@1/(byte) main::i#1 )
+  [6] *((const byte*) main::SCREEN#0 + (byte) main::i#2) ← (byte) main::i#2
+  [7] (byte) main::i#1 ← ++ (byte) main::i#2
+  [8] if((byte) main::i#1<(byte/signed byte/word/signed word/dword/signed dword) $a) goto main::@1
+  to:main::@return
+main::@return: scope:[main]  from main::@1
+  [9] return 
+  to:@return
diff --git a/src/test/ref/for-empty-init.log b/src/test/ref/for-empty-init.log
new file mode 100644
index 000000000..8a184d34b
--- /dev/null
+++ b/src/test/ref/for-empty-init.log
@@ -0,0 +1,306 @@
+
+CONTROL FLOW GRAPH SSA
+@begin: scope:[]  from
+  to:@1
+main: scope:[main]  from @1
+  (byte*) main::SCREEN#0 ← ((byte*)) (word/signed word/dword/signed dword) $400
+  (byte) main::i#0 ← (byte/signed byte/word/signed word/dword/signed dword) 0
+  to:main::@1
+main::@1: scope:[main]  from main main::@1
+  (byte) main::i#2 ← phi( main/(byte) main::i#0 main::@1/(byte) main::i#1 )
+  *((byte*) main::SCREEN#0 + (byte) main::i#2) ← (byte) main::i#2
+  (byte) main::i#1 ← ++ (byte) main::i#2
+  (bool~) main::$0 ← (byte) main::i#1 < (byte/signed byte/word/signed word/dword/signed dword) $a
+  if((bool~) main::$0) goto main::@1
+  to:main::@return
+main::@return: scope:[main]  from main::@1
+  return 
+  to:@return
+@1: scope:[]  from @begin
+  call main 
+  to:@2
+@2: scope:[]  from @1
+  to:@end
+@end: scope:[]  from @2
+
+SYMBOL TABLE SSA
+(label) @1
+(label) @2
+(label) @begin
+(label) @end
+(void()) main()
+(bool~) main::$0
+(label) main::@1
+(label) main::@return
+(byte*) main::SCREEN
+(byte*) main::SCREEN#0
+(byte) main::i
+(byte) main::i#0
+(byte) main::i#1
+(byte) main::i#2
+
+Culled Empty Block (label) @2
+Successful SSA optimization Pass2CullEmptyBlocks
+Simple Condition (bool~) main::$0 [6] if((byte) main::i#1<(byte/signed byte/word/signed word/dword/signed dword) $a) goto main::@1
+Successful SSA optimization Pass2ConditionalJumpSimplification
+Constant (const byte*) main::SCREEN#0 = ((byte*))$400
+Constant (const byte) main::i#0 = 0
+Successful SSA optimization Pass2ConstantIdentification
+Inlining constant with var siblings (const byte) main::i#0
+Constant inlined main::i#0 = (byte/signed byte/word/signed word/dword/signed dword) 0
+Successful SSA optimization Pass2ConstantInlining
+Added new block during phi lifting main::@3(between main::@1 and main::@1)
+Adding NOP phi() at start of @begin
+Adding NOP phi() at start of @1
+Adding NOP phi() at start of @end
+Adding NOP phi() at start of main
+CALL GRAPH
+Calls in [] to main:2 
+
+Created 1 initial phi equivalence classes
+Coalesced [10] main::i#3 ← main::i#1
+Coalesced down to 1 phi equivalence classes
+Culled Empty Block (label) main::@3
+Adding NOP phi() at start of @begin
+Adding NOP phi() at start of @1
+Adding NOP phi() at start of @end
+Adding NOP phi() at start of main
+
+FINAL CONTROL FLOW GRAPH
+@begin: scope:[]  from
+  [0] phi()
+  to:@1
+@1: scope:[]  from @begin
+  [1] phi()
+  [2] call main 
+  to:@end
+@end: scope:[]  from @1
+  [3] phi()
+main: scope:[main]  from @1
+  [4] phi()
+  to:main::@1
+main::@1: scope:[main]  from main main::@1
+  [5] (byte) main::i#2 ← phi( main/(byte/signed byte/word/signed word/dword/signed dword) 0 main::@1/(byte) main::i#1 )
+  [6] *((const byte*) main::SCREEN#0 + (byte) main::i#2) ← (byte) main::i#2
+  [7] (byte) main::i#1 ← ++ (byte) main::i#2
+  [8] if((byte) main::i#1<(byte/signed byte/word/signed word/dword/signed dword) $a) goto main::@1
+  to:main::@return
+main::@return: scope:[main]  from main::@1
+  [9] return 
+  to:@return
+
+
+VARIABLE REGISTER WEIGHTS
+(void()) main()
+(byte*) main::SCREEN
+(byte) main::i
+(byte) main::i#1 16.5
+(byte) main::i#2 22.0
+
+Initial phi equivalence classes
+[ main::i#2 main::i#1 ]
+Complete equivalence classes
+[ main::i#2 main::i#1 ]
+Allocated zp ZP_BYTE:2 [ main::i#2 main::i#1 ]
+
+INITIAL ASM
+//SEG0 File Comments
+// Tests that for()-loops can have empty inits
+//SEG1 Basic Upstart
+.pc = $801 "Basic"
+:BasicUpstart(bbegin)
+.pc = $80d "Program"
+//SEG2 Global Constants & labels
+//SEG3 @begin
+bbegin:
+//SEG4 [1] phi from @begin to @1 [phi:@begin->@1]
+b1_from_bbegin:
+  jmp b1
+//SEG5 @1
+b1:
+//SEG6 [2] call main 
+//SEG7 [4] phi from @1 to main [phi:@1->main]
+main_from_b1:
+  jsr main
+//SEG8 [3] phi from @1 to @end [phi:@1->@end]
+bend_from_b1:
+  jmp bend
+//SEG9 @end
+bend:
+//SEG10 main
+main: {
+    .label SCREEN = $400
+    .label i = 2
+  //SEG11 [5] phi from main to main::@1 [phi:main->main::@1]
+  b1_from_main:
+  //SEG12 [5] phi (byte) main::i#2 = (byte/signed byte/word/signed word/dword/signed dword) 0 [phi:main->main::@1#0] -- vbuz1=vbuc1 
+    lda #0
+    sta i
+    jmp b1
+  //SEG13 [5] phi from main::@1 to main::@1 [phi:main::@1->main::@1]
+  b1_from_b1:
+  //SEG14 [5] phi (byte) main::i#2 = (byte) main::i#1 [phi:main::@1->main::@1#0] -- register_copy 
+    jmp b1
+  //SEG15 main::@1
+  b1:
+  //SEG16 [6] *((const byte*) main::SCREEN#0 + (byte) main::i#2) ← (byte) main::i#2 -- pbuc1_derefidx_vbuz1=vbuz1 
+    ldy i
+    tya
+    sta SCREEN,y
+  //SEG17 [7] (byte) main::i#1 ← ++ (byte) main::i#2 -- vbuz1=_inc_vbuz1 
+    inc i
+  //SEG18 [8] if((byte) main::i#1<(byte/signed byte/word/signed word/dword/signed dword) $a) goto main::@1 -- vbuz1_lt_vbuc1_then_la1 
+    lda i
+    cmp #$a
+    bcc b1_from_b1
+    jmp breturn
+  //SEG19 main::@return
+  breturn:
+  //SEG20 [9] return 
+    rts
+}
+
+REGISTER UPLIFT POTENTIAL REGISTERS
+Potential registers zp ZP_BYTE:2 [ main::i#2 main::i#1 ] : zp ZP_BYTE:2 , reg byte a , reg byte x , reg byte y , 
+
+REGISTER UPLIFT SCOPES
+Uplift Scope [main] 38.5: zp ZP_BYTE:2 [ main::i#2 main::i#1 ] 
+Uplift Scope [] 
+
+Uplifting [main] best 263 combination reg byte x [ main::i#2 main::i#1 ] 
+Uplifting [] best 263 combination 
+
+ASSEMBLER BEFORE OPTIMIZATION
+//SEG0 File Comments
+// Tests that for()-loops can have empty inits
+//SEG1 Basic Upstart
+.pc = $801 "Basic"
+:BasicUpstart(bbegin)
+.pc = $80d "Program"
+//SEG2 Global Constants & labels
+//SEG3 @begin
+bbegin:
+//SEG4 [1] phi from @begin to @1 [phi:@begin->@1]
+b1_from_bbegin:
+  jmp b1
+//SEG5 @1
+b1:
+//SEG6 [2] call main 
+//SEG7 [4] phi from @1 to main [phi:@1->main]
+main_from_b1:
+  jsr main
+//SEG8 [3] phi from @1 to @end [phi:@1->@end]
+bend_from_b1:
+  jmp bend
+//SEG9 @end
+bend:
+//SEG10 main
+main: {
+    .label SCREEN = $400
+  //SEG11 [5] phi from main to main::@1 [phi:main->main::@1]
+  b1_from_main:
+  //SEG12 [5] phi (byte) main::i#2 = (byte/signed byte/word/signed word/dword/signed dword) 0 [phi:main->main::@1#0] -- vbuxx=vbuc1 
+    ldx #0
+    jmp b1
+  //SEG13 [5] phi from main::@1 to main::@1 [phi:main::@1->main::@1]
+  b1_from_b1:
+  //SEG14 [5] phi (byte) main::i#2 = (byte) main::i#1 [phi:main::@1->main::@1#0] -- register_copy 
+    jmp b1
+  //SEG15 main::@1
+  b1:
+  //SEG16 [6] *((const byte*) main::SCREEN#0 + (byte) main::i#2) ← (byte) main::i#2 -- pbuc1_derefidx_vbuxx=vbuxx 
+    txa
+    sta SCREEN,x
+  //SEG17 [7] (byte) main::i#1 ← ++ (byte) main::i#2 -- vbuxx=_inc_vbuxx 
+    inx
+  //SEG18 [8] if((byte) main::i#1<(byte/signed byte/word/signed word/dword/signed dword) $a) goto main::@1 -- vbuxx_lt_vbuc1_then_la1 
+    cpx #$a
+    bcc b1_from_b1
+    jmp breturn
+  //SEG19 main::@return
+  breturn:
+  //SEG20 [9] return 
+    rts
+}
+
+ASSEMBLER OPTIMIZATIONS
+Removing instruction jmp b1
+Removing instruction jmp bend
+Removing instruction jmp b1
+Removing instruction jmp breturn
+Succesful ASM optimization Pass5NextJumpElimination
+Replacing label b1_from_b1 with b1
+Removing instruction b1_from_bbegin:
+Removing instruction b1:
+Removing instruction main_from_b1:
+Removing instruction bend_from_b1:
+Removing instruction b1_from_b1:
+Succesful ASM optimization Pass5RedundantLabelElimination
+Removing instruction bend:
+Removing instruction b1_from_main:
+Removing instruction breturn:
+Succesful ASM optimization Pass5UnusedLabelElimination
+Updating BasicUpstart to call main directly
+Removing instruction jsr main
+Succesful ASM optimization Pass5SkipBegin
+Removing instruction jmp b1
+Succesful ASM optimization Pass5NextJumpElimination
+Removing instruction bbegin:
+Succesful ASM optimization Pass5UnusedLabelElimination
+
+FINAL SYMBOL TABLE
+(label) @1
+(label) @begin
+(label) @end
+(void()) main()
+(label) main::@1
+(label) main::@return
+(byte*) main::SCREEN
+(const byte*) main::SCREEN#0 SCREEN = ((byte*))(word/signed word/dword/signed dword) $400
+(byte) main::i
+(byte) main::i#1 reg byte x 16.5
+(byte) main::i#2 reg byte x 22.0
+
+reg byte x [ main::i#2 main::i#1 ]
+
+
+FINAL ASSEMBLER
+Score: 161
+
+//SEG0 File Comments
+// Tests that for()-loops can have empty inits
+//SEG1 Basic Upstart
+.pc = $801 "Basic"
+:BasicUpstart(main)
+.pc = $80d "Program"
+//SEG2 Global Constants & labels
+//SEG3 @begin
+//SEG4 [1] phi from @begin to @1 [phi:@begin->@1]
+//SEG5 @1
+//SEG6 [2] call main 
+//SEG7 [4] phi from @1 to main [phi:@1->main]
+//SEG8 [3] phi from @1 to @end [phi:@1->@end]
+//SEG9 @end
+//SEG10 main
+main: {
+    .label SCREEN = $400
+  //SEG11 [5] phi from main to main::@1 [phi:main->main::@1]
+  //SEG12 [5] phi (byte) main::i#2 = (byte/signed byte/word/signed word/dword/signed dword) 0 [phi:main->main::@1#0] -- vbuxx=vbuc1 
+    ldx #0
+  //SEG13 [5] phi from main::@1 to main::@1 [phi:main::@1->main::@1]
+  //SEG14 [5] phi (byte) main::i#2 = (byte) main::i#1 [phi:main::@1->main::@1#0] -- register_copy 
+  //SEG15 main::@1
+  b1:
+  //SEG16 [6] *((const byte*) main::SCREEN#0 + (byte) main::i#2) ← (byte) main::i#2 -- pbuc1_derefidx_vbuxx=vbuxx 
+    txa
+    sta SCREEN,x
+  //SEG17 [7] (byte) main::i#1 ← ++ (byte) main::i#2 -- vbuxx=_inc_vbuxx 
+    inx
+  //SEG18 [8] if((byte) main::i#1<(byte/signed byte/word/signed word/dword/signed dword) $a) goto main::@1 -- vbuxx_lt_vbuc1_then_la1 
+    cpx #$a
+    bcc b1
+  //SEG19 main::@return
+  //SEG20 [9] return 
+    rts
+}
+
diff --git a/src/test/ref/for-empty-init.sym b/src/test/ref/for-empty-init.sym
new file mode 100644
index 000000000..33144f5d6
--- /dev/null
+++ b/src/test/ref/for-empty-init.sym
@@ -0,0 +1,13 @@
+(label) @1
+(label) @begin
+(label) @end
+(void()) main()
+(label) main::@1
+(label) main::@return
+(byte*) main::SCREEN
+(const byte*) main::SCREEN#0 SCREEN = ((byte*))(word/signed word/dword/signed dword) $400
+(byte) main::i
+(byte) main::i#1 reg byte x 16.5
+(byte) main::i#2 reg byte x 22.0
+
+reg byte x [ main::i#2 main::i#1 ]