Skip to content

Commit 065a910

Browse files
[3.14] gh-64502: Fix Argument Clinic support of optional groups with defaults (GH-155191) (GH-155216)
Parameters with a default value which are not in any group were always required in the generated argument parsing code, although they were rendered as optional in the signature. They can now be omitted, and ambiguous combinations of optional groups and parameters with a default value are rejected. (cherry picked from commit caac927) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 8ece4da commit 065a910

7 files changed

Lines changed: 279 additions & 12 deletions

File tree

Lib/test/clinic.test.c

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5641,6 +5641,67 @@ Test___init___impl(TestObj *self, PyObject *a, int group_right_1,
56415641
/*[clinic end generated code: output=2bbb8ea60e8f57a6 input=10f5d0f1e8e466ef]*/
56425642

56435643

5644+
/*[clinic input]
5645+
group_and_optional_parameter
5646+
[
5647+
a: object
5648+
b: object
5649+
]
5650+
c: object = None
5651+
/
5652+
The optional parameter can be omitted with or without the group.
5653+
[clinic start generated code]*/
5654+
5655+
PyDoc_STRVAR(group_and_optional_parameter__doc__,
5656+
"group_and_optional_parameter([a, b,] c=None)\n"
5657+
"The optional parameter can be omitted with or without the group.");
5658+
5659+
#define GROUP_AND_OPTIONAL_PARAMETER_METHODDEF \
5660+
{"group_and_optional_parameter", (PyCFunction)group_and_optional_parameter, METH_VARARGS, group_and_optional_parameter__doc__},
5661+
5662+
static PyObject *
5663+
group_and_optional_parameter_impl(PyObject *module, int group_left_1,
5664+
PyObject *a, PyObject *b, PyObject *c);
5665+
5666+
static PyObject *
5667+
group_and_optional_parameter(PyObject *module, PyObject *args)
5668+
{
5669+
PyObject *return_value = NULL;
5670+
int group_left_1 = 0;
5671+
PyObject *a = NULL;
5672+
PyObject *b = NULL;
5673+
PyObject *c = Py_None;
5674+
5675+
switch (PyTuple_GET_SIZE(args)) {
5676+
case 0:
5677+
case 1:
5678+
if (!PyArg_ParseTuple(args, "|O:group_and_optional_parameter", &c)) {
5679+
goto exit;
5680+
}
5681+
break;
5682+
case 2:
5683+
case 3:
5684+
if (!PyArg_ParseTuple(args, "OO|O:group_and_optional_parameter", &a, &b, &c)) {
5685+
goto exit;
5686+
}
5687+
group_left_1 = 1;
5688+
break;
5689+
default:
5690+
PyErr_SetString(PyExc_TypeError, "group_and_optional_parameter requires 0 to 3 arguments");
5691+
goto exit;
5692+
}
5693+
return_value = group_and_optional_parameter_impl(module, group_left_1, a, b, c);
5694+
5695+
exit:
5696+
return return_value;
5697+
}
5698+
5699+
static PyObject *
5700+
group_and_optional_parameter_impl(PyObject *module, int group_left_1,
5701+
PyObject *a, PyObject *b, PyObject *c)
5702+
/*[clinic end generated code: output=3faea69eafd5bbbe input=7f0fbb6124f5a972]*/
5703+
5704+
56445705
/*[clinic input]
56455706
Test._pyarg_parsestackandkeywords
56465707
cls: defining_class

Lib/test/test_clinic.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,24 @@ def __init__(self):
330330
"""
331331
self.expect_failure(block, err, lineno=8)
332332

333+
def test_ambiguous_group_and_optional_parameters(self):
334+
err = ("Function 'my_test_func' has an ambiguous group configuration: "
335+
"a call with 2 argument(s) can be parsed in more than one way.")
336+
block = """
337+
/*[clinic input]
338+
my_test_func
339+
340+
[
341+
a: object
342+
b: object
343+
]
344+
c: object = None
345+
d: object = None
346+
/
347+
[clinic start generated code]*/
348+
"""
349+
self.expect_failure(block, err)
350+
333351
def test_star_after_vararg(self):
334352
err = "'my_test_func' uses '*' more than once."
335353
block = """
@@ -3828,6 +3846,27 @@ def test_varpos_kwonly_req_opt(self):
38283846
self.assertEqual(fn(1, a=2, b=3), ((1,), 2, 3, False))
38293847
self.assertEqual(fn(1, a=2, b=3, c=4), ((1,), 2, 3, 4))
38303848

3849+
def test_group_and_opt(self):
3850+
# fn([a, b,] c=None)
3851+
fn = ac_tester.group_and_opt
3852+
self.assertEqual(fn(), (False, None, None, None))
3853+
self.assertEqual(fn(1), (False, None, None, 1))
3854+
self.assertEqual(fn(1, 2), (True, 1, 2, None))
3855+
self.assertEqual(fn(1, 2, 3), (True, 1, 2, 3))
3856+
self.assertRaises(TypeError, fn, 1, 2, 3, 4)
3857+
self.assertRaises(TypeError, fn, c=1)
3858+
3859+
def test_group_and_two_opt(self):
3860+
# fn([a, b, c,] d=None, e=None)
3861+
fn = ac_tester.group_and_two_opt
3862+
self.assertEqual(fn(), (False, None, None, None, None, None))
3863+
self.assertEqual(fn(1), (False, None, None, None, 1, None))
3864+
self.assertEqual(fn(1, 2), (False, None, None, None, 1, 2))
3865+
self.assertEqual(fn(1, 2, 3), (True, 1, 2, 3, None, None))
3866+
self.assertEqual(fn(1, 2, 3, 4), (True, 1, 2, 3, 4, None))
3867+
self.assertEqual(fn(1, 2, 3, 4, 5), (True, 1, 2, 3, 4, 5))
3868+
self.assertRaises(TypeError, fn, 1, 2, 3, 4, 5, 6)
3869+
38313870
def test_gh_32092_oob(self):
38323871
ac_tester.gh_32092_oob(1, 2, 3, 4, kw1=5, kw2=6)
38333872

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix Argument Clinic support of parameters with a default value used together
2+
with optional groups.
3+
Such parameters were always required in the generated parsing code.

Modules/_testclinic.c

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1232,6 +1232,52 @@ posonly_poskw_varpos_array_impl(PyObject *module, PyObject *a, PyObject *b,
12321232
}
12331233

12341234

1235+
/*[clinic input]
1236+
group_and_opt
1237+
1238+
[
1239+
a: object
1240+
b: object
1241+
]
1242+
c: object = None
1243+
/
1244+
1245+
[clinic start generated code]*/
1246+
1247+
static PyObject *
1248+
group_and_opt_impl(PyObject *module, int group_left_1, PyObject *a,
1249+
PyObject *b, PyObject *c)
1250+
/*[clinic end generated code: output=23413ec545526111 input=8a84d8f44bc8bd0b]*/
1251+
{
1252+
return pack_arguments_newref(4, group_left_1 ? Py_True : Py_False,
1253+
a, b, c);
1254+
}
1255+
1256+
1257+
/*[clinic input]
1258+
group_and_two_opt
1259+
1260+
[
1261+
a: object
1262+
b: object
1263+
c: object
1264+
]
1265+
d: object = None
1266+
e: object = None
1267+
/
1268+
1269+
[clinic start generated code]*/
1270+
1271+
static PyObject *
1272+
group_and_two_opt_impl(PyObject *module, int group_left_1, PyObject *a,
1273+
PyObject *b, PyObject *c, PyObject *d, PyObject *e)
1274+
/*[clinic end generated code: output=1427c4b3c35f24ff input=cdda98eec1e365ea]*/
1275+
{
1276+
return pack_arguments_newref(6, group_left_1 ? Py_True : Py_False,
1277+
a, b, c, d, e);
1278+
}
1279+
1280+
12351281

12361282
/*[clinic input]
12371283
gh_32092_oob
@@ -2368,6 +2414,8 @@ static PyMethodDef tester_methods[] = {
23682414
POSONLY_VARPOS_ARRAY_METHODDEF
23692415
POSONLY_REQ_OPT_VARPOS_ARRAY_METHODDEF
23702416
POSONLY_POSKW_VARPOS_ARRAY_METHODDEF
2417+
GROUP_AND_OPT_METHODDEF
2418+
GROUP_AND_TWO_OPT_METHODDEF
23712419

23722420
GH_32092_OOB_METHODDEF
23732421
GH_32092_KW_PASS_METHODDEF

Modules/clinic/_testclinic.c.h

Lines changed: 91 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Tools/c-analyzer/cpython/_parser.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,7 @@ def format_tsv_lines(lines):
309309
_abs('Modules/posixmodule.c'): (20_000, 500),
310310
_abs('Modules/termios.c'): (10_000, 800),
311311
_abs('Modules/_testcapimodule.c'): (20_000, 400),
312+
_abs('Modules/_testclinic.c'): (20_000, 400),
312313
_abs('Modules/expat/expat.h'): (10_000, 400),
313314
_abs('Objects/stringlib/unicode_format.h'): (10_000, 400),
314315
_abs('Objects/typeobject.c'): (35_000, 200),
@@ -333,7 +334,7 @@ def format_tsv_lines(lines):
333334
_abs('Modules/_ssl_data_300.h'): (80_000, 10_000),
334335
_abs('Modules/_ssl_data_111.h'): (80_000, 10_000),
335336
_abs('Modules/cjkcodecs/mappings_*.h'): (160_000, 2_000),
336-
_abs('Modules/clinic/_testclinic.c.h'): (120_000, 5_000),
337+
_abs('Modules/clinic/_testclinic.c.h'): (125_000, 5_000),
337338
_abs('Modules/unicodedata_db.h'): (180_000, 3_000),
338339
_abs('Modules/unicodename_db.h'): (1_200_000, 15_000),
339340
_abs('Objects/unicodetype_db.h'): (240_000, 3_000),

Tools/clinic/libclinic/clanguage.py

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
from __future__ import annotations
22
import itertools
3-
import sys
43
import textwrap
54
from typing import TYPE_CHECKING, Literal, Final
65
from operator import attrgetter
@@ -12,7 +11,7 @@
1211
from libclinic.codegen import CRenderData, TemplateDict, CodeGen
1312
from libclinic.language import Language
1413
from libclinic.function import (
15-
Module, Class, Function, Parameter,
14+
Module, Class, Function, Parameter, ParamTuple,
1615
permute_optional_groups,
1716
GETTER, SETTER, METHOD_INIT)
1817
from libclinic.converters import self_converter
@@ -21,6 +20,20 @@
2120
from libclinic.app import Clinic
2221

2322

23+
def count_required(subset: ParamTuple) -> int:
24+
"""Return the number of arguments which cannot be omitted.
25+
26+
A parameter in an optional group is passed together with its group,
27+
so only trailing parameters with a default value can be omitted.
28+
"""
29+
count = len(subset)
30+
for p in reversed(subset):
31+
if p.group or not p.is_optional():
32+
break
33+
count -= 1
34+
return count
35+
36+
2437
def c_id(name: str) -> str:
2538
if len(name) == 1 and ord(name) < 256:
2639
if name.isalnum():
@@ -301,18 +314,26 @@ def render_option_group_parsing(
301314
assert group is not None
302315
group.append(p)
303316

304-
count_min = sys.maxsize
305-
count_max = -1
317+
# Map the number of arguments to the subset which accepts it.
318+
subsets: dict[int, ParamTuple] = {}
319+
for subset in permute_optional_groups(left, required, right):
320+
for count in range(count_required(subset), len(subset) + 1):
321+
if count in subsets:
322+
fail(f"Function {f.full_name!r} has an ambiguous group "
323+
f"configuration: a call with {count} argument(s) "
324+
f"can be parsed in more than one way.")
325+
subsets[count] = subset
306326

307327
if limited_capi:
308328
nargs = 'PyTuple_Size(args)'
309329
else:
310330
nargs = 'PyTuple_GET_SIZE(args)'
311331
out.append(f"switch ({nargs}) {{\n")
312-
for subset in permute_optional_groups(left, required, right):
313-
count = len(subset)
314-
count_min = min(count_min, count)
315-
count_max = max(count_max, count)
332+
for count, subset in sorted(subsets.items()):
333+
if count < len(subset):
334+
# The omitted parameters are parsed by the following case.
335+
out.append(f" case {count}:\n")
336+
continue
316337

317338
if count == 0:
318339
out.append(""" case 0:
@@ -326,7 +347,11 @@ def render_option_group_parsing(
326347
d: dict[str, str | int] = {}
327348
d['count'] = count
328349
d['name'] = f.name
329-
d['format_units'] = "".join(p.converter.format_unit for p in subset)
350+
format_units = [p.converter.format_unit for p in subset]
351+
n_required = count_required(subset)
352+
if n_required < count:
353+
format_units.insert(n_required, '|')
354+
d['format_units'] = "".join(format_units)
330355

331356
parse_arguments: list[str] = []
332357
for p in subset:
@@ -353,7 +378,7 @@ def render_option_group_parsing(
353378

354379
out.append(" default:\n")
355380
s = ' PyErr_SetString(PyExc_TypeError, "{} requires {} to {} arguments");\n'
356-
out.append(s.format(f.full_name, count_min, count_max))
381+
out.append(s.format(f.full_name, min(subsets), max(subsets)))
357382
out.append(' goto exit;\n')
358383
out.append("}")
359384

0 commit comments

Comments
 (0)