[FORK] Add runtime type check for function return types

[imalsogreg]

TODO:

• Benchmarks (in a new PR)

Runtime type checking previously ignored user's type annotations for a function's return type. This repl code would run to completion, even though it is not well typed:

(module m g
  (defcap g () true)

  ;; `foo` declares that it returns a string, but it actually returns integer
  (defun foo:string () 1)

  ;; This function calls `foo`. It fails to `typecheck`, but still succeeds
  ;; at runtime to use `foo`'s integer return value.
  (defun use_foo:integer () (foo) + 2))

(m.use_foo)

This PR adds runtime type checking to function application, ensuring that the body evaluates to a term that has the type declared in the function's type signature.

The typechecking is behind a feature flag in case previous contracts made use of this accidental behavior.

Without the feature flag:

pact> (m.use_foo)
2

With the feature flag:

pact> (env-exec-config ["DisablePact46"])
pact> (m.use_foo)
<interactive>:7:28:Error: Type error: expected string, found integer
 at <interactive>:0:0: (use_foo)

[jwiegley]

Shouldn't this be something like unlessExecutionFlagSet FlagDisablePact47? It seems odd that I would want this 4.7 functionality when I'm disabling 4.6 functionality...

[jmcardon]

this should be DisablePact47

[imalsogreg]

Ok, I see how the flags are meant to be read now and updated the PR. DisablePact47 is the only flag used.

Type checking happens unless DisablePact47 is set.

The tests first use normal behavior (no flag set, typechecking turned on), then they use the prior behavior (DisablePact47 is set, typechecking turned off).

[sirlensalot]

A general comment/question: one reason in the past this was avoided was that since natives typecheck, and arguments to user functions are typechecked, return-value typechecking would only avoid typechecking for toplevel functions, since any other application would get caught via being computed on elsewhere.

With this change we are thus typechecking intermediate values twice.

Unfortunately, our benchmarks do not cover user function applications which is regrettable. I might suggest we open a separate PR with some user-application benchmarks -- and we'd want to have them with objects and lists since these are the most expensive to typecheck -- get that merged, and then we could compare this PR to master. Thoughts?

[imalsogreg]

@sirlensalot Good point about performance. We could either add benchmarking code, measure replay time, or both. Is replay time a good enough metric for now, adding benchmarks as needed if replay results are ambiguous?

After some more discussion, yah I think benchmarking makes sense.

[emilypi]

@imalsogreg if you vendor in the changes in #1212, we should have this PR passing.

[imalsogreg]

@emilypi you are correct! So now I just need to add benchmarks and do the replay.

[sirlensalot]

Approved with small spellcheck