Skip to content

Commit 119ccf3

Browse files
committed
gh-135763: Argument Clinic: Impl allow_negative for ssize_t
1 parent 95d6e0b commit 119ccf3

File tree

7 files changed

+94
-16
lines changed

7 files changed

+94
-16
lines changed

Include/internal/pycore_abstract.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ extern int _PyObject_RealIsSubclass(PyObject *derived, PyObject *cls);
5151
// Export for '_bisect' shared extension.
5252
PyAPI_FUNC(int) _Py_convert_optional_to_ssize_t(PyObject *, void *);
5353

54+
// Convert Python int to Py_ssize_t. Do nothing if the argument is None.
55+
// Raises ValueError if argument is negative
56+
PyAPI_FUNC(int) _Py_convert_optional_to_non_negative_ssize_t(PyObject *, void *);
57+
5458
// Same as PyNumber_Index() but can return an instance of a subclass of int.
5559
// Export for 'math' shared extension.
5660
PyAPI_FUNC(PyObject*) _PyNumber_Index(PyObject *o);

Lib/test/clinic.test.c

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1612,12 +1612,14 @@ test_Py_ssize_t_converter
16121612
a: Py_ssize_t = 12
16131613
b: Py_ssize_t(accept={int}) = 34
16141614
c: Py_ssize_t(accept={int, NoneType}) = 56
1615+
d: Py_ssize_t(accept={int}, allow_negative=False) = 78
1616+
e: Py_ssize_t(accept={int, NoneType}, allow_negative=False) = 90
16151617
/
16161618
16171619
[clinic start generated code]*/
16181620

16191621
PyDoc_STRVAR(test_Py_ssize_t_converter__doc__,
1620-
"test_Py_ssize_t_converter($module, a=12, b=34, c=56, /)\n"
1622+
"test_Py_ssize_t_converter($module, a=12, b=34, c=56, d=78, e=90, /)\n"
16211623
"--\n"
16221624
"\n");
16231625

@@ -1626,7 +1628,7 @@ PyDoc_STRVAR(test_Py_ssize_t_converter__doc__,
16261628

16271629
static PyObject *
16281630
test_Py_ssize_t_converter_impl(PyObject *module, Py_ssize_t a, Py_ssize_t b,
1629-
Py_ssize_t c);
1631+
Py_ssize_t c, Py_ssize_t d, Py_ssize_t e);
16301632

