Skip to content

Commit a317020

Browse files
wiomocwiomoc
committed
Apply suggestions from review
1 parent 31863c4 commit a317020

File tree

8 files changed

+105
-37
lines changed

8 files changed

+105
-37
lines changed

‎Include/internal/pycore_abstract.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ extern int _PyObject_RealIsSubclass(PyObject *derived, PyObject *cls);
5252
PyAPI_FUNC(int) _Py_convert_optional_to_ssize_t(PyObject *, void *);
5353

5454
// Convert Python int to Py_ssize_t. Do nothing if the argument is None.
55-
// Raises ValueError if argument is negative
55+
// Raises ValueError if argument is negative.
5656
PyAPI_FUNC(int) _Py_convert_optional_to_non_negative_ssize_t(PyObject *, void *);
5757

5858
// Same as PyNumber_Index() but can return an instance of a subclass of int.

‎Lib/test/clinic.test.c‎

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1614,12 +1614,15 @@ test_Py_ssize_t_converter
16141614
c: Py_ssize_t(accept={int, NoneType}) = 56
16151615
d: Py_ssize_t(accept={int}, allow_negative=False) = 78
16161616
e: Py_ssize_t(accept={int, NoneType}, allow_negative=False) = 90
1617+
f: Py_ssize_t(accept={int}, allow_negative=True) = 12
1618+
g: Py_ssize_t(accept={int, NoneType}, allow_negative=True) = 34
16171619
/
16181620
16191621
[clinic start generated code]*/
16201622

16211623
PyDoc_STRVAR(test_Py_ssize_t_converter__doc__,
1622-
"test_Py_ssize_t_converter($module, a=12, b=34, c=56, d=78, e=90, /)\n"
1624+
"test_Py_ssize_t_converter($module, a=12, b=34, c=56, d=78, e=90, f=12,\n"
1625+
" g=34, /)\n"
16231626
"--\n"
16241627
"\n");
16251628

@@ -1628,7 +1631,8 @@ PyDoc_STRVAR(test_Py_ssize_t_converter__doc__,
16281631

16291632
static PyObject *
16301633
test_Py_ssize_t_converter_impl(PyObject *module, Py_ssize_t a, Py_ssize_t b,
1631-
Py_ssize_t c, Py_ssize_t d, Py_ssize_t e);
1634+
Py_ssize_t c, Py_ssize_t d, Py_ssize_t e,
1635+
Py_ssize_t f, Py_ssize_t g);
16321636

