[PATCH] pkl: Use `format` function in `assert` implementation

[Mohammad-Reza Nabipoor]

This commit changes the implementation of `assert` statement to
pass source location info (FILENAME, LINE and COLUMN) separately,
instead of passing a pre-formated string
("<FILENAME>:<LINE>:<COLUMN>"). And makes `msg` string using `format`
function.

The reason that this commit, separates the runtime library (`pkl-rt.pk`)
into two separate files (`pkl-rt-1.pk` and `pkl-rt-2.pk`) is that
codegen of `format` generates a function call to
`_pkl_reduce_string_array`, and it's inaccessible to codegen at this
stage. Because `_pkl_reduce_string_array` is also defined in the runtime
library. Moving `_pkl_assert` function to `pkl-rt-2.pk` makes codegen
happy!

2021-06-16  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>

        * libpoke/pkl-tab.y (pkl_make_assertion): Change the invocation of
        `_pkl_assert` to pass FILENAME, LINE and COLUMN as separate arguments
        instead of passing as a pre-formated string.
        * libpoke/pkl.c (pkl_load_rt): New function to load runtime library.
        (pkl_new): Load `pkl-rt-1.pk` and `pkl-rt-2.pk` runtime libraries.
        * libpoke/pkl-rt.pk: Rename to `libpoke/pkl-rt-1`.
        (_pkl_assert): Remove.
        * libpoke/pkl-rt-1.pk: New file.
        * libpoke/pkl-rt-2.pk: New file. New implementation of `_pkl_assert`
        which uses `format` to report FILENAME, LINE and COLUMN.
        * libpoke/Makefile.am (dist_pkgdata_DATA): Update
        * etc/hacking.org: Rename `pkl-rt.pk` to what's appropriate.
        * HACKING: Likewise.
        * doc/learn-poke-language-in-y-minutes.pk: Likewise.
        * libpoke/pvm.h: Likewise.
        * libpoke/pvm-val.c: Likewise.
        * libpoke/ras: Likewise.
---


Hi, Jose.

Side suggestion: WDYT about having an ASTREF_CHAIN function?

In `pkl_make_assertion` function in `libpoke/pkl-tab.y`,
I've called `ASTREF` for `arg_{name,msg,fname,line,col}` AST nodes
explicitly. But ASTREF will be called on `arg_cond` (the first
funcall_arg) in `pkl_ast_make_funcall` function.

Isn't it more consistent if `pkl_ast_make_funcall` calls `ASTREF`
for all chained arguments?


Regards,
Mohammad-Reza


 ChangeLog                               | 20 +++++++++
 HACKING                                 | 10 ++---
 doc/learn-poke-language-in-y-minutes.pk |  2 +-
 etc/hacking.org                         | 10 ++---
 libpoke/Makefile.am                     |  2 +-
 libpoke/{pkl-rt.pk => pkl-rt-1.pk}      | 22 +---------
 libpoke/pkl-rt-2.pk                     | 39 +++++++++++++++++
 libpoke/pkl-tab.y                       | 58 +++++++++++++++----------
 libpoke/pkl.c                           | 43 ++++++++++++------
 libpoke/pvm-val.c                       |  2 +-
 libpoke/pvm.h                           |  2 +-
 libpoke/ras                             |  2 +-
 12 files changed, 138 insertions(+), 74 deletions(-)
 rename libpoke/{pkl-rt.pk => pkl-rt-1.pk} (95%)
 create mode 100644 libpoke/pkl-rt-2.pk

diff --git a/ChangeLog b/ChangeLog
index 76ea7c5d..5c6ba08a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,23 @@
+2021-06-16  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
+
+       * libpoke/pkl-tab.y (pkl_make_assertion): Change the invocation of
+       `_pkl_assert` to pass FILENAME, LINE and COLUMN as separate arguments
+       instead of passing as a pre-formated string.
+       * libpoke/pkl.c (pkl_load_rt): New function to load runtime library.
+       (pkl_new): Load `pkl-rt-1.pk` and `pkl-rt-2.pk` runtime libraries.
+       * libpoke/pkl-rt.pk: Rename to `libpoke/pkl-rt-1`.
+       (_pkl_assert): Remove.
+       * libpoke/pkl-rt-1.pk: New file.
+       * libpoke/pkl-rt-2.pk: New file. New implementation of `_pkl_assert`
+       which uses `format` to report FILENAME, LINE and COLUMN.
+       * libpoke/Makefile.am (dist_pkgdata_DATA): Update
+       * etc/hacking.org: Rename `pkl-rt.pk` to what's appropriate.
+       * HACKING: Likewise.
+       * doc/learn-poke-language-in-y-minutes.pk: Likewise.
+       * libpoke/pvm.h: Likewise.
+       * libpoke/pvm-val.c: Likewise.
+       * libpoke/ras: Likewise.
+
 2021-06-06  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
 
        * common/pk-utils.h (pk_format_binary): New function declaration.