16311633
static PyObject *
16321634
test_Py_ssize_t_converter(PyObject *module, PyObject *const *args, Py_ssize_t nargs)
@@ -1635,8 +1637,10 @@ test_Py_ssize_t_converter(PyObject *module, PyObject *const *args, Py_ssize_t na
16351637
Py_ssize_t a = 12;
16361638
Py_ssize_t b = 34;
16371639
Py_ssize_t c = 56;
1640+
Py_ssize_t d = 78;
1641+
Py_ssize_t e = 90;
16381642

1639-
if (!_PyArg_CheckPositional("test_Py_ssize_t_converter", nargs, 0, 3)) {
1643+
if (!_PyArg_CheckPositional("test_Py_ssize_t_converter", nargs, 0, 5)) {
16401644
goto exit;
16411645
}
16421646
if (nargs < 1) {
@@ -1675,17 +1679,43 @@ test_Py_ssize_t_converter(PyObject *module, PyObject *const *args, Py_ssize_t na
16751679
if (!_Py_convert_optional_to_ssize_t(args[2], &c)) {
16761680
goto exit;
16771681
}
1682+
if (nargs < 4) {
1683+
goto skip_optional;
1684+
}
1685+
{
1686+
Py_ssize_t ival = -1;
1687+
PyObject *iobj = _PyNumber_Index(args[3]);
1688+
if (iobj != NULL) {
1689+
ival = PyLong_AsSsize_t(iobj);
1690+
Py_DECREF(iobj);
1691+
}
1692+
if (ival == -1 && PyErr_Occurred()) {
1693+
goto exit;
1694+
}
1695+
d = ival;
1696+
if (d < 0) {
1697+
PyErr_SetString(PyExc_ValueError,
1698+
"d must not be negative");
1699+
goto exit;
1700+
}
1701+
}
1702+
if (nargs < 5) {
1703+
goto skip_optional;
1704+
}
1705+
if (!_Py_convert_optional_to_non_negative_ssize_t(args[4], &e)) {
1706+
goto exit;
1707+
}
16781708
skip_optional:
1679-
return_value = test_Py_ssize_t_converter_impl(module, a, b, c);
1709+
return_value = test_Py_ssize_t_converter_impl(module, a, b, c, d, e);
16801710

16811711
exit:
16821712
return return_value;
16831713
}
16841714

16851715
static PyObject *
16861716
test_Py_ssize_t_converter_impl(PyObject *module, Py_ssize_t a, Py_ssize_t b,
1687-
Py_ssize_t c)
1688-
/*[clinic end generated code: output=48214bc3d01f4dd7 input=3855f184bb3f299d]*/
1717+
Py_ssize_t c, Py_ssize_t d, Py_ssize_t e)
1718+
/*[clinic end generated code: output=df3873c05e53e497 input=5c693ea198fa3dd5]*/
16891719

16901720

16911721
/*[clinic input]

Lib/test/test_clinic.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3194,8 +3194,12 @@ def test_py_ssize_t_converter(self):
31943194
ac_tester.py_ssize_t_converter(PY_SSIZE_T_MAX + 1)
31953195
with self.assertRaises(TypeError):
31963196
ac_tester.py_ssize_t_converter([])
3197-
self.assertEqual(ac_tester.py_ssize_t_converter(), (12, 34, 56))
3198-
self.assertEqual(ac_tester.py_ssize_t_converter(1, 2, None), (1, 2, 56))
3197+
with self.assertRaises(ValueError):
3198+
ac_tester.py_ssize_t_converter(12, 34, 56, -1)
3199+
with self.assertRaises(ValueError):
3200+
ac_tester.py_ssize_t_converter(12, 34, 56, 78, -1)
3201+
self.assertEqual(ac_tester.py_ssize_t_converter(), (12, 34, 56, 78, 90))
3202+
self.assertEqual(ac_tester.py_ssize_t_converter(1, 2, None, 3, None), (1, 2, 56, 3, 90))
31993203

32003204
def test_slice_index_converter(self):
32013205
from _testcapi import PY_SSIZE_T_MIN, PY_SSIZE_T_MAX
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Add ``allow_negative`` flag to ``Py_ssize_t`` Argument Clinic converter, to
2+
enable generation of bound checking

Modules/_testclinic.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -443,16 +443,18 @@ py_ssize_t_converter
443443
a: Py_ssize_t = 12
444444
b: Py_ssize_t(accept={int}) = 34
445445
c: Py_ssize_t(accept={int, NoneType}) = 56
446+
d: Py_ssize_t(accept={int}, allow_negative=False) = 78
447+
e: Py_ssize_t(accept={int, NoneType}, allow_negative=False) = 90
446448
/
447449
448450
[clinic start generated code]*/
449451

450452
static PyObject *
451453
py_ssize_t_converter_impl(PyObject *module, Py_ssize_t a, Py_ssize_t b,
452-
Py_ssize_t c)
453-
/*[clinic end generated code: output=ce252143e0ed0372 input=76d0f342e9317a1f]*/
454+
Py_ssize_t c, Py_ssize_t d, Py_ssize_t e)
455+
/*[clinic end generated code: output=32c03be1ad1b7c97 input=121f2d9ed09ae210]*/
454456
{
455-
RETURN_PACKED_ARGS(3, PyLong_FromSsize_t, Py_ssize_t, a, b, c);
457+
RETURN_PACKED_ARGS(5, PyLong_FromSsize_t, Py_ssize_t, a, b, c, d, e);
456458
}
457459

458460

Python/modsupport.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,23 @@ _Py_convert_optional_to_ssize_t(PyObject *obj, void *result)
3333
return 1;
3434
}
3535

