Improve callstack errors, check for recursion.

[jmcardon]

This PR improves the state of error messages in core, as well as adds a runtime recursion check for module references.

Given this repl file:

(module caller g

  (defcap g () true)

  (defun caller ()
    (caller1)
  )

  (defun caller1 ()
    (caller2 31)
  )

  (defun caller2 (a:integer)
    (caller3 31 "bob`")
  )

  (defun caller3 (a:integer arg:string)
    (enforce false "whoopsie!")
  )
  )

(caller)

Pre-this PR, the following is a repl snippet of the error emitted by loading the file:

pact>(load "example.repl" true)
"Loading example.repl..."
Loaded module caller, hash Z9GxDb9xe8joF3x1SfeqbMuN9RrSSekRm5N2TBTJ5zI
example.repl:18:4: Program encountered an unhandled raised error: whoopsie!
 18 |     (enforce false "whoopsie!")
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^

Post-this, the error improves to

pact>(load "example.repl" true)
"Loading example.repl..."
Loaded module caller, hash Z9GxDb9xe8joF3x1SfeqbMuN9RrSSekRm5N2TBTJ5zI
example.repl:18:4: Program encountered an unhandled raised error: whoopsie!
 18 |     (enforce false "whoopsie!")
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  at (caller.caller3 31 "bob`")
  at (caller.caller2 31)
  at (caller.caller1)
  at (caller.caller)

PR checklist:

• Test coverage for the proposed changes
• PR description contains example output from repl interaction or a snippet from unit test output
• Any changes that could be relevant to users have been recorded in the changelog

[emilypi]

let's add a case for direct self-recursion as well. Also, is mutual recursion covered for both modrefs and two concrete mutually recursive calls?

[jmcardon]

No, a case with direct self recursion will not compile.

[emilypi]

So correct me if i'm wrong, but the process is as follows:

When we see a fqn that's new, we check the current frame calling context to see if there's already a reference to it anywhere, treating the NonEmpty RecursionCheck state as a second stack that tracks names that it's already seen, and if it has been seen already, abort, but if not, load it into the associated recursion tracking context if it's not.

This could get pretty costly. We'll need to find a way to gas this because any significantly complicated defun is going to have a lot of recursion checks no?

[0xd34df00d]

My gut feel is this shouldn't be too costly relative to the other stuff CEK machine is doing, but it'd make a great benchmark case! I can take a look at that separately soon-ish.

[jmcardon]

This could get pretty costly.

Why would this be costly? The set will not grow very large in practice, and it just checks a qualified name.

We'll need to find a way to gas this because any significantly complicated defun is going to have a lot of recursion checks no?

We already charge an overhead for function application. The overhead covers the cost of a few things anyway:

• Pushing the arguments onto the environment
• Pushing a stackframe into the context stack
• Recursion checking.

In practice, that set will be small, ankd even if it were 10000 functions deep, it would not be slow enough for it to matter in the grand scheme of things, I believe, but we will simply use the benchmarks for function app to verify this 👍