Fix BinNat on C fixing a while loop compilation issue (#1287)

.github/workflows/ci.yml

@@ -127,14 +127,15 @@ jobs:
       - name: "build assembly"
         run: "sbt \"++${{matrix.scala}}; cli/assembly\""
       - name: "generate c code"
-        run: "./bosatsuj transpile --input_dir test_workspace/ --package_root test_workspace/ --outdir c_out c --test --filter Bosatsu/List --filter IntTest"
+        run: "./bosatsuj transpile --input_dir test_workspace/ --package_root test_workspace/ --outdir c_out c --test --filter Bosatsu/List --filter IntTest --filter Bosatsu/BinNat"
       - name: "compile generated c code"
         run: |
           cp c_runtime/*.h c_out
           cp c_runtime/*.c c_out
           cp c_runtime/Makefile.test c_out/Makefile
           cd c_out
           make
+          cat output.log
     timeout-minutes: 30
 name: ci
 on:

c_runtime/bosatsu_runtime.c

@@ -1888,7 +1888,12 @@ BSTS_PassFail bsts_check_test(BValue v, int indent) {
       this_fails += tests.fails;
     }
     print_indent(next_indent);
-    printf("passed: \033[32m%i\033[0m, failed: \033[31m%i\033[0m\n", this_passes, this_fails);
+    if (this_fails == 0) {
+      printf("passed: \033[32m%i\033[0m\n", this_passes);
+    }
+    else {
+      printf("passed: \033[32m%i\033[0m, failed: \033[31m%i\033[0m\n", this_passes, this_fails);
+    }
     passes += this_passes;
     fails += this_fails;
   }
@@ -1923,6 +1928,11 @@ int bsts_test_result_print_summary(int count, BSTS_Test_Result* results) {
     printf("\n");
   }
 
-  printf("\npassed: \033[32m%i\033[0m, failed: \033[31m%i\033[0m\n", total_passes, total_fails);
+  if (total_fails == 0) {
+    printf("\npassed: \033[32m%i\033[0m\n", total_passes);
+  }
+  else {
+    printf("\npassed: \033[32m%i\033[0m, failed: \033[31m%i\033[0m\n", total_passes, total_fails);
+  }
   return (total_fails > 0);
 }

cli/src/test/scala/org/bykn/bosatsu/codegen/clang/ClangGenTest.scala

@@ -45,7 +45,7 @@ class ClangGenTest extends munit.FunSuite {
       To inspect the code, change the hash, and it will print the code out
      */
     testFilesCompilesToHash("test_workspace/Ackermann.bosatsu")(
-      "9230e9e785b702777c0c019d86bc76eb"
+      "c4e556fd42f149731c0f31256db8a0b3"
     )
   }
 }

core/src/main/scala/org/bykn/bosatsu/codegen/clang/ClangGen.scala