36+
int
37+
_Py_convert_optional_to_non_negative_ssize_t(PyObject *obj, void *result)
38+
{
39+
if (_Py_convert_optional_to_ssize_t(obj, result)) {
40+
if(obj == Py_None || *((Py_ssize_t *)result) >= 0) {
41+
return 1;
42+
}
43+
else {
44+
PyErr_SetString(PyExc_ValueError,
45+
"argument must not be negative");
46+
return 0;
47+
}
48+
}
49+
else {
50+
return 0;
51+
}
52+
}
3653

3754
/* Helper for mkvalue() to scan the length of a format */
3855

Tools/clinic/libclinic/converters.py

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -420,21 +420,38 @@ class Py_ssize_t_converter(CConverter):
420420
type = 'Py_ssize_t'
421421
c_ignored_default = "0"
422422

423-
def converter_init(self, *, accept: TypeSet = {int}) -> None:
423+
def converter_init(self, *, accept: TypeSet = {int}, allow_negative: bool = True) -> None:
424+
self.allow_negative = allow_negative
424425
if accept == {int}:
425426
self.format_unit = 'n'
426427
self.default_type = int
427428
elif accept == {int, NoneType}:
428-
self.converter = '_Py_convert_optional_to_ssize_t'
429+
if self.allow_negative:
430+
self.converter = '_Py_convert_optional_to_ssize_t'
431+
else:
432+
self.converter = '_Py_convert_optional_to_non_negative_ssize_t'
429433
else:
430434
fail(f"Py_ssize_t_converter: illegal 'accept' argument {accept!r}")
431435

432436
def use_converter(self) -> None:
433437
if self.converter == '_Py_convert_optional_to_ssize_t':
434438
self.add_include('pycore_abstract.h',
435439
'_Py_convert_optional_to_ssize_t()')
440+
elif self.converter == '_Py_convert_optional_to_non_negative_ssize_t':
441+
self.add_include('pycore_abstract.h',
442+
'_Py_convert_optional_to_non_negative_ssize_t()')
436443

437444
def parse_arg(self, argname: str, displayname: str, *, limited_capi: bool) -> str | None:
445+
if self.allow_negative:
446+
non_negative_check = ""
447+
else:
448+
non_negative_check = self.format_code("""
449+
if ({paramname} < 0) {{{{
450+
PyErr_SetString(PyExc_ValueError,
451+
"{paramname} must not be negative");
452+
goto exit;
453+
}}}}""",
454+
argname=argname)
438455
if self.format_unit == 'n':
439456
if limited_capi:
440457
PyNumber_Index = 'PyNumber_Index'
@@ -452,11 +469,12 @@ def parse_arg(self, argname: str, displayname: str, *, limited_capi: bool) -> st
452469
if (ival == -1 && PyErr_Occurred()) {{{{
453470
goto exit;
454471
}}}}
455-
{paramname} = ival;
472+
{paramname} = ival;{non_negative_check}
456473
}}}}
457474
""",
458475
argname=argname,
459-
PyNumber_Index=PyNumber_Index)
476+
PyNumber_Index=PyNumber_Index,
477+
non_negative_check=non_negative_check)
460478
if not limited_capi:
461479
return super().parse_arg(argname, displayname, limited_capi=limited_capi)
462480
return self.format_code("""
@@ -465,7 +483,7 @@ def parse_arg(self, argname: str, displayname: str, *, limited_capi: bool) -> st
465483
{paramname} = PyNumber_AsSsize_t({argname}, PyExc_OverflowError);
466484
if ({paramname} == -1 && PyErr_Occurred()) {{{{
467485
goto exit;
468-
}}}}
486+
}}}}{non_negative_check}
469487
}}}}
470488
else {{{{
471489
{bad_argument}
@@ -475,6 +493,7 @@ def parse_arg(self, argname: str, displayname: str, *, limited_capi: bool) -> st
475493
""",
476494
argname=argname,
477495
bad_argument=self.bad_argument(displayname, 'integer or None', limited_capi=limited_capi),
496+
non_negative_check=non_negative_check
478497
)
479498

480499

0 commit comments

Comments
 (0)