From 04cb2ecd5dddfa8c57b3f383d4f1781a47099406 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sat, 5 Nov 2022 15:09:59 +0300 Subject: [PATCH 1/3] gh-94808: cover `PySequence_{Set,Del}Slice` --- Lib/test/test_capi.py | 64 +++++++++++++++++++++++++++++++++++++++ Modules/_testcapimodule.c | 33 ++++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 49f207ed953c2d..1d9145f4db5c6f 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -416,6 +416,70 @@ class SubDict(dict): self.assertTrue(_testcapi.mapping_has_key(dct2, 'a')) self.assertFalse(_testcapi.mapping_has_key(dct2, 'b')) + def test_sequence_set_slice(self): + # Correct case: + data = [1, 2, 3, 4, 5] + data_copy = data.copy() + + _testcapi.sequence_set_slice(data, 1, 3, [8, 9]) + data_copy[1:3] = [8, 9] + self.assertEqual(data, data_copy) + self.assertEqual(data, [1, 8, 9, 4, 5]) + + # Custom class: + class Custom: + def __setitem__(self, index, value): + self.index = index + self.value = value + + c = Custom() + _testcapi.sequence_set_slice(c, 0, 5, 'abc') + self.assertEqual(c.index, slice(0, 5)) + self.assertEqual(c.value, 'abc') + + # Immutable sequences must raise: + with self.assertRaises(TypeError): + _testcapi.sequence_set_slice((1, 2, 3, 4), 1, 3, (8, 9)) + with self.assertRaises(TypeError): + _testcapi.sequence_set_slice('abcd', 1, 3, 'xy') + + # Not a sequnce: + with self.assertRaises(TypeError): + _testcapi.sequence_set_slice(object(), 1, 3, 'xy') + with self.assertRaises(TypeError): + _testcapi.sequence_set_slice({1: 'a', 2: 'b', 3: 'c'}, 1, 3, 'xy') + + def test_sequence_del_slice(self): + # Correct case: + data = [1, 2, 3, 4, 5] + data_copy = data.copy() + + _testcapi.sequence_del_slice(data, 1, 3) + del data_copy[1:3] + self.assertEqual(data, data_copy) + self.assertEqual(data, [1, 4, 5]) + + # Custom class: + class Custom: + def __delitem__(self, index): + self.index = index + + c = Custom() + _testcapi.sequence_del_slice(c, 0, 5) + self.assertEqual(c.index, slice(0, 5)) + + # Immutable sequences must raise: + with self.assertRaises(TypeError): + _testcapi.sequence_del_slice((1, 2, 3, 4), 1, 3) + with self.assertRaises(TypeError): + _testcapi.sequence_del_slice('abcd', 1, 3) + + # Not a sequnce: + with self.assertRaises(TypeError): + _testcapi.sequence_del_slice(object(), 1, 3) + with self.assertRaises(TypeError): + _testcapi.sequence_del_slice({1: 'a', 2: 'b', 3: 'c'}, 1, 3) + @unittest.skipUnless(hasattr(_testcapi, 'negative_refcount'), 'need _testcapi.negative_refcount') def test_negative_refcount(self): diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 19ceb108ed4e36..4b0cec58be12fe 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -4794,6 +4794,37 @@ mapping_has_key(PyObject* self, PyObject *args) return PyLong_FromLong(PyMapping_HasKey(context, key)); } +static PyObject * +sequence_set_slice(PyObject* self, PyObject *args) +{ + PyObject *sequence, *obj; + Py_ssize_t i1, i2; + if (!PyArg_ParseTuple(args, "OnnO", &sequence, &i1, &i2, &obj)) { + return NULL; + } + + int res = PySequence_SetSlice(sequence, i1, i2, obj); + if (res == -1) { + return NULL; + } + Py_RETURN_NONE; +} + +static PyObject * +sequence_del_slice(PyObject* self, PyObject *args) +{ + PyObject *sequence; + Py_ssize_t i1, i2; + if (!PyArg_ParseTuple(args, "Onn", &sequence, &i1, &i2)) { + return NULL; + } + + int res = PySequence_DelSlice(sequence, i1, i2); + if (res == -1) { + return NULL; + } + Py_RETURN_NONE; +} static PyObject * test_pythread_tss_key_state(PyObject *self, PyObject *args) @@ -6190,6 +6221,8 @@ static PyMethodDef TestMethods[] = { {"get_mapping_items", get_mapping_items, METH_O}, {"test_mapping_has_key_string", test_mapping_has_key_string, METH_NOARGS}, {"mapping_has_key", mapping_has_key, METH_VARARGS}, + {"sequence_set_slice", sequence_set_slice, METH_VARARGS}, + {"sequence_del_slice", sequence_del_slice, METH_VARARGS}, {"test_pythread_tss_key_state", test_pythread_tss_key_state, METH_VARARGS}, {"hamt", new_hamt, METH_NOARGS}, {"bad_get", _PyCFunction_CAST(bad_get), METH_FASTCALL}, From adfd19fb09d704befa400186bf5e146dbd7cc009 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sat, 5 Nov 2022 16:05:52 +0300 Subject: [PATCH 2/3] Address review --- Lib/test/test_capi.py | 58 +++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 1d9145f4db5c6f..5b97a1e48e7c31 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -437,17 +437,24 @@ def __setitem__(self, index, value): self.assertEqual(c.index, slice(0, 5)) self.assertEqual(c.value, 'abc') - # Immutable sequences must raise: - with self.assertRaises(TypeError): - _testcapi.sequence_set_slice((1, 2, 3, 4), 1, 3, (8, 9)) - with self.assertRaises(TypeError): - _testcapi.sequence_set_slice('abcd', 1, 3, 'xy') - - # Not a sequnce: - with self.assertRaises(TypeError): - _testcapi.sequence_set_slice(object(), 1, 3, 'xy') - with self.assertRaises(TypeError): - _testcapi.sequence_set_slice({1: 'a', 2: 'b', 3: 'c'}, 1, 3, 'xy') + # Wrong cases: + import copy + + bad_calls = [ + # Immutable sequences must raise: + ((1, 2, 3, 4), 1, 3, (8, 9)), + ('abcd', 1, 3, 'xy'), + # Not a sequnce: + (None, 1, 3, 'xy'), + ({1: 'a', 2: 'b', 3: 'c'}, 1, 3, 'xy'), # is a mapping + ] + + for seq, i1, i2, obj in bad_calls: + with self.subTest(seq=seq, i1=i1, i2=i2, obj=obj): + seq_copy = copy.copy(seq) + with self.assertRaises(TypeError): + _testcapi.sequence_set_slice(seq, i1, i2, obj) + self.assertEqual(seq, seq_copy) # it must not change def test_sequence_del_slice(self): # Correct case: @@ -468,17 +475,24 @@ def __delitem__(self, index): _testcapi.sequence_del_slice(c, 0, 5) self.assertEqual(c.index, slice(0, 5)) - # Immutable sequences must raise: - with self.assertRaises(TypeError): - _testcapi.sequence_del_slice((1, 2, 3, 4), 1, 3) - with self.assertRaises(TypeError): - _testcapi.sequence_del_slice('abcd', 1, 3) - - # Not a sequnce: - with self.assertRaises(TypeError): - _testcapi.sequence_del_slice(object(), 1, 3) - with self.assertRaises(TypeError): - _testcapi.sequence_del_slice({1: 'a', 2: 'b', 3: 'c'}, 1, 3) + # Wrong cases: + import copy + + bad_calls = [ + # Immutable sequences must raise: + ((1, 2, 3, 4), 1, 3), + ('abcd', 1, 3), + # Not a sequnce: + (None, 1, 3), + ({1: 'a', 2: 'b', 3: 'c'}, 1, 3), # is a mapping + ] + + for seq, i1, i2 in bad_calls: + with self.subTest(seq=seq, i1=i1, i2=i2): + seq_copy = copy.copy(seq) + with self.assertRaises(TypeError): + _testcapi.sequence_del_slice(seq, i1, i2) + self.assertEqual(seq, seq_copy) # it must not change @unittest.skipUnless(hasattr(_testcapi, 'negative_refcount'), 'need _testcapi.negative_refcount') From 51695a894332519e5372c212afb82da78ac7561a Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sat, 5 Nov 2022 16:43:37 +0300 Subject: [PATCH 3/3] Address review 2 --- Lib/test/test_capi.py | 74 ++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 36 deletions(-) diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 5b97a1e48e7c31..f005d693afc73c 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -437,24 +437,25 @@ def __setitem__(self, index, value): self.assertEqual(c.index, slice(0, 5)) self.assertEqual(c.value, 'abc') - # Wrong cases: - import copy - - bad_calls = [ - # Immutable sequences must raise: - ((1, 2, 3, 4), 1, 3, (8, 9)), - ('abcd', 1, 3, 'xy'), - # Not a sequnce: - (None, 1, 3, 'xy'), - ({1: 'a', 2: 'b', 3: 'c'}, 1, 3, 'xy'), # is a mapping - ] - - for seq, i1, i2, obj in bad_calls: - with self.subTest(seq=seq, i1=i1, i2=i2, obj=obj): - seq_copy = copy.copy(seq) - with self.assertRaises(TypeError): - _testcapi.sequence_set_slice(seq, i1, i2, obj) - self.assertEqual(seq, seq_copy) # it must not change + # Immutable sequences must raise: + bad_seq1 = (1, 2, 3, 4) + with self.assertRaises(TypeError): + _testcapi.sequence_set_slice(bad_seq1, 1, 3, (8, 9)) + self.assertEqual(bad_seq1, (1, 2, 3, 4)) + + bad_seq2 = 'abcd' + with self.assertRaises(TypeError): + _testcapi.sequence_set_slice(bad_seq2, 1, 3, 'xy') + self.assertEqual(bad_seq2, 'abcd') + + # Not a sequence: + with self.assertRaises(TypeError): + _testcapi.sequence_set_slice(None, 1, 3, 'xy') + + mapping = {1: 'a', 2: 'b', 3: 'c'} + with self.assertRaises(TypeError): + _testcapi.sequence_set_slice(mapping, 1, 3, 'xy') + self.assertEqual(mapping, {1: 'a', 2: 'b', 3: 'c'}) def test_sequence_del_slice(self): # Correct case: @@ -475,24 +476,25 @@ def __delitem__(self, index): _testcapi.sequence_del_slice(c, 0, 5) self.assertEqual(c.index, slice(0, 5)) - # Wrong cases: - import copy - - bad_calls = [ - # Immutable sequences must raise: - ((1, 2, 3, 4), 1, 3), - ('abcd', 1, 3), - # Not a sequnce: - (None, 1, 3), - ({1: 'a', 2: 'b', 3: 'c'}, 1, 3), # is a mapping - ] - - for seq, i1, i2 in bad_calls: - with self.subTest(seq=seq, i1=i1, i2=i2): - seq_copy = copy.copy(seq) - with self.assertRaises(TypeError): - _testcapi.sequence_del_slice(seq, i1, i2) - self.assertEqual(seq, seq_copy) # it must not change + # Immutable sequences must raise: + bad_seq1 = (1, 2, 3, 4) + with self.assertRaises(TypeError): + _testcapi.sequence_del_slice(bad_seq1, 1, 3) + self.assertEqual(bad_seq1, (1, 2, 3, 4)) + + bad_seq2 = 'abcd' + with self.assertRaises(TypeError): + _testcapi.sequence_del_slice(bad_seq2, 1, 3) + self.assertEqual(bad_seq2, 'abcd') + + # Not a sequence: + with self.assertRaises(TypeError): + _testcapi.sequence_del_slice(None, 1, 3) + + mapping = {1: 'a', 2: 'b', 3: 'c'} + with self.assertRaises(TypeError): + _testcapi.sequence_del_slice(mapping, 1, 3) + self.assertEqual(mapping, {1: 'a', 2: 'b', 3: 'c'}) @unittest.skipUnless(hasattr(_testcapi, 'negative_refcount'), 'need _testcapi.negative_refcount')