From 45c67fae0c2fbe78b608d372d0951ffe05f02690 Mon Sep 17 00:00:00 2001
From: ruby0x1 <ruby0x1@pm.me>
Date: Fri, 18 Sep 2020 15:42:37 -0700
Subject: [PATCH] Fn call: move arity check into interpret loop, which avoid
 the expensive if after the call, since runtime errors originating inside the
 call itself will still be handled, we only have the one emitted from call
 itself.

This brings the benchmark back up to where it was.
---
 src/vm/wren_core.c                         |  7 -------
 src/vm/wren_vm.c                           | 21 ++++++++++++++++++++-
 test/core/function/call_runtime_error.wren |  3 +++
 3 files changed, 23 insertions(+), 8 deletions(-)
 create mode 100644 test/core/function/call_runtime_error.wren

diff --git a/src/vm/wren_core.c b/src/vm/wren_core.c
index 5f9f8974..0aaf8c29 100644
--- a/src/vm/wren_core.c
+++ b/src/vm/wren_core.c
@@ -248,13 +248,6 @@ DEF_PRIMITIVE(fn_arity)
 
 static void call_fn(WrenVM* vm, Value* args, int numArgs)
 {
-  // We only care about missing arguments, not extras.
-  if (AS_CLOSURE(args[0])->fn->arity > numArgs)
-  {
-    vm->fiber->error = CONST_STRING(vm, "Function expects more arguments.");
-    return;
-  }
-
   // +1 to include the function itself.
   wrenCallFunction(vm, vm->fiber, AS_CLOSURE(args[0]), numArgs + 1);
 }
diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c
index 984b2cc4..42e4dd97 100644
--- a/src/vm/wren_vm.c
+++ b/src/vm/wren_vm.c
@@ -774,6 +774,21 @@ static Value getModuleVariable(WrenVM* vm, ObjModule* module,
   return NULL_VAL;
 }
 
+inline static bool checkArity(WrenVM* vm, Value value, int numArgs)
+{
+  ASSERT(IS_CLOSURE(value), "Receiver must be a closure.");
+  ObjFn* fn = AS_CLOSURE(value)->fn;
+
+  // We only care about missing arguments, not extras. The "- 1" is because
+  // numArgs includes the receiver, the function itself, which we don't want to
+  // count.
+  if (numArgs - 1 >= fn->arity) return true;
+
+  vm->fiber->error = CONST_STRING(vm, "Function expects more arguments.");
+  return false;
+}
+
+
 // The main bytecode interpreter loop. This is where the magic happens. It is
 // also, as you can imagine, highly performance critical.
 static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber)
@@ -1016,9 +1031,13 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber)
           break;
 
         case METHOD_FUNCTION_CALL: 
+          if (!checkArity(vm, args[0], numArgs)) {
+            RUNTIME_ERROR();
+            break;
+          }
+
           STORE_FRAME();
           method->as.primitive(vm, args);
-          if (wrenHasError(fiber)) RUNTIME_ERROR();
           LOAD_FRAME();
           break;
 
diff --git a/test/core/function/call_runtime_error.wren b/test/core/function/call_runtime_error.wren
new file mode 100644
index 00000000..71a6edcc
--- /dev/null
+++ b/test/core/function/call_runtime_error.wren
@@ -0,0 +1,3 @@
+var f1 = Fn.new {|a, b| a + b } // expect runtime error: Bool does not implement '+(_)'.
+f1.call(true, false)
+