@@ -244,7 +244,7 @@ object ClangGen {
 
       // assign any results to result and set the condition to false
       // and replace any tail calls to nm(args) with assigning args to those values
-      def toWhileBody(fnName: Code.Ident, args: NonEmptyList[Code.Param], isClosure: Boolean, cond: Code.Ident, result: Code.Ident, body: Code.ValueLike): Code.Block = {
+      def toWhileBody(fnName: Code.Ident, argTemp: NonEmptyList[(Code.Param, Code.Ident)], isClosure: Boolean, cond: Code.Ident, result: Code.Ident, body: Code.ValueLike): Code.Block = {
 
         import Code._
 
@@ -260,21 +260,24 @@ object ClangGen {
                 if (isClosure) appArgs.tail
                 else appArgs
               // we know the length of appArgs must match args or the code wouldn't have compiled
-              val assigns = args
+              // we have to first assign to the temp variables, and then assign the temp variables
+              // to the results to make sure we don't have any data dependency issues with the values;
+              val tmpAssigns = argTemp
                 .iterator
                 .zip(newArgsList.iterator)
-                .flatMap { case (Param(_, name), value) =>
+                .flatMap { case ((Param(_, name), tmp), value) =>
                   if (name != value)
                     // don't create self assignments
-                    Iterator.single(Assignment(name, value))
+                    Iterator.single((Assignment(tmp, value), Assignment(name, tmp)))
                   else
                     Iterator.empty
                 }
                 .toList
 
-              // there is always at least one new argument or the loop
-              // won't terminate
-              val assignNEL = NonEmptyList.fromListUnsafe(assigns)
+              // there must be at least one assignment
+              val assignNEL = NonEmptyList.fromListUnsafe(
+                tmpAssigns.map(_._1) ::: tmpAssigns.map(_._2)
+              )
               Some(Statements(assignNEL))
             case IfElseValue(c, t, f) =>
               // this can possible have tail calls inside the branches
@@ -1138,16 +1141,23 @@ object ClangGen {
                   cond <- newLocalName("cond")
                   res <- newLocalName("res")
                   bodyVL <- innerToValue(body)
-                  argParams <- args.traverse { b =>
-                    getBinding(b).map { i => Code.Param(Code.TypeIdent.BValue, i) }
+                  argParamsTemps <- args.traverse { b =>
+                    (getBinding(b), newLocalName("loop_temp")).mapN { (i, t) => (Code.Param(Code.TypeIdent.BValue, i), t) }
                   }
-                  whileBody = toWhileBody(fnName, argParams, isClosure = captures.nonEmpty, cond = cond, result = res, body = bodyVL)
+                  whileBody = toWhileBody(fnName, argParamsTemps, isClosure = captures.nonEmpty, cond = cond, result = res, body = bodyVL)
+                  declTmps = Code.Statements(
+                    argParamsTemps.map { case (_, tmp) =>
+                      Code.DeclareVar(Nil, Code.TypeIdent.BValue, tmp, None)
+                    }
+                  )
                   fnBody = Code.block(
+                    declTmps,
                     Code.DeclareVar(Nil, Code.TypeIdent.Bool, cond, Some(Code.TrueLit)),
                     Code.DeclareVar(Nil, Code.TypeIdent.BValue, res, None),
                     Code.While(cond, whileBody),
                     Code.Return(Some(res))
                   )
+                  argParams = argParamsTemps.map(_._1)
                   allArgs =
                     if (captures.isEmpty) argParams
                     else {

core/src/test/scala/org/bykn/bosatsu/codegen/clang/ClangGenTest.scala

@@ -84,6 +84,8 @@ BValue ___bsts_g_Bosatsu_l_Predef_l_foldr__List(BValue __bsts_b_list0,
 BValue __bsts_t_closure0(BValue* __bstsi_slot,
     BValue __bsts_b_lst1,
     BValue __bsts_b_item1) {
+    BValue __bsts_l_loop__temp3;
+    BValue __bsts_l_loop__temp4;
     _Bool __bsts_l_cond1 = 1;
     BValue __bsts_l_res2;
     while (__bsts_l_cond1) {
@@ -94,10 +96,12 @@ BValue __bsts_t_closure0(BValue* __bstsi_slot,
         else {
             BValue __bsts_b_head0 = get_enum_index(__bsts_b_lst1, 0);
             BValue __bsts_b_tail0 = get_enum_index(__bsts_b_lst1, 1);
-            __bsts_b_lst1 = __bsts_b_tail0;
-            __bsts_b_item1 = call_fn2(__bstsi_slot[0],
+            __bsts_l_loop__temp3 = __bsts_b_tail0;
+            __bsts_l_loop__temp4 = call_fn2(__bstsi_slot[0],
                 __bsts_b_item1,
                 __bsts_b_head0);
+            __bsts_b_lst1 = __bsts_l_loop__temp3;
+            __bsts_b_item1 = __bsts_l_loop__temp4;
         }
     }
     return __bsts_l_res2;
@@ -106,22 +110,22 @@ BValue __bsts_t_closure0(BValue* __bstsi_slot,
 BValue ___bsts_g_Bosatsu_l_Predef_l_foldLeft(BValue __bsts_b_lst0,
     BValue __bsts_b_item0,
     BValue __bsts_b_fn0) {
-    BValue __bsts_l_captures3[1] = { __bsts_b_fn0 };
+    BValue __bsts_l_captures5[1] = { __bsts_b_fn0 };
     BValue __bsts_b_loop0 = alloc_closure2(1,
-        __bsts_l_captures3,
+        __bsts_l_captures5,
         __bsts_t_closure0);
     return call_fn2(__bsts_b_loop0, __bsts_b_lst0, __bsts_b_item0);
 }
 
-BValue __bsts_t_lambda4(BValue __bsts_b_tail0, BValue __bsts_b_h0) {
+BValue __bsts_t_lambda6(BValue __bsts_b_tail0, BValue __bsts_b_h0) {
     return alloc_enum2(1, __bsts_b_h0, __bsts_b_tail0);
 }
 
 BValue ___bsts_g_Bosatsu_l_Predef_l_reverse__concat(BValue __bsts_b_front0,
     BValue __bsts_b_back0) {
     return ___bsts_g_Bosatsu_l_Predef_l_foldLeft(__bsts_b_front0,
         __bsts_b_back0,
-        alloc_boxed_pure_fn2(__bsts_t_lambda4));
+        alloc_boxed_pure_fn2(__bsts_t_lambda6));
 }""")
   }
 }

test_workspace/BinNat.bosatsu

@@ -34,10 +34,10 @@ def toNat(b: BinNat) -> Nat:
 def toBinNat(n: Int) -> BinNat:
   # build up a list in reverse of transformations
   fns = int_loop(n, [], \n, fns ->
-    is_even = mod_Int(n, 2).eq_Int(0)
+    is_even = n.and_Int(1) matches 0
     (hfn, dec) = (n -> Even(n), n -> n.sub(1)) if is_even else (n -> Odd(n), n -> n)
     fns = [hfn, *fns]
-    n = n.div(2)
+    n = n.shift_right_Int(1)
     (dec(n), fns)
   )
   # Now apply all the transformations
@@ -372,7 +372,7 @@ test = TestSuite(
     Assertion(fib(Zero).toInt().eq_Int(1), "fib(0) == 1"),
     Assertion(fib(one).toInt().eq_Int(1), "fib(1) == 1"),
     Assertion(fib(two).toInt().eq_Int(2), "fib(2) == 2"),
-    Assertion(fib(three).toInt().eq_Int(3), "fib(3) == 3"),
+    Assertion(fib(three).toInt().eq_Int(3), "fib(3) == 3 (got ${int_to_String(fib(three).toInt())})"),
     Assertion(fib(four).toInt().eq_Int(5), "fib(4) == 5"),
     Assertion(cmp_BinNat(54.toBinNat(), 54.toBinNat()) matches EQ, "54 == 54"),
   ])

test_workspace/IntTest.bosatsu

@@ -17,6 +17,11 @@ diff1 = 36893488147419103232 - 1
 and_test_cases = [
     # Small positive integers
     (3 & 1, 1, "3 & 1"),
+    (0 & 1, 0, "0 & 1"),
+    (1 & 1, 1, "1 & 1"),
+    (2 & 1, 0, "2 & 1"),
+    (3 & 1, 1, "3 & 1"),
+    (4 & 1, 0, "4 & 1"),
     # Small negative integers
     (-3 & 1, 1, "-3 & 1"),
     (-5 & -2, -6, "-5 & -2"),
@@ -41,7 +46,9 @@ and_test_cases = [
 ]
 
 and_tests = TestSuite("and tests",
-  and_test_cases.map_List(((got, ex, m)) -> assert_eq(got, ex, m)))
+  [ Assertion((2 & 1) matches 0, "2 & 1 matches 0"),
+    *and_test_cases.map_List(((got, ex, m)) -> assert_eq(got, ex, m))
+  ])
 
 or_test_cases = [
     # Small positive integers