Fix an arg-handling bug in typeCallInternal

In typeCallInternal, we used to expand out any starargs in order to take a look
at the first arg (and change it when passing it).

We had a bug in this code, and rather than make that code more complicated
to fix it, just call back into callFunc to resolve it.  This is kind of tricky
since callFunc will call typeCall, and we don't want typeCall to duplicate
the typeCallInternal behavior (that's not any better than duplicating the
arg behavior), so we want typeCall to call into typeCallInternal.  But
typeCall receives varargs! which typeCallInternal doesn't support.  So typeCall
has to do some (simpler) arg handling to expand out the varargs.

In the end, it simplifies the code a little bit but causes a bunch of extra calls
in the varargs case, so it's less of a win than I thought, but at least it
fixes the bug.

src/runtime/objmodel.cpp

@@ -102,14 +102,6 @@ static Box* (*runtimeCallInternal2)(Box*, CallRewriteArgs*, ArgPassSpec, Box*, B
 static Box* (*runtimeCallInternal3)(Box*, CallRewriteArgs*, ArgPassSpec, Box*, Box*, Box*)
     = (Box * (*)(Box*, CallRewriteArgs*, ArgPassSpec, Box*, Box*, Box*))runtimeCallInternal;
 
-static Box* (*typeCallInternal1)(BoxedFunctionBase*, CallRewriteArgs* rewrite_args, ArgPassSpec argspec, Box*)
-    = (Box * (*)(BoxedFunctionBase*, CallRewriteArgs*, ArgPassSpec, Box*))typeCallInternal;
-static Box* (*typeCallInternal2)(BoxedFunctionBase*, CallRewriteArgs* rewrite_args, ArgPassSpec argspec, Box*, Box*)
-    = (Box * (*)(BoxedFunctionBase*, CallRewriteArgs*, ArgPassSpec, Box*, Box*))typeCallInternal;
-static Box* (*typeCallInternal3)(BoxedFunctionBase*, CallRewriteArgs* rewrite_args, ArgPassSpec argspec, Box*, Box*,
-                                 Box*)
-    = (Box * (*)(BoxedFunctionBase*, CallRewriteArgs*, ArgPassSpec, Box*, Box*, Box*))typeCallInternal;
-
 bool checkClass(LookupScope scope) {
     return (scope & CLASS_ONLY) != 0;
 }
@@ -3555,47 +3547,10 @@ Box* typeCallInternal(BoxedFunctionBase* f, CallRewriteArgs* rewrite_args, ArgPa
     static StatCounter slowpath_typecall("slowpath_typecall");
     slowpath_typecall.log();
 
-    // TODO shouldn't have to redo this argument handling here...
-    if (argspec.has_starargs) {
-        rewrite_args = NULL;
-        REWRITE_ABORTED("");
-
-        Box* starargs;
-        if (argspec.num_args == 0)
-            starargs = arg1;
-        else if (argspec.num_args == 1)
-            starargs = arg2;
-        else
-            abort();
-
-        assert(starargs->cls == tuple_cls);
-        BoxedTuple* targs = static_cast<BoxedTuple*>(starargs);
-
-        int n = targs->elts.size();
-
-        if (argspec.num_args == 0) {
-            if (n >= 1)
-                arg1 = targs->elts[0];
-            if (n >= 2)
-                arg2 = targs->elts[1];
-            if (n >= 3)
-                arg3 = targs->elts[2];
-            if (n >= 4)
-                args = &targs->elts[3];
-        } else if (argspec.num_args == 1) {
-            if (n >= 1)
-                arg2 = targs->elts[0];
-            if (n >= 2)
-                arg3 = targs->elts[1];
-            if (n >= 3)
-                args = &targs->elts[2];
-        } else {
-            abort(); // unhandled
-        }
-
-        argspec = ArgPassSpec(n + argspec.num_args);
-    }
+    if (argspec.has_starargs)
+        return callFunc(f, rewrite_args, argspec, arg1, arg2, arg3, args, keyword_names);
 
+    assert(argspec.num_args >= 1);
     Box* _cls = arg1;
 
     if (!isSubclass(_cls->cls, type_cls)) {
@@ -3867,16 +3822,24 @@ Box* typeCallInternal(BoxedFunctionBase* f, CallRewriteArgs* rewrite_args, ArgPa
     return made;
 }
 
-Box* typeCall(Box* obj, BoxedList* vararg) {
-    assert(vararg->cls == list_cls);
-    if (vararg->size == 0)
-        return typeCallInternal1(NULL, NULL, ArgPassSpec(1), obj);
-    else if (vararg->size == 1)
-        return typeCallInternal2(NULL, NULL, ArgPassSpec(2), obj, vararg->elts->elts[0]);
-    else if (vararg->size == 2)
-        return typeCallInternal3(NULL, NULL, ArgPassSpec(3), obj, vararg->elts->elts[0], vararg->elts->elts[1]);
-    else
-        abort();
+Box* typeCall(Box* obj, BoxedTuple* vararg, BoxedDict* kwargs) {
+    assert(vararg->cls == tuple_cls);
+
+    int n = vararg->elts.size();
+    int args_to_pass = n + 2; // 1 for obj, 1 for kwargs
+
+    Box** args = NULL;
+    if (args_to_pass > 3)
+        args = (Box**)alloca(sizeof(Box*) * (args_to_pass - 3));
+
+    Box* arg1, *arg2, *arg3;
+    arg1 = obj;
+    for (int i = 0; i < n; i++) {
+        getArg(i + 1, arg1, arg2, arg3, args) = vararg->elts[i];
+    }
+    getArg(n + 1, arg1, arg2, arg3, args) = kwargs;
+
+    return typeCallInternal(NULL, NULL, ArgPassSpec(n + 1, 0, false, true), arg1, arg2, arg3, args, NULL);
 }
 
 extern "C" void delGlobal(BoxedModule* m, const std::string* name) {

src/runtime/objmodel.h

@@ -29,6 +29,7 @@ class BoxedInt;
 class BoxedList;
 class BoxedString;
 class BoxedGenerator;
+class BoxedTuple;
 
 // user-level raise functions that implement python-level semantics
 extern "C" void raise0() __attribute__((__noreturn__));
@@ -130,7 +131,7 @@ extern "C" void raiseAttributeErrorStr(const char* typeName, const char* attr) _
 extern "C" void raiseAttributeError(Box* obj, const char* attr) __attribute__((__noreturn__));
 extern "C" void raiseNotIterableError(const char* typeName) __attribute__((__noreturn__));
 
-Box* typeCall(Box*, BoxedList*);
+Box* typeCall(Box*, BoxedTuple*, BoxedDict*);
 Box* typeNew(Box* cls, Box* arg1, Box* arg2, Box** _args);
 bool isUserDefined(BoxedClass* cls);
 

src/runtime/types.cpp

@@ -1226,7 +1226,7 @@ void setupRuntime() {
     object_cls->giveAttr("__str__", new BoxedFunction(boxRTFunction((void*)objectStr, UNKNOWN, 1, 0, false, false)));
     object_cls->freeze();
 
-    auto typeCallObj = boxRTFunction((void*)typeCall, UNKNOWN, 1, 0, true, false);
+    auto typeCallObj = boxRTFunction((void*)typeCall, UNKNOWN, 1, 0, true, true);
     typeCallObj->internal_callable = &typeCallInternal;
 
     type_cls->giveAttr("__name__", new BoxedGetsetDescriptor(type_name, type_set_name, NULL));

test/tests/constructor_args.py

@@ -6,3 +6,15 @@ C(1)
 C(a=2)
 C(*(3,))
 C(**{'a':4})
+
+
+# Regression test: make sure we pass these through correctly:
+class C(object):
+    def __init__(self, k=None):
+        print k
+
+def f(*args, **kw):
+    print args, kw
+    return C(*args, **kw)
+
+f(k=1)