Refactor positional arguments handling

Summary: Refactor positional default argument filling in `JITRT_BindKeywordArgs`: rename variables `(n, m)` for clarity, simplify the missing-arg check to return early, and add tests covering the keyword call path.

Reviewed By: alexmalyshev

Differential Revision: D96811731

fbshipit-source-id: f5c971a5c5711d0c70f373847af60096b0106bcc

Author: yoney

cinderx/Jit/jit_rt.cpp

@@ -144,30 +144,26 @@ static int JITRT_BindKeywordArgs(
 
   // Add missing positional arguments (copy default values from defs)
   if (argcount < co->co_argcount) {
-    Py_ssize_t defcount;
-    if (func->func_defaults != nullptr) {
-      defcount = PyTuple_Size(func->func_defaults);
-    } else {
-      defcount = 0;
-    }
-    Py_ssize_t m = co->co_argcount - defcount;
-    Py_ssize_t missing = 0;
-    for (Py_ssize_t i = argcount; i < m; i++) {
+    PyObject* defaults = func->func_defaults;
+    Py_ssize_t defcount = defaults == nullptr ? 0 : PyTuple_Size(defaults);
+    Py_ssize_t first_default_arg = co->co_argcount - defcount;
+
+    // Any unset slot before the defaults region means we are missing
+    // a required positional argument.
+    for (Py_ssize_t i = argcount; i < first_default_arg; i++) {
       if (arg_space[i] == nullptr) {
-        missing++;
+        return 0;
       }
     }
-    if (missing) {
-      return 0;
-    }
 
-    if (defcount) {
-      PyObject* const* defs =
-          &((PyTupleObject*)func->func_defaults)->ob_item[0];
-      for (Py_ssize_t i = std::max<Py_ssize_t>(n - m, 0); i < defcount; i++) {
-        if (arg_space[m + i] == nullptr) {
-          PyObject* def = defs[i];
-          arg_space[m + i] = def;
+    if (defaults != nullptr) {
+      PyObject* const* defs = &((PyTupleObject*)defaults)->ob_item[0];
+      // Only slots in the defaults-covered tail can still need filling.
+      Py_ssize_t arg_index = std::max(argcount, first_default_arg);
+      for (; arg_index < co->co_argcount; arg_index++) {
+        if (arg_space[arg_index] == nullptr) {
+          Py_ssize_t def_index = arg_index - first_default_arg;
+          arg_space[arg_index] = defs[def_index];
         }
       }
     }

cinderx/PythonLib/test_cinderx/test_cinderjit.py

@@ -1458,6 +1458,26 @@ def test_kwonly_args_and_varkwargs(self) -> None:
         self.assertEqual(self.f4(2, y=10, z=30, a=40, b=50), {"a": 40, "b": 50})
 
 
+class PositionalDefaultsWithKeywordCallTest(unittest.TestCase):
+    """Test that positional args with defaults are filled correctly
+    when the function is called with keyword arguments."""
+
+    @cinder_support.failUnlessJITCompiled
+    def f(self, a, b=2, c=3):
+        return (a, b, c)
+
+    def test_skip_default_with_kwarg(self) -> None:
+        self.assertEqual(self.f(1, b=20), (1, 20, 3))
+        self.assertEqual(self.f(1, c=30), (1, 2, 30))
+
+    def test_override_defaults_with_kwargs(self) -> None:
+        self.assertEqual(self.f(1, b=20, c=30), (1, 20, 30))
+
+    def test_missing_required_arg_with_kwargs(self) -> None:
+        with self.assertRaises(TypeError):
+            self.f(b=20, c=30)
+
+
 class ClassA:
     z = 100
     x = 41