From 997df1c684e2f373507c78bc51a49b43251c115f Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 22 May 2024 16:53:33 +0100 Subject: [PATCH 1/7] Make slices marshallable --- Python/marshal.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/Python/marshal.c b/Python/marshal.c index b1708a7306f9e7..f99ff1089beb2e 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -76,6 +76,7 @@ module marshal #define TYPE_UNKNOWN '?' #define TYPE_SET '<' #define TYPE_FROZENSET '>' +#define TYPE_SLICE ':' #define FLAG_REF '\x80' /* with a type, add obj to index */ #define TYPE_ASCII 'a' @@ -613,6 +614,13 @@ w_complex_object(PyObject *v, char flag, WFILE *p) w_pstring(view.buf, view.len, p); PyBuffer_Release(&view); } + else if (PySlice_Check(v)) { + PySliceObject *slice = (PySliceObject *)v; + W_TYPE(TYPE_SLICE, p); + w_object(slice->start, p); + w_object(slice->stop, p); + w_object(slice->step, p); + } else { W_TYPE(TYPE_UNKNOWN, p); p->error = WFERR_UNMARSHALLABLE; @@ -1534,6 +1542,30 @@ r_object(RFILE *p) retval = Py_NewRef(v); break; + case TYPE_SLICE: + { + PyObject *stop = NULL; + PyObject *step = NULL; + PyObject *start = r_object(p); + if (start == NULL) { + goto cleanup; + } + stop = r_object(p); + if (stop == NULL) { + goto cleanup; + } + step = r_object(p); + if (step == NULL) { + goto cleanup; + } + retval = PySlice_New(start, stop, step); + cleanup: + Py_XDECREF(start); + Py_XDECREF(stop); + Py_XDECREF(step); + break; + } + default: /* Bogus data got written, which isn't ideal. This will let you keep working and recover. */ From 0e19ac7d8c860137b862edba598c82d20dbe511a Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Mon, 7 Oct 2024 15:01:53 -0400 Subject: [PATCH 2/7] Emit slices as constants --- Include/internal/pycore_magic_number.h | 3 +- Lib/test/test_compile.py | 27 +++++++++++- Objects/codeobject.c | 1 + Python/codegen.c | 58 ++++++++++++++++++++++---- Python/marshal.c | 3 ++ 5 files changed, 81 insertions(+), 11 deletions(-) diff --git a/Include/internal/pycore_magic_number.h b/Include/internal/pycore_magic_number.h index 2414d25d41bfbf..a88ff2deeba941 100644 --- a/Include/internal/pycore_magic_number.h +++ b/Include/internal/pycore_magic_number.h @@ -259,6 +259,7 @@ Known values: Python 3.14a1 3605 (Move ENTER_EXECUTOR to opcode 255) Python 3.14a1 3606 (Specialize CALL_KW) Python 3.14a1 3607 (Add pseudo instructions JUMP_IF_TRUE/FALSE) + Python 3.14a1 3608 (Add support for slices) Python 3.15 will start with 3650 @@ -271,7 +272,7 @@ PC/launcher.c must also be updated. */ -#define PYC_MAGIC_NUMBER 3607 +#define PYC_MAGIC_NUMBER 3608 /* This is equivalent to converting PYC_MAGIC_NUMBER to 2 bytes (little-endian) and then appending b'\r\n'. */ #define PYC_MAGIC_NUMBER_TOKEN \ diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index e9ee72cf234fdc..981b485d9680a0 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -1373,6 +1373,14 @@ def check_op_count(func, op, expected): actual += 1 self.assertEqual(actual, expected) + def check_consts(func, typ, expected): + slice_consts = 0 + consts = func.__code__.co_consts + for instr in dis.Bytecode(func): + if instr.opname == "LOAD_CONST" and isinstance(consts[instr.oparg], typ): + slice_consts += 1 + self.assertEqual(slice_consts, expected) + def load(): return x[a:b] + x [a:] + x[:b] + x[:] @@ -1388,15 +1396,30 @@ def long_slice(): def aug(): x[a:b] += y - check_op_count(load, "BINARY_SLICE", 4) + def aug_const(): + x[1:2] += y + + def compound_const_slice(): + x[1:2:3, 4:5:6] = y + + check_op_count(load, "BINARY_SLICE", 3) check_op_count(load, "BUILD_SLICE", 0) - check_op_count(store, "STORE_SLICE", 4) + check_consts(load, slice, 1) + check_op_count(store, "STORE_SLICE", 3) check_op_count(store, "BUILD_SLICE", 0) + check_consts(store, slice, 1) check_op_count(long_slice, "BUILD_SLICE", 1) check_op_count(long_slice, "BINARY_SLICE", 0) check_op_count(aug, "BINARY_SLICE", 1) check_op_count(aug, "STORE_SLICE", 1) check_op_count(aug, "BUILD_SLICE", 0) + check_op_count(aug_const, "BINARY_SLICE", 0) + check_op_count(aug_const, "STORE_SLICE", 0) + check_consts(aug_const, slice, 1) + check_op_count(compound_const_slice, "BINARY_SLICE", 0) + check_op_count(compound_const_slice, "BUILD_SLICE", 0) + check_consts(compound_const_slice, slice, 0) + check_consts(compound_const_slice, tuple, 1) def test_compare_positions(self): for opname_prefix, op in [ diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 6f0b3f8b9a3262..8a2f4d32b911d9 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -2336,6 +2336,7 @@ _PyCode_ConstantKey(PyObject *op) if (op == Py_None || op == Py_Ellipsis || PyLong_CheckExact(op) || PyUnicode_CheckExact(op) + || PySlice_Check(op) /* code_richcompare() uses _PyCode_ConstantKey() internally */ || PyCode_Check(op)) { diff --git a/Python/codegen.c b/Python/codegen.c index 896c30cc14952a..cd0c449528d516 100644 --- a/Python/codegen.c +++ b/Python/codegen.c @@ -5008,7 +5008,9 @@ codegen_visit_expr(compiler *c, expr_ty e) { int n = codegen_slice(c, e); RETURN_IF_ERROR(n); - ADDOP_I(c, loc, BUILD_SLICE, n); + if (n) { + ADDOP_I(c, loc, BUILD_SLICE, n); + } break; } case Name_kind: @@ -5029,6 +5031,18 @@ is_two_element_slice(expr_ty s) s->v.Slice.step == NULL; } +static bool +is_constant_slice(expr_ty s) +{ + return s->kind == Slice_kind && + (s->v.Slice.lower == NULL || + s->v.Slice.lower->kind == Constant_kind) && + (s->v.Slice.upper == NULL || + s->v.Slice.upper->kind == Constant_kind) && + (s->v.Slice.step == NULL || + s->v.Slice.step->kind == Constant_kind); +} + static int codegen_augassign(compiler *c, stmt_ty s) { @@ -5046,8 +5060,11 @@ codegen_augassign(compiler *c, stmt_ty s) break; case Subscript_kind: VISIT(c, expr, e->v.Subscript.value); - if (is_two_element_slice(e->v.Subscript.slice)) { - RETURN_IF_ERROR(codegen_slice(c, e->v.Subscript.slice)); + if (!is_constant_slice(e->v.Subscript.slice) && + is_two_element_slice(e->v.Subscript.slice)) { + int n = codegen_slice(c, e->v.Subscript.slice); + assert(n == 2 || n == -1); + RETURN_IF_ERROR(n); ADDOP_I(c, loc, COPY, 3); ADDOP_I(c, loc, COPY, 3); ADDOP_I(c, loc, COPY, 3); @@ -5084,7 +5101,8 @@ codegen_augassign(compiler *c, stmt_ty s) ADDOP_NAME(c, loc, STORE_ATTR, e->v.Attribute.attr, names); break; case Subscript_kind: - if (is_two_element_slice(e->v.Subscript.slice)) { + if (!is_constant_slice(e->v.Subscript.slice) && + is_two_element_slice(e->v.Subscript.slice)) { ADDOP_I(c, loc, SWAP, 4); ADDOP_I(c, loc, SWAP, 3); ADDOP_I(c, loc, SWAP, 2); @@ -5231,8 +5249,13 @@ codegen_subscript(compiler *c, expr_ty e) } VISIT(c, expr, e->v.Subscript.value); - if (is_two_element_slice(e->v.Subscript.slice) && ctx != Del) { - RETURN_IF_ERROR(codegen_slice(c, e->v.Subscript.slice)); + if (!is_constant_slice(e->v.Subscript.slice) && + is_two_element_slice(e->v.Subscript.slice) && + ctx != Del + ) { + int n = codegen_slice(c, e->v.Subscript.slice); + assert(n == 2 || n == -1); + RETURN_IF_ERROR(n); if (ctx == Load) { ADDOP(c, loc, BINARY_SLICE); } @@ -5254,14 +5277,33 @@ codegen_subscript(compiler *c, expr_ty e) return SUCCESS; } -/* Returns the number of the values emitted, - * thus are needed to build the slice, or -1 if there is an error. */ +/* Returns the number of the values emitted that are needed + * to build the slice. May be 0 if a constant slice was emitted. + * -1 if there is an error. */ static int codegen_slice(compiler *c, expr_ty s) { int n = 2; assert(s->kind == Slice_kind); + if (is_constant_slice(s)) { + PyObject *start = NULL; + if (s->v.Slice.lower) { + start = s->v.Slice.lower->v.Constant.value; + } + PyObject *stop = NULL; + if (s->v.Slice.upper) { + stop = s->v.Slice.upper->v.Constant.value; + } + PyObject *step = NULL; + if (s->v.Slice.step) { + step = s->v.Slice.step->v.Constant.value; + } + PyObject *slice = PySlice_New(start, stop, step); + ADDOP_LOAD_CONST_NEW(c, LOC(s), slice); + return 0; + } + /* only handles the cases where BUILD_SLICE is emitted */ if (s->v.Slice.lower) { VISIT(c, expr, s->v.Slice.lower); diff --git a/Python/marshal.c b/Python/marshal.c index f99ff1089beb2e..35922788a3c52c 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -1544,6 +1544,7 @@ r_object(RFILE *p) case TYPE_SLICE: { + int idx = r_ref_reserve(flag, p); PyObject *stop = NULL; PyObject *step = NULL; PyObject *start = r_object(p); @@ -1559,6 +1560,8 @@ r_object(RFILE *p) goto cleanup; } retval = PySlice_New(start, stop, step); + if (idx) + r_ref_insert(retval, idx, flag, p); cleanup: Py_XDECREF(start); Py_XDECREF(stop); From e3ddaea1a9fb222aac36a592e3c1ca69b1e243a4 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Mon, 7 Oct 2024 18:28:11 -0400 Subject: [PATCH 3/7] Update Python/marshal.c Co-authored-by: Peter Bierma --- Python/marshal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/marshal.c b/Python/marshal.c index 35922788a3c52c..82c7512b6df055 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -1544,7 +1544,7 @@ r_object(RFILE *p) case TYPE_SLICE: { - int idx = r_ref_reserve(flag, p); + Py_ssize_t idx = r_ref_reserve(flag, p); PyObject *stop = NULL; PyObject *step = NULL; PyObject *start = r_object(p); From 86ab911c7a5489f67c362bfe7953e2cd0f4f79ee Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Tue, 8 Oct 2024 08:46:59 -0400 Subject: [PATCH 4/7] Refactor codegen_slice into two functions so it always has the same net effect --- Python/codegen.c | 82 +++++++++++++++++++++++------------------------- 1 file changed, 40 insertions(+), 42 deletions(-) diff --git a/Python/codegen.c b/Python/codegen.c index cd0c449528d516..ccf9ae5dbf4cce 100644 --- a/Python/codegen.c +++ b/Python/codegen.c @@ -194,6 +194,7 @@ static int codegen_visit_expr(compiler *, expr_ty); static int codegen_augassign(compiler *, stmt_ty); static int codegen_annassign(compiler *, stmt_ty); static int codegen_subscript(compiler *, expr_ty); +static int codegen_slice_two_parts(compiler *, expr_ty); static int codegen_slice(compiler *, expr_ty); static bool are_all_items_const(asdl_expr_seq *, Py_ssize_t, Py_ssize_t); @@ -5005,14 +5006,8 @@ codegen_visit_expr(compiler *c, expr_ty e) } break; case Slice_kind: - { - int n = codegen_slice(c, e); - RETURN_IF_ERROR(n); - if (n) { - ADDOP_I(c, loc, BUILD_SLICE, n); - } + RETURN_IF_ERROR(codegen_slice(c, e)); break; - } case Name_kind: return codegen_nameop(c, loc, e->v.Name.id, e->v.Name.ctx); /* child nodes of List and Tuple will have expr_context set */ @@ -5024,13 +5019,6 @@ codegen_visit_expr(compiler *c, expr_ty e) return SUCCESS; } -static bool -is_two_element_slice(expr_ty s) -{ - return s->kind == Slice_kind && - s->v.Slice.step == NULL; -} - static bool is_constant_slice(expr_ty s) { @@ -5043,6 +5031,14 @@ is_constant_slice(expr_ty s) s->v.Slice.step->kind == Constant_kind); } +static bool +should_apply_two_element_slice_optimization(expr_ty s) +{ + return !is_constant_slice(s) && + s->kind == Slice_kind && + s->v.Slice.step == NULL; +} + static int codegen_augassign(compiler *c, stmt_ty s) { @@ -5060,11 +5056,8 @@ codegen_augassign(compiler *c, stmt_ty s) break; case Subscript_kind: VISIT(c, expr, e->v.Subscript.value); - if (!is_constant_slice(e->v.Subscript.slice) && - is_two_element_slice(e->v.Subscript.slice)) { - int n = codegen_slice(c, e->v.Subscript.slice); - assert(n == 2 || n == -1); - RETURN_IF_ERROR(n); + if (should_apply_two_element_slice_optimization(e->v.Subscript.slice)) { + RETURN_IF_ERROR(codegen_slice_two_parts(c, e->v.Subscript.slice)); ADDOP_I(c, loc, COPY, 3); ADDOP_I(c, loc, COPY, 3); ADDOP_I(c, loc, COPY, 3); @@ -5101,8 +5094,7 @@ codegen_augassign(compiler *c, stmt_ty s) ADDOP_NAME(c, loc, STORE_ATTR, e->v.Attribute.attr, names); break; case Subscript_kind: - if (!is_constant_slice(e->v.Subscript.slice) && - is_two_element_slice(e->v.Subscript.slice)) { + if (should_apply_two_element_slice_optimization(e->v.Subscript.slice)) { ADDOP_I(c, loc, SWAP, 4); ADDOP_I(c, loc, SWAP, 3); ADDOP_I(c, loc, SWAP, 2); @@ -5249,13 +5241,10 @@ codegen_subscript(compiler *c, expr_ty e) } VISIT(c, expr, e->v.Subscript.value); - if (!is_constant_slice(e->v.Subscript.slice) && - is_two_element_slice(e->v.Subscript.slice) && + if (should_apply_two_element_slice_optimization(e->v.Subscript.slice) && ctx != Del ) { - int n = codegen_slice(c, e->v.Subscript.slice); - assert(n == 2 || n == -1); - RETURN_IF_ERROR(n); + RETURN_IF_ERROR(codegen_slice_two_parts(c, e->v.Subscript.slice)); if (ctx == Load) { ADDOP(c, loc, BINARY_SLICE); } @@ -5277,8 +5266,28 @@ codegen_subscript(compiler *c, expr_ty e) return SUCCESS; } -/* Returns the number of the values emitted that are needed - * to build the slice. May be 0 if a constant slice was emitted. +static int +codegen_slice_two_parts(compiler *c, expr_ty s) +{ + if (s->v.Slice.lower) { + VISIT(c, expr, s->v.Slice.lower); + } + else { + ADDOP_LOAD_CONST(c, LOC(s), Py_None); + } + + if (s->v.Slice.upper) { + VISIT(c, expr, s->v.Slice.upper); + } + else { + ADDOP_LOAD_CONST(c, LOC(s), Py_None); + } + + return 0; +} + +/* Returns the number of the values emitted as part of the slice. + * May be 0 if a constant slice was emitted. * -1 if there is an error. */ static int codegen_slice(compiler *c, expr_ty s) @@ -5304,25 +5313,14 @@ codegen_slice(compiler *c, expr_ty s) return 0; } - /* only handles the cases where BUILD_SLICE is emitted */ - if (s->v.Slice.lower) { - VISIT(c, expr, s->v.Slice.lower); - } - else { - ADDOP_LOAD_CONST(c, LOC(s), Py_None); - } - - if (s->v.Slice.upper) { - VISIT(c, expr, s->v.Slice.upper); - } - else { - ADDOP_LOAD_CONST(c, LOC(s), Py_None); - } + RETURN_IF_ERROR(codegen_slice_two_parts(c, s)); if (s->v.Slice.step) { n++; VISIT(c, expr, s->v.Slice.step); } + + ADDOP_I(c, LOC(s), BUILD_SLICE, n); return n; } From 1c9cc518ad22494eb2bda4c9f38ff924a5386661 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Tue, 8 Oct 2024 08:49:20 -0400 Subject: [PATCH 5/7] Fix for free-threaded builds --- Objects/sliceobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/sliceobject.c b/Objects/sliceobject.c index 1b6d35998c2b69..4fef0af93fe095 100644 --- a/Objects/sliceobject.c +++ b/Objects/sliceobject.c @@ -343,7 +343,7 @@ Create a slice object. This is used for extended slicing (e.g. a[0:10:2])."); static void slice_dealloc(PySliceObject *r) { - _PyObject_GC_UNTRACK(r); + PyObject_GC_UnTrack(r); Py_DECREF(r->step); Py_DECREF(r->start); Py_DECREF(r->stop); From 44e25a6572bcce3fa28af5002d57d8881d5d6e79 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Tue, 8 Oct 2024 09:02:49 -0400 Subject: [PATCH 6/7] Simplify marshal loading of slices --- Python/marshal.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Python/marshal.c b/Python/marshal.c index 82c7512b6df055..3d127b4e331d0d 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -1560,8 +1560,7 @@ r_object(RFILE *p) goto cleanup; } retval = PySlice_New(start, stop, step); - if (idx) - r_ref_insert(retval, idx, flag, p); + r_ref_insert(retval, idx, flag, p); cleanup: Py_XDECREF(start); Py_XDECREF(stop); From e3948bb7c82982820bce4a4ec6ffef7ff5d99a8d Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Tue, 8 Oct 2024 10:07:24 -0400 Subject: [PATCH 7/7] Only return SUCCESS/ERROR from codegen_slice --- Python/codegen.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Python/codegen.c b/Python/codegen.c index ccf9ae5dbf4cce..689d2b5124e9d3 100644 --- a/Python/codegen.c +++ b/Python/codegen.c @@ -5286,9 +5286,6 @@ codegen_slice_two_parts(compiler *c, expr_ty s) return 0; } -/* Returns the number of the values emitted as part of the slice. - * May be 0 if a constant slice was emitted. - * -1 if there is an error. */ static int codegen_slice(compiler *c, expr_ty s) { @@ -5309,8 +5306,11 @@ codegen_slice(compiler *c, expr_ty s) step = s->v.Slice.step->v.Constant.value; } PyObject *slice = PySlice_New(start, stop, step); + if (slice == NULL) { + return ERROR; + } ADDOP_LOAD_CONST_NEW(c, LOC(s), slice); - return 0; + return SUCCESS; } RETURN_IF_ERROR(codegen_slice_two_parts(c, s)); @@ -5321,7 +5321,7 @@ codegen_slice(compiler *c, expr_ty s) } ADDOP_I(c, LOC(s), BUILD_SLICE, n); - return n; + return SUCCESS; }