16331637
static PyObject *
16341638
test_Py_ssize_t_converter(PyObject *module, PyObject *const *args, Py_ssize_t nargs)
@@ -1639,8 +1643,10 @@ test_Py_ssize_t_converter(PyObject *module, PyObject *const *args, Py_ssize_t na
16391643
Py_ssize_t c = 56;
16401644
Py_ssize_t d = 78;
16411645
Py_ssize_t e = 90;
1646+
Py_ssize_t f = 12;
1647+
Py_ssize_t g = 34;
16421648

1643-
if (!_PyArg_CheckPositional("test_Py_ssize_t_converter", nargs, 0, 5)) {
1649+
if (!_PyArg_CheckPositional("test_Py_ssize_t_converter", nargs, 0, 7)) {
16441650
goto exit;
16451651
}
16461652
if (nargs < 1) {
@@ -1695,7 +1701,7 @@ test_Py_ssize_t_converter(PyObject *module, PyObject *const *args, Py_ssize_t na
16951701
d = ival;
16961702
if (d < 0) {
16971703
PyErr_SetString(PyExc_ValueError,
1698-
"d must not be negative");
1704+
"d must be >=0");
16991705
goto exit;
17001706
}
17011707
}
@@ -1705,17 +1711,39 @@ test_Py_ssize_t_converter(PyObject *module, PyObject *const *args, Py_ssize_t na
17051711
if (!_Py_convert_optional_to_non_negative_ssize_t(args[4], &e)) {
17061712
goto exit;
17071713
}
1714+
if (nargs < 6) {
1715+
goto skip_optional;
1716+
}
1717+
{
1718+
Py_ssize_t ival = -1;
1719+
PyObject *iobj = _PyNumber_Index(args[5]);
1720+
if (iobj != NULL) {
1721+
ival = PyLong_AsSsize_t(iobj);
1722+
Py_DECREF(iobj);
1723+
}
1724+
if (ival == -1 && PyErr_Occurred()) {
1725+
goto exit;
1726+
}
1727+
f = ival;
1728+
}
1729+
if (nargs < 7) {
1730+
goto skip_optional;
1731+
}
1732+
if (!_Py_convert_optional_to_ssize_t(args[6], &g)) {
1733+
goto exit;
1734+
}
17081735
skip_optional:
1709-
return_value = test_Py_ssize_t_converter_impl(module, a, b, c, d, e);
1736+
return_value = test_Py_ssize_t_converter_impl(module, a, b, c, d, e, f, g);
17101737

17111738
exit:
17121739
return return_value;
17131740
}
17141741

17151742
static PyObject *
17161743
test_Py_ssize_t_converter_impl(PyObject *module, Py_ssize_t a, Py_ssize_t b,
1717-
Py_ssize_t c, Py_ssize_t d, Py_ssize_t e)
1718-
/*[clinic end generated code: output=df3873c05e53e497 input=5c693ea198fa3dd5]*/
1744+
Py_ssize_t c, Py_ssize_t d, Py_ssize_t e,
1745+
Py_ssize_t f, Py_ssize_t g)
1746+
/*[clinic end generated code: output=8721e4925ea46578 input=0d80cb5c942b6e0f]*/
17191747

17201748

17211749
/*[clinic input]

‎Lib/test/test_clinic.py‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2610,6 +2610,19 @@ def test_disallow_defining_class_at_module_level(self):
26102610
"""
26112611
self.expect_failure(block, err, lineno=2)
26122612

2613+
def test_allow_negative_accepted_by_py_ssize_t_converter_only(self):
2614+
errmsg = re.escape("converter_init() got an unexpected keyword argument 'allow_negative'")
2615+
unsupported_converters = [converter_name for converter_name in converters.keys()
2616+
if converter_name != "Py_ssize_t"]
2617+
for converter in unsupported_converters:
2618+
with self.subTest(converter=converter):
2619+
block = f"""
2620+
module m
2621+
m.func
2622+
a: {converter}(allow_negative=True)
2623+
"""
2624+
with self.assertRaisesRegex((AssertionError, TypeError), errmsg):
2625+
self.parse_function(block)
26132626

26142627
class ClinicExternalTest(TestCase):
26152628
maxDiff = None
@@ -3198,8 +3211,8 @@ def test_py_ssize_t_converter(self):
31983211
ac_tester.py_ssize_t_converter(12, 34, 56, -1)
31993212
with self.assertRaises(ValueError):
32003213
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))
3214+
self.assertEqual(ac_tester.py_ssize_t_converter(), (12, 34, 56, 78, 90, -1, -1))
3215+
self.assertEqual(ac_tester.py_ssize_t_converter(1, 2, None, 3, None, 4, None), (1, 2, 56, 3, 90, 4, -1))
32033216

32043217
def test_slice_index_converter(self):
32053218
from _testcapi import PY_SSIZE_T_MIN, PY_SSIZE_T_MAX

Misc/NEWS.d/next/Library/2025-08-26-09-07-02.gh-issue-135763.DVpGUB.rst renamed to Misc/NEWS.d/next/Tools-Demos/2025-08-26-09-07-02.gh-issue-135763.DVpGUB.rst

File renamed without changes.

‎Modules/_testclinic.c‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -445,16 +445,19 @@ py_ssize_t_converter
445445
c: Py_ssize_t(accept={int, NoneType}) = 56
446446
d: Py_ssize_t(accept={int}, allow_negative=False) = 78
447447
e: Py_ssize_t(accept={int, NoneType}, allow_negative=False) = 90
448+
f: Py_ssize_t(accept={int}, allow_negative=False) = -1
449+
g: Py_ssize_t(accept={int, NoneType}, allow_negative=False) = -1
448450
/
449451
450452
[clinic start generated code]*/
451453

452454
static PyObject *
453455
py_ssize_t_converter_impl(PyObject *module, Py_ssize_t a, Py_ssize_t b,
454-
Py_ssize_t c, Py_ssize_t d, Py_ssize_t e)
455-
/*[clinic end generated code: output=32c03be1ad1b7c97 input=121f2d9ed09ae210]*/
456+
Py_ssize_t c, Py_ssize_t d, Py_ssize_t e,
457+
Py_ssize_t f, Py_ssize_t g)
458+
/*[clinic end generated code: output=ecf8e1a4a9abc95e input=d4571dd2ba885665]*/
456459
{
457-
RETURN_PACKED_ARGS(5, PyLong_FromSsize_t, Py_ssize_t, a, b, c, d, e);
460+
RETURN_PACKED_ARGS(7, PyLong_FromSsize_t, Py_ssize_t, a, b, c, d, e, f, g);
458461
}
459462

460463

‎Modules/clinic/_testclinic.c.h‎

Lines changed: 36 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/modsupport.c‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,14 @@ _Py_convert_optional_to_ssize_t(PyObject *obj, void *result)
3636
int
3737
_Py_convert_optional_to_non_negative_ssize_t(PyObject *obj, void *result)
3838
{
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 {
39+
if (!_Py_convert_optional_to_ssize_t(obj, result)) {
5040
return 0;
5141
}
42+
if (obj != Py_None && *((Py_ssize_t *)result) < 0) {
43+
PyErr_SetString(PyExc_ValueError, "argument must be >= 0");
44+
return 0;
45+
}
46+
return 1;
5247
}
5348

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

‎Tools/clinic/libclinic/converters.py‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -434,12 +434,11 @@ def converter_init(self, *, accept: TypeSet = {int}, allow_negative: bool = True
434434
fail(f"Py_ssize_t_converter: illegal 'accept' argument {accept!r}")
435435

436436
def use_converter(self) -> None:
437-
if self.converter == '_Py_convert_optional_to_ssize_t':
438-
self.add_include('pycore_abstract.h',
439-
'_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()')
437+
if self.converter in {
438+
'_Py_convert_optional_to_ssize_t',
439+
'_Py_convert_optional_to_non_negative_ssize_t',
440+
}:
441+
self.add_include('pycore_abstract.h', f'{self.converter}()')
443442

444443
def parse_arg(self, argname: str, displayname: str, *, limited_capi: bool) -> str | None:
445444
if self.allow_negative:
@@ -448,7 +447,7 @@ def parse_arg(self, argname: str, displayname: str, *, limited_capi: bool) -> st
448447
non_negative_check = self.format_code("""
449448
if ({paramname} < 0) {{{{
450449
PyErr_SetString(PyExc_ValueError,
451-
"{paramname} must not be negative");
450+
"{paramname} must be >=0");
452451
goto exit;
453452
}}}}""",
454453
argname=argname)

0 commit comments

Comments
 (0)