Commit ca25dfd2 authored by Josh Tobin's avatar Josh Tobin

posonly_optimization: address review feedback

parent aae70a1c
......@@ -3692,8 +3692,6 @@ class DefNodeWrapper(FuncDefNode):
code.putln('{')
all_args = tuple(positional_args) + tuple(kw_only_args)
non_posonly_args = [arg for arg in all_args if not arg.pos_only]
do_generate_kw_unpacking = bool(non_posonly_args) or self.starstar_arg
if do_generate_kw_unpacking:
code.putln("static PyObject **%s[] = {%s};" % (
Naming.pykwdlist_cname,
','.join(['&%s' % code.intern_identifier(arg.name)
......@@ -3709,18 +3707,41 @@ class DefNodeWrapper(FuncDefNode):
# was passed for them.
self.generate_argument_values_setup_code(all_args, code)
# If all args are positional-only, we can raise an error
# straight away if we receive a non-empty kw-dict.
# This requires a PyDict_Size call. This call is wasteful
# for functions which do accept kw-args, so we do not generate
# the PyDict_Size call unless all args are positional-only.
accept_kwd_args = non_posonly_args or self.starstar_arg
if accept_kwd_args:
kw_unpacking_condition = Naming.kwds_cname
else:
kw_unpacking_condition = "%s && PyDict_Size(%s) > 0" % (
Naming.kwds_cname, Naming.kwds_cname)
# --- optimised code when we receive keyword arguments
code.putln("if (%s(%s)) {" % (
(self.num_required_kw_args > 0) and "likely" or "unlikely",
Naming.kwds_cname))
kw_unpacking_condition))
if do_generate_kw_unpacking:
if accept_kwd_args:
self.generate_keyword_unpacking_code(
min_positional_args, max_positional_args,
has_fixed_positional_count, has_kw_only_args, all_args, argtuple_error_label, code)
else:
code.putln("PyErr_Format(PyExc_TypeError, \"%s() takes no keyword arguments\");" % self.name)
code.putln(code.error_goto(self.pos))
# Here we do not accept kw-args but we are passed a non-empty kw-dict.
# We call ParseOptionalKeywords which will raise an appropriate error if
# the kw-args dict passed is non-empty.
code.globalstate.use_utility_code(
UtilityCode.load_cached("ParseKeywords", "FunctionArguments.c"))
code.putln('if (unlikely(__Pyx_ParseOptionalKeywords(%s, %s, %s, %s, %s, "%s") < 0)) %s' % (
Naming.kwds_cname,
Naming.pykwdlist_cname,
self.starstar_arg and self.starstar_arg.entry.cname or '0',
'values',
0,
self.name,
code.error_goto(self.pos)))
# --- optimised code when we do not receive any keyword arguments
if (self.num_required_kw_args and min_positional_args > 0) or min_positional_args == max_positional_args:
......@@ -3899,25 +3920,27 @@ class DefNodeWrapper(FuncDefNode):
code.putln('switch (pos_args) {')
if self.star_arg:
code.putln('default:')
for i in range(max_positional_args-1, -1, -1):
for i in range(max_positional_args-1, num_required_posonly_args-1, -1):
code.put('case %2d: ' % (i+1))
if i+1 > num_required_posonly_args:
code.putln("values[%d] = PyTuple_GET_ITEM(%s, %d);" % (
i, Naming.args_cname, i))
code.putln('CYTHON_FALLTHROUGH;')
elif i+1 == num_required_posonly_args:
for j in range(num_required_posonly_args-1, -1, -1):
if num_required_posonly_args > 0:
code.put('case %2d: ' % num_required_posonly_args)
for i in range(num_required_posonly_args-1, -1, -1):
code.putln("values[%d] = PyTuple_GET_ITEM(%s, %d);" % (
j, Naming.args_cname, j))
i, Naming.args_cname, i))
code.putln('break;')
else:
for i in range(num_required_posonly_args-2, -1, -1):
code.put('case %2d: ' % (i+1))
code.putln('CYTHON_FALLTHROUGH;')
code.put('case 0: ')
if num_required_posonly_args == 0:
code.putln('case 0: break;')
code.putln('break;')
else:
# catch-all for not enough pos-only args passed
code.putln('case 0:')
code.put_goto(argtuple_error_label)
if not self.star_arg:
code.put('default: ') # more arguments than allowed
......@@ -3943,9 +3966,7 @@ class DefNodeWrapper(FuncDefNode):
last_required_arg = max_positional_args-1
if max_positional_args > num_posonly_args:
code.putln('switch (pos_args) {')
for i, arg in enumerate(all_args[:last_required_arg+1]):
if i < num_posonly_args:
continue
for i, arg in enumerate(all_args[num_posonly_args:last_required_arg+1], num_posonly_args):
if max_positional_args > num_posonly_args and i <= max_positional_args:
if i != num_posonly_args:
code.putln('CYTHON_FALLTHROUGH;')
......
......@@ -2,8 +2,6 @@
# mode: run
# tag: posonly
# TODO: remove posonly tag before merge
import cython
import sys
import pickle
......@@ -126,7 +124,7 @@ def test_use_positional_as_keyword3(a, b, /):
>>> test_use_positional_as_keyword3(1, 2)
>>> test_use_positional_as_keyword3(a=1, b=2)
Traceback (most recent call last):
TypeError: test_use_positional_as_keyword3() takes no keyword arguments
TypeError: test_use_positional_as_keyword3() got an unexpected keyword argument 'a'
"""
def test_positional_only_and_arg_invalid_calls(a, b, /, c):
......@@ -266,7 +264,7 @@ class TestPosonlyMethods(object):
Got type error
>>> TestPosonlyMethods().f(1, b=2)
Traceback (most recent call last):
TypeError: f() takes no keyword arguments
TypeError: f() got an unexpected keyword argument 'b'
"""
def f(self, a, b, /):
return a, b
......@@ -367,7 +365,7 @@ def test_serialization1(a, b, /):
(1, 2)
>>> unpickled_posonly(a=1, b=2)
Traceback (most recent call last):
TypeError: test_serialization1() takes no keyword arguments
TypeError: test_serialization1() got an unexpected keyword argument 'a'
"""
return (a, b)
......@@ -510,7 +508,8 @@ def f_call_posonly_kwarg(a,/,**kw):
"""
>>> f_call_posonly_kwarg(1)
(1, {})
>>> f_call_posonly_kwarg(1, b=2, c=3, d=4)==(1, {'b': 2, 'c': 3, 'd': 4})
>>> all_args = f_call_posonly_kwarg(1, b=2, c=3, d=4)
>>> all_args == (1, {'b': 2, 'c': 3, 'd': 4}) or all_args
True
"""
return (a,kw)
......@@ -521,9 +520,23 @@ def f_call_posonly_stararg_kwarg(a,/,*args,**kw):
(1, (), {})
>>> f_call_posonly_stararg_kwarg(1, 2)
(1, (2,), {})
>>> f_call_posonly_stararg_kwarg(1, b=3, c=4)==(1, (), {'b': 3, 'c': 4})
>>> all_args = f_call_posonly_stararg_kwarg(1, b=3, c=4)
>>> all_args == (1, (), {'b': 3, 'c': 4}) or all_args
True
>>> f_call_posonly_stararg_kwarg(1, 2, b=3, c=4)==(1, (2,), {'b': 3, 'c': 4})
>>> all_args = f_call_posonly_stararg_kwarg(1, 2, b=3, c=4)
>>> all_args == (1, (2,), {'b': 3, 'c': 4}) or all_args
True
"""
return (a,args,kw)
def test_empty_kwargs(a, b, /):
"""
>>> test_empty_kwargs(1, 2)
(1, 2)
>>> test_empty_kwargs(1, 2, **{})
(1, 2)
>>> test_empty_kwargs(1, 2, **{'c': 3})
Traceback (most recent call last):
TypeError: test_empty_kwargs() got an unexpected keyword argument 'c'
"""
return (a,b)
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment