Let object.__new__ take kwargs

Also, improve rewriting to still be able to rewrite object construction.
For now, be able to rewrite the case that a function takes kwargs but
the kwargs is empty.

Also, add an even faster path to typeCallInternal.  This is partially
obviated by the improved rewriting, but we might as well keep it.

src/runtime/objmodel.cpp

@@ -2312,12 +2312,15 @@ static std::string getFunctionName(CLFunction* f) {
     return "<unknown function>";
 }
 
-static void placeKeyword(const ParamNames& param_names, std::vector<bool>& params_filled, const std::string& kw_name,
-                         Box* kw_val, Box*& oarg1, Box*& oarg2, Box*& oarg3, Box** oargs, BoxedDict* okwargs,
-                         CLFunction* cl) {
+enum class KeywordDest {
+    POSITIONAL,
+    KWARGS,
+};
+static KeywordDest placeKeyword(const ParamNames& param_names, std::vector<bool>& params_filled,
+                                const std::string& kw_name, Box* kw_val, Box*& oarg1, Box*& oarg2, Box*& oarg3,
+                                Box** oargs, BoxedDict* okwargs, CLFunction* cl) {
     assert(kw_val);
 
-    bool found = false;
     for (int j = 0; j < param_names.args.size(); j++) {
         if (param_names.args[j].str() == kw_name && kw_name.size() > 0) {
             if (params_filled[j]) {
@@ -2328,23 +2331,21 @@ static void placeKeyword(const ParamNames& param_names, std::vector<bool>& param
             getArg(j, oarg1, oarg2, oarg3, oargs) = kw_val;
             params_filled[j] = true;
 
-            found = true;
-            break;
+            return KeywordDest::POSITIONAL;
         }
     }
 
-    if (!found) {
-        if (okwargs) {
-            Box*& v = okwargs->d[boxString(kw_name)];
-            if (v) {
-                raiseExcHelper(TypeError, "%.200s() got multiple values for keyword argument '%s'",
-                               getFunctionName(cl).c_str(), kw_name.c_str());
-            }
-            v = kw_val;
-        } else {
-            raiseExcHelper(TypeError, "%.200s() got an unexpected keyword argument '%s'", getFunctionName(cl).c_str(),
-                           kw_name.c_str());
+    if (okwargs) {
+        Box*& v = okwargs->d[boxString(kw_name)];
+        if (v) {
+            raiseExcHelper(TypeError, "%.200s() got multiple values for keyword argument '%s'",
+                           getFunctionName(cl).c_str(), kw_name.c_str());
         }
+        v = kw_val;
+        return KeywordDest::KWARGS;
+    } else {
+        raiseExcHelper(TypeError, "%.200s() got an unexpected keyword argument '%s'", getFunctionName(cl).c_str(),
+                       kw_name.c_str());
     }
 }
 
@@ -2370,7 +2371,7 @@ Box* callFunc(BoxedFunctionBase* func, CallRewriteArgs* rewrite_args, ArgPassSpe
 
     BoxedClosure* closure = func->closure;
 
-    if (argspec.has_starargs || argspec.has_kwargs || f->takes_kwargs || func->isGenerator) {
+    if (argspec.has_starargs || argspec.has_kwargs || func->isGenerator) {
         rewrite_args = NULL;
         REWRITE_ABORTED("");
     }
@@ -2502,9 +2503,23 @@ Box* callFunc(BoxedFunctionBase* func, CallRewriteArgs* rewrite_args, ArgPassSpe
 
     BoxedDict* okwargs = NULL;
     if (f->takes_kwargs) {
-        assert(!rewrite_args && "would need to be handled here");
+        int kwargs_idx = f->num_args + (f->takes_varargs ? 1 : 0);
+        if (rewrite_args) {
+            assert(!unused_positional.size());
+            RewriterVar* r_kwargs = rewrite_args->rewriter->call(false, (void*)createDict);
+
+            if (kwargs_idx == 0)
+                rewrite_args->arg1 = r_kwargs;
+            if (kwargs_idx == 1)
+                rewrite_args->arg2 = r_kwargs;
+            if (kwargs_idx == 2)
+                rewrite_args->arg3 = r_kwargs;
+            if (kwargs_idx >= 3)
+                rewrite_args->args->setAttr((kwargs_idx - 3) * sizeof(Box*), r_kwargs);
+        }
+
         okwargs = new BoxedDict();
-        getArg(f->num_args + (f->takes_varargs ? 1 : 0), oarg1, oarg2, oarg3, oargs) = okwargs;
+        getArg(kwargs_idx, oarg1, oarg2, oarg3, oargs) = okwargs;
     }
 
     const ParamNames& param_names = f->param_names;
@@ -2523,11 +2538,15 @@ Box* callFunc(BoxedFunctionBase* func, CallRewriteArgs* rewrite_args, ArgPassSpe
 
         if (!param_names.takes_param_names) {
             assert(okwargs);
+            rewrite_args = NULL; // would need to add it to r_kwargs
             okwargs->d[boxStringPtr((*keyword_names)[i])] = kw_val;
             continue;
         }
 
-        placeKeyword(param_names, params_filled, *(*keyword_names)[i], kw_val, oarg1, oarg2, oarg3, oargs, okwargs, f);
+        auto dest = placeKeyword(param_names, params_filled, *(*keyword_names)[i], kw_val, oarg1, oarg2, oarg3, oargs,
+                                 okwargs, f);
+        if (dest == KeywordDest::KWARGS)
+            rewrite_args = NULL;
     }
 
     if (argspec.has_kwargs) {
@@ -2545,8 +2564,10 @@ Box* callFunc(BoxedFunctionBase* func, CallRewriteArgs* rewrite_args, ArgPassSpe
             BoxedString* s = static_cast<BoxedString*>(p.first);
 
             if (param_names.takes_param_names) {
+                assert(!rewrite_args && "would need to make sure that this didn't need to go into r_kwargs");
                 placeKeyword(param_names, params_filled, s->s, p.second, oarg1, oarg2, oarg3, oargs, okwargs, f);
             } else {
+                assert(!rewrite_args && "would need to make sure that this didn't need to go into r_kwargs");
                 assert(okwargs);
 
                 Box*& v = okwargs->d[p.first];
@@ -3694,29 +3715,42 @@ Box* typeCallInternal(BoxedFunctionBase* f, CallRewriteArgs* rewrite_args, ArgPa
     }
 
     if (rewrite_args) {
-        CallRewriteArgs srewrite_args(rewrite_args->rewriter, r_new, rewrite_args->destination);
-        int new_npassed_args = new_argspec.totalPassed();
-
-        if (new_npassed_args >= 1)
-            srewrite_args.arg1 = r_ccls;
-        if (new_npassed_args >= 2)
-            srewrite_args.arg2 = rewrite_args->arg2;
-        if (new_npassed_args >= 3)
-            srewrite_args.arg3 = rewrite_args->arg3;
-        if (new_npassed_args >= 4)
-            srewrite_args.args = rewrite_args->args;
-        srewrite_args.args_guarded = true;
-        srewrite_args.func_guarded = true;
-
-        made = runtimeCallInternal(new_attr, &srewrite_args, new_argspec, cls, arg2, arg3, args, keyword_names);
-        ASSERT(made->cls == cls, "We should only have allowed the rewrite to continue if we were guaranteed that made "
-                                 "would have class cls!");
-
-        if (!srewrite_args.out_success) {
-            rewrite_args = NULL;
+        if (new_attr == object_new && init_attr != object_init) {
+            // Fast case: if we are calling object_new, we normally doesn't look at the arguments at all.
+            // (Except in the case when init_attr != object_init, in which case object_new looks at the number
+            // of arguments and throws an exception.)
+            //
+            // Another option is to rely on rewriting to make this fast, which would probably require adding
+            // a custom internal callable to object.__new__
+            made = objectNewNoArgs(cls);
+            r_made = rewrite_args->rewriter->call(false, (void*)objectNewNoArgs, r_ccls);
         } else {
-            r_made = srewrite_args.out_rtn;
+            CallRewriteArgs srewrite_args(rewrite_args->rewriter, r_new, rewrite_args->destination);
+            srewrite_args.args_guarded = true;
+            srewrite_args.func_guarded = true;
+
+            int new_npassed_args = new_argspec.totalPassed();
+
+            if (new_npassed_args >= 1)
+                srewrite_args.arg1 = r_ccls;
+            if (new_npassed_args >= 2)
+                srewrite_args.arg2 = rewrite_args->arg2;
+            if (new_npassed_args >= 3)
+                srewrite_args.arg3 = rewrite_args->arg3;
+            if (new_npassed_args >= 4)
+                srewrite_args.args = rewrite_args->args;
+
+            made = runtimeCallInternal(new_attr, &srewrite_args, new_argspec, cls, arg2, arg3, args, keyword_names);
+
+            if (!srewrite_args.out_success) {
+                rewrite_args = NULL;
+            } else {
+                r_made = srewrite_args.out_rtn;
+            }
         }
+
+        ASSERT(made->cls == cls, "We should only have allowed the rewrite to continue if we were guaranteed that made "
+                                 "would have class cls!");
     } else {
         made = runtimeCallInternal(new_attr, NULL, new_argspec, cls, arg2, arg3, args, keyword_names);
     }

src/runtime/types.cpp

@@ -945,13 +945,25 @@ Box* makeAttrWrapper(Box* b) {
     return new AttrWrapper(b);
 }
 
-Box* objectNew(BoxedClass* cls, BoxedTuple* args) {
+Box* objectNewNoArgs(BoxedClass* cls) {
+    assert(isSubclass(cls->cls, type_cls));
+    assert(typeLookup(cls, "__new__", NULL) == typeLookup(object_cls, "__new__", NULL)
+           && typeLookup(cls, "__init__", NULL) != typeLookup(object_cls, "__init__", NULL));
+    return new (cls) Box();
+}
+
+Box* objectNew(BoxedClass* cls, BoxedTuple* args, BoxedDict* kwargs) {
     assert(isSubclass(cls->cls, type_cls));
     assert(args->cls == tuple_cls);
+    assert(kwargs->cls == dict_cls);
 
-    if (args->elts.size() != 0) {
-        // TODO slow
-        if (typeLookup(cls, "__init__", NULL) == typeLookup(object_cls, "__init__", NULL))
+    // We use a different strategy from CPython: we let object.__new__ take extra
+    // arguments, but raise an error if they wouldn't be handled by the corresponding init.
+    // TODO switch to the CPython approach?
+    if (args->elts.size() != 0 || kwargs->d.size() != 0) {
+        // TODO slow (We already cache these in typeCall -- should use that here too?)
+        if (typeLookup(cls, "__new__", NULL) != typeLookup(object_cls, "__new__", NULL)
+            || typeLookup(cls, "__init__", NULL) == typeLookup(object_cls, "__init__", NULL))
             raiseExcHelper(TypeError, objectNewParameterTypeErrorMsg());
     }
 
@@ -1161,7 +1173,7 @@ void setupRuntime() {
     LONG = typeFromClass(long_cls);
     BOXED_COMPLEX = typeFromClass(complex_cls);
 
-    object_cls->giveAttr("__new__", new BoxedFunction(boxRTFunction((void*)objectNew, UNKNOWN, 1, 0, true, false)));
+    object_cls->giveAttr("__new__", new BoxedFunction(boxRTFunction((void*)objectNew, UNKNOWN, 1, 0, true, true)));
     object_cls->giveAttr("__init__", new BoxedFunction(boxRTFunction((void*)objectInit, UNKNOWN, 1, 0, true, false)));
     object_cls->giveAttr("__repr__", new BoxedFunction(boxRTFunction((void*)objectRepr, UNKNOWN, 1, 0, false, false)));
     object_cls->giveAttr("__str__", new BoxedFunction(boxRTFunction((void*)objectStr, UNKNOWN, 1, 0, false, false)));

src/runtime/types.h

@@ -606,6 +606,7 @@ extern "C" void boxGCHandler(GCVisitor* v, Box* b);
 Box* exceptionNew1(BoxedClass* cls);
 Box* exceptionNew2(BoxedClass* cls, Box* message);
 Box* exceptionNew(BoxedClass* cls, BoxedTuple* args);
+Box* objectNewNoArgs(BoxedClass* cls);
 
 extern "C" BoxedClass* Exception, *AssertionError, *AttributeError, *TypeError, *NameError, *KeyError, *IndexError,
     *IOError, *OSError, *ZeroDivisionError, *ValueError, *UnboundLocalError, *RuntimeError, *ImportError,

test/tests/constructor_args.py

+class C(object):
+    def __init__(self, a):
+        print a
+
+C(1)
+C(a=2)
+C(*(3,))
+C(**{'a':4})

test/tests/kwarg_ics.py

+# run_args: -n
+# statcheck: noninit_count('slowpath_runtimecall') < 20
+
+# A test to make sure we can rewrite certain kwargs cases
+
+# For now, just support rewriting if the kw is empty:
+def f(n, **kw):
+    print len(kw)
+    if n == 1000:
+        kw[n] = 1
+
+for i in xrange(20000):
+    f(i)

test/tests/object_new_arguments.py

@@ -12,6 +12,7 @@ class C2(object):
 
 print "Trying C1"
 object.__new__(C1, 1)
+object.__new__(C1, a=1)
 
 print "Trying C2"
 object.__new__(C2, 1)