diff --git a/HACKING b/HACKING
index 815096ca..a09a4ea5 100644
--- a/HACKING
+++ b/HACKING
@@ -631,7 +631,7 @@ with GNU poke.  If not, see [https://www.gnu.org/licenses/].
   to perform some operation defined recursively.
 
   In these situations, the solution is to first write a Poke function in
-  the compiler's runtime library, `pkl-rt.pk', like:
+  the compiler's runtime library, `pkl-rt*.pk', like:
 
   ,----
   | fun _pkl_foo = (uint<64> ival, uint<64> eval,
@@ -1572,7 +1572,7 @@ with GNU poke.  If not, see 
[https://www.gnu.org/licenses/].
 
   ,----
   | The final step is to define the built-in function proper, in the
-  | compiler run-time, in =pkl-rt.pk=:
+  | compiler run-time, in =pkl-rt-1.pk=:
   `----
 
   ,----
@@ -1587,10 +1587,10 @@ with GNU poke.  If not, see 
[https://www.gnu.org/licenses/].
 ~~~~~~~~~~~~~~~~~~~~~~~
 
   Exception types or codes are signed 32-bit integers, and are defined
-  in `libpoke/pkl-rt.pk' as `EC_*' variables.
+  in `libpoke/pkl-rt-1.pk' as `EC_*' variables.
 
   The exceptions themselves are struct values of type Exception, also
-  defined in `libpoke/pkl-rt.pk'.
+  defined in `libpoke/pkl-rt-1.pk'.
 
   There are two ways an exception can be raised in the PVM:
 
@@ -1611,7 +1611,7 @@ with GNU poke.  If not, see 
[https://www.gnu.org/licenses/].
   is installed by the macro-assembler in `libpoke/pkl-asm.c:pkl_asm_new'
   and `libpoke/pkl-asm.c:pkl_asm_finish'.  It calls the function
   `_pkl_exception_handler', that is defined in the compiler runtime in
-  `libpoke/pkl-rt.pkl'.
+  `libpoke/pkl-rt-1.pkl'.
 
 
 12.2 Signal Handling
diff --git a/doc/learn-poke-language-in-y-minutes.pk 
b/doc/learn-poke-language-in-y-minutes.pk
index 738319d4..9b4e05b7 100644
--- a/doc/learn-poke-language-in-y-minutes.pk
+++ b/doc/learn-poke-language-in-y-minutes.pk
@@ -581,7 +581,7 @@ catch (Exception e)
   }
 
 /* Exception codes in the range `0..254` are reserved for Poke. These codes
- * are defined by `EC_*` variables in `pkl-rt.pk` file.
+ * are defined by `EC_*` variables in `pkl-rt-1.pk` file.
  *
  * Users also can define their own exceptions by calling `exception_code`
  * function.
diff --git a/etc/hacking.org b/etc/hacking.org
index cd6a21f2..87cb40c2 100644
--- a/etc/hacking.org
+++ b/etc/hacking.org
@@ -451,7 +451,7 @@ Indu Bhagat             <indu.bhagat@oracle.com>
     complex formulae or to perform some operation defined recursively.
 
     In these situations, the solution is to first write a Poke
-    function in the compiler's runtime library, =pkl-rt.pk=, like:
+    function in the compiler's runtime library, =pkl-rt*.pk=, like:
 
     : fun _pkl_foo = (uint<64> ival, uint<64> eval,
     :                 uint<32> ivalw, uint<32> fieldw) uint<64>:
@@ -1256,7 +1256,7 @@ have a testsuite in =testsuite/poke.FOO= with a driver
    :      break;
 
    : The final step is to define the built-in function proper, in the
-   : compiler run-time, in =pkl-rt.pk=:
+   : compiler run-time, in =pkl-rt-1.pk=:
 
    : fun rand = int<32>: __PKL_BUILTIN_RAND__;
 
@@ -1265,10 +1265,10 @@ have a testsuite in =testsuite/poke.FOO= with a driver
 ** Exception Handling
 
    Exception types or codes are signed 32-bit integers, and are
-   defined in =libpoke/pkl-rt.pk= as =EC_*= variables.
+   defined in =libpoke/pkl-rt-1.pk= as =EC_*= variables.
 
    The exceptions themselves are struct values of type Exception, also
-   defined in =libpoke/pkl-rt.pk=.
+   defined in =libpoke/pkl-rt-1.pk=.
 
    There are two ways an exception can be raised in the PVM:
 
@@ -1291,7 +1291,7 @@ have a testsuite in =testsuite/poke.FOO= with a driver
    =libpoke/pkl-asm.c:pkl_asm_new= and
    =libpoke/pkl-asm.c:pkl_asm_finish=.  It calls the function
    =_pkl_exception_handler=, that is defined in the compiler runtime
-   in =libpoke/pkl-rt.pkl=.
+   in =libpoke/pkl-rt-1.pkl=.
 
 ** Signal Handling
 
diff --git a/libpoke/Makefile.am b/libpoke/Makefile.am
index d549f662..428cd827 100644
--- a/libpoke/Makefile.am
+++ b/libpoke/Makefile.am
@@ -22,7 +22,7 @@ CLEANFILES =
 DISTCLEANFILES =
 MAINTAINERCLEANFILES =
 
-dist_pkgdata_DATA = pkl-rt.pk std.pk std-types.pk
+dist_pkgdata_DATA = pkl-rt-1.pk pkl-rt-2.pk std.pk std-types.pk
 dist_pkgconfig_lib_DATA = $(pkgconfig_libfile)
 
 lib_LTLIBRARIES = libpoke.la
diff --git a/libpoke/pkl-rt.pk b/libpoke/pkl-rt-1.pk
similarity index 95%
rename from libpoke/pkl-rt.pk
rename to libpoke/pkl-rt-1.pk
index 9535f22e..14f70c96 100644
--- a/libpoke/pkl-rt.pk
+++ b/libpoke/pkl-rt-1.pk
@@ -1,4 +1,4 @@
-/* pkl-rt.pkl - Run-time library for the poke compiler.  */
+/* pkl-rt-1.pk - Run-time library for the poke compiler.  */
 
 /* Copyright (C) 2019, 2020, 2021 Jose E. Marchesi.  */
 
@@ -196,26 +196,6 @@ fun _pkl_exception_handler = (Exception exception) int<32>:
    return exception.exit_status;
   }
 
-/* Assertion function.
-
-   The compiler transforms assert statement to invocation of this
-   function.  COND is first argument of assert statement, and MSG is
-   the optional second argument.  LINEINFO contains the source location
-   of the assert statement formatted like "<FILENAME>:<LINE>:<COLUMN>".
-   */
-
-fun _pkl_assert = (uint<64> cond, string msg, string lineinfo) void:
-  {
-    if (cond)
-      return;
-
-    raise Exception {
-      code = EC_assert,
-      msg = "assertion failed at " + lineinfo + (msg'length ? ": " + msg : ""),
-      exit_status = 1,
-    };
-  }
-
 /* Exit a Poke program with the given exit status code.  This is equivalent
    to raise an E_exit exception, but provides a more conventional
    syntax.  */
diff --git a/libpoke/pkl-rt-2.pk b/libpoke/pkl-rt-2.pk
new file mode 100644
index 00000000..bdc73f77
--- /dev/null
+++ b/libpoke/pkl-rt-2.pk
@@ -0,0 +1,39 @@
+/* pkl-rt-2.pkl - Run-time library for the poke compiler.  */
+
+/* Copyright (C) 2021 The poke authors.  */
+
+/* This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/* Assertion function.
+
+   The compiler transforms assert statement to invocation of this
+   function.  COND is first argument of assert statement, and MSG is
+   the optional second argument.  FILENAME is the name of source
+   file.  LINE and COL are, respectively, line and column number
+   of the assert statement in the source file.  */
+
+fun _pkl_assert = (uint<64> cond, string msg, string filename,
+                   uint<64> line, uint<64> col) void:
+  {
+    if (cond)
+      return;
+
+    raise Exception {
+      code = EC_assert,
+      msg = format ("assertion failed at %s:%u64d:%u64d%s",
+                    filename, line, col, msg'length ? ": " + msg : ""),
+      exit_status = 1,
+    };
+  }
diff --git a/libpoke/pkl-tab.y b/libpoke/pkl-tab.y
index f8ce36c2..8c1913b4 100644
--- a/libpoke/pkl-tab.y
+++ b/libpoke/pkl-tab.y
@@ -134,7 +134,8 @@ pkl_make_assertion (struct pkl_parser *p, pkl_ast_node 
cond, pkl_ast_node msg,
                     struct pkl_ast_loc stmt_loc)
 {
   pkl_ast_node vfunc, call, call_arg;
-  pkl_ast_node arg_cond, arg_msg, arg_lineinfo; /* _pkl_assert args */
+  /* _pkl_assert args */
+  pkl_ast_node arg_cond, arg_msg, arg_fname, arg_line, arg_col;
 
   /* Make variable for `_pkl_assert` function */
   {
@@ -154,43 +155,52 @@ pkl_make_assertion (struct pkl_parser *p, pkl_ast_node 
cond, pkl_ast_node msg,
                               vfunc_init, back, over);
   }
 
-  /* First argument of _pkl_assert */
+  /* First argument of _pkl_assert: condition */
   arg_cond = pkl_ast_make_funcall_arg (p->ast, cond, NULL);
   PKL_AST_LOC (arg_cond) = PKL_AST_LOC (cond);
 
-  /* Second argument of _pkl_assert */
+  /* Second argument of _pkl_assert: user message */
   if (msg == NULL)
     {
-      pkl_ast_node stype = pkl_ast_make_string_type (p->ast);
-
       msg = pkl_ast_make_string (p->ast, "");
-      PKL_AST_TYPE (msg) = ASTREF (stype);
+      PKL_AST_TYPE (msg) = ASTREF (pkl_ast_make_string_type (p->ast));
     }
   arg_msg = pkl_ast_make_funcall_arg (p->ast, msg, NULL);
   arg_msg = ASTREF (arg_msg);
   PKL_AST_LOC (arg_msg) = PKL_AST_LOC (msg);
 
-  /* Third argument of _pkl_assert to report the location of the assert
-     statement with the following format "<FILENAME>:<LINE>:<COLUMN>".  */
+  /* Third argument of _pkl_assert: file name */
+  {
+    pkl_ast_node fname
+        = pkl_ast_make_string (p->ast, p->filename ? p->filename : "<stdin>");
+
+    PKL_AST_TYPE (fname) = ASTREF (pkl_ast_make_string_type (p->ast));
+    arg_fname = pkl_ast_make_funcall_arg (p->ast, fname, NULL);
+    arg_fname = ASTREF (arg_fname);
+  }
+
+  /* Fourth argument of _pkl_assert: line */
   {
-    char *str;
-    pkl_ast_node lineinfo, stype;
-
-    if (asprintf (&str, "%s:%d:%d", p->filename ? p->filename : "<stdin>",
-                  stmt_loc.first_line, stmt_loc.first_column)
-        == -1)
-      return NULL;
-    lineinfo = pkl_ast_make_string (p->ast, str);
-    free (str);
-    stype = pkl_ast_make_string_type (p->ast);
-    PKL_AST_TYPE (lineinfo) = ASTREF (stype);
-
-    arg_lineinfo = pkl_ast_make_funcall_arg (p->ast, lineinfo, NULL);
-    arg_lineinfo = ASTREF (arg_lineinfo);
+    pkl_ast_node line = pkl_ast_make_integer (p->ast, stmt_loc.first_line);
+
+    PKL_AST_TYPE (line) = ASTREF (pkl_ast_make_integral_type (p->ast, 64, 0));
+    arg_line = pkl_ast_make_funcall_arg (p->ast, line, NULL);
+    arg_line = ASTREF (arg_line);
+  }
+
+  /* Fifth argument of _pkl_assert: column */
+  {
+    pkl_ast_node col = pkl_ast_make_integer (p->ast, stmt_loc.first_column);
+
+    PKL_AST_TYPE (col) = ASTREF (pkl_ast_make_integral_type (p->ast, 64, 0));
+    arg_col = pkl_ast_make_funcall_arg (p->ast, col, NULL);
+    arg_col = ASTREF (arg_col);
   }
 
-  call_arg
-      = pkl_ast_chainon (arg_cond, pkl_ast_chainon (arg_msg, arg_lineinfo));
+  call_arg = pkl_ast_chainon (arg_line, arg_col);
+  call_arg = pkl_ast_chainon (arg_fname, call_arg);
+  call_arg = pkl_ast_chainon (arg_msg, call_arg);
+  call_arg = pkl_ast_chainon (arg_cond, call_arg);
   call = pkl_ast_make_funcall (p->ast, vfunc, call_arg);
   return pkl_ast_make_exp_stmt (p->ast, call);
 }
diff --git a/libpoke/pkl.c b/libpoke/pkl.c
index 46d4bc7e..2f5c274f 100644
--- a/libpoke/pkl.c
+++ b/libpoke/pkl.c
@@ -72,6 +72,27 @@ struct pkl_compiler
   pkl_alien_token_handler_fn alien_token_fn;
 };
 
+
+static int
+pkl_load_rt(pkl_compiler compiler, char *poke_rt_pk)
+{
+  if (!pkl_execute_file (compiler, poke_rt_pk, NULL))
+    {
+      free (poke_rt_pk);
+
+      pk_term_class ("error");
+      pk_puts ("internal error: ");
+      pk_term_end_class ("error");
+      pk_puts ("compiler failed to bootstrap itself\n");
+
+      pkl_free (compiler);
+      return 0;
+    }
+  free (poke_rt_pk);
+  compiler->bootstrapped = 1;
+  return 1;
+}
+
 pkl_compiler
 pkl_new (pvm vm, const char *rt_path, uint32_t flags)
 {
@@ -99,25 +120,19 @@ pkl_new (pvm vm, const char *rt_path, uint32_t flags)
   /* Bootstrap the compiler.  An error bootstraping is an internal
      error and should be reported as such.  */
   {
-    char *poke_rt_pk = pk_str_concat (rt_path, "/pkl-rt.pk", NULL);
+    char *poke_rt_pk = pk_str_concat (rt_path, "/pkl-rt-1.pk", NULL);
     if (!poke_rt_pk)
       goto out_of_memory;
 
-    if (!pkl_execute_file (compiler, poke_rt_pk, NULL))
-      {
-        free (poke_rt_pk);
-
-        pk_term_class ("error");
-        pk_puts ("internal error: ");
-        pk_term_end_class ("error");
-        pk_puts ("compiler failed to bootstrap itself\n");
+    if (!pkl_load_rt (compiler, poke_rt_pk))
+      return NULL;
 
-        pkl_free (compiler);
-        return NULL;
-      }
-    free (poke_rt_pk);
+    poke_rt_pk = pk_str_concat (rt_path, "/pkl-rt-2.pk", NULL);
+    if (!poke_rt_pk)
+      goto out_of_memory;
 
-    compiler->bootstrapped = 1;
+    if (!pkl_load_rt (compiler, poke_rt_pk))
+      return NULL;
   }
 
 #define LOAD_STD(NAME)                                          \
diff --git a/libpoke/pvm-val.c b/libpoke/pvm-val.c
index b938c3ea..d6fc812a 100644
--- a/libpoke/pvm-val.c
+++ b/libpoke/pvm-val.c
@@ -1650,7 +1650,7 @@ pvm_call_pretty_printer (pvm vm, pvm_val val)
 }
 
 /* IMPORTANT: please keep pvm_make_exception in sync with the
-   definition of the struct Exception in pkl-rt.pk.  */
+   definition of the struct Exception in pkl-rt-1.pk.  */
 
 pvm_val
 pvm_make_exception (int code, char *message, int exit_status)
diff --git a/libpoke/pvm.h b/libpoke/pvm.h
index ad9e9939..87082c62 100644
--- a/libpoke/pvm.h
+++ b/libpoke/pvm.h
@@ -477,7 +477,7 @@ enum pvm_exit_code
   };
 
 /* Exceptions.  These should be in sync with the exception code
-   variables, and the exception messages, declared in pkl-rt.pkl */
+   variables, and the exception messages, declared in pkl-rt-1.pk */
 
 #define PVM_E_GENERIC       0
 #define PVM_E_GENERIC_MSG "generic"
diff --git a/libpoke/ras b/libpoke/ras
index 3f300332..3dc6709b 100755
--- a/libpoke/ras
+++ b/libpoke/ras
@@ -160,7 +160,7 @@
 #
 # The called function should be defined in the global environment.
 # Most typically this directive is used to call functions defined in
-# the compiler's run-time library (pkl-rt.pk).
+# the compiler's run-time library (pkl-rt*.pk).
 #
 # Literal C Code
 # --------------
-- 
2.32.0