From 732faf17a618d65d264ffe775fa6db4508e3d9ff Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Thu, 15 Feb 2024 14:20:19 +0000 Subject: gh-115347: avoid emitting redundant NOP for the docstring with -OO (#115494) --- Lib/test/test_compile.py | 26 +++++++++++++++ .../2024-02-14-23-50-43.gh-issue-115347.VkHvQC.rst | 2 ++ Python/compile.c | 38 ++++++++++++---------- 3 files changed, 48 insertions(+), 18 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-02-14-23-50-43.gh-issue-115347.VkHvQC.rst diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index ebb479f..4d8647b 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -1,4 +1,6 @@ +import contextlib import dis +import io import math import os import unittest @@ -812,6 +814,30 @@ class TestSpecifics(unittest.TestCase): 'RETURN_CONST', list(dis.get_instructions(unused_code_at_end))[-1].opname) + @support.cpython_only + def test_docstring_omitted(self): + # See gh-115347 + src = textwrap.dedent(""" + def f(): + "docstring1" + def h(): + "docstring2" + return 42 + + class C: + "docstring3" + pass + + return h + """) + for opt in [-1, 0, 1, 2]: + with self.subTest(opt=opt): + code = compile(src, "", "exec", optimize=opt) + output = io.StringIO() + with contextlib.redirect_stdout(output): + dis.dis(code) + self.assertNotIn('NOP' , output.getvalue()) + def test_dont_merge_constants(self): # Issue #25843: compile() must not merge constants which are equal # but have a different type. diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-02-14-23-50-43.gh-issue-115347.VkHvQC.rst b/Misc/NEWS.d/next/Core and Builtins/2024-02-14-23-50-43.gh-issue-115347.VkHvQC.rst new file mode 100644 index 0000000..16f357a --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-02-14-23-50-43.gh-issue-115347.VkHvQC.rst @@ -0,0 +1,2 @@ +Fix bug where docstring was replaced by a redundant NOP when Python is run +with ``-OO``. diff --git a/Python/compile.c b/Python/compile.c index 15e5cf3..f95e3cd 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -1692,16 +1692,13 @@ compiler_unwind_fblock_stack(struct compiler *c, location *ploc, static int compiler_body(struct compiler *c, location loc, asdl_stmt_seq *stmts) { - int i = 0; - stmt_ty st; - PyObject *docstring; /* Set current line number to the line number of first statement. This way line number for SETUP_ANNOTATIONS will always coincide with the line number of first "real" statement in module. If body is empty, then lineno will be set later in optimize_and_assemble. */ if (c->u->u_scope_type == COMPILER_SCOPE_MODULE && asdl_seq_LEN(stmts)) { - st = (stmt_ty)asdl_seq_GET(stmts, 0); + stmt_ty st = (stmt_ty)asdl_seq_GET(stmts, 0); loc = LOC(st); } /* Every annotated class and module should have __annotations__. */ @@ -1711,16 +1708,17 @@ compiler_body(struct compiler *c, location loc, asdl_stmt_seq *stmts) if (!asdl_seq_LEN(stmts)) { return SUCCESS; } - /* if not -OO mode, set docstring */ - if (c->c_optimize < 2) { - docstring = _PyAST_GetDocString(stmts); - if (docstring) { + Py_ssize_t first_instr = 0; + PyObject *docstring = _PyAST_GetDocString(stmts); + if (docstring) { + first_instr = 1; + /* if not -OO mode, set docstring */ + if (c->c_optimize < 2) { PyObject *cleandoc = _PyCompile_CleanDoc(docstring); if (cleandoc == NULL) { return ERROR; } - i = 1; - st = (stmt_ty)asdl_seq_GET(stmts, 0); + stmt_ty st = (stmt_ty)asdl_seq_GET(stmts, 0); assert(st->kind == Expr_kind); location loc = LOC(st->v.Expr.value); ADDOP_LOAD_CONST(c, loc, cleandoc); @@ -1728,7 +1726,7 @@ compiler_body(struct compiler *c, location loc, asdl_stmt_seq *stmts) RETURN_IF_ERROR(compiler_nameop(c, NO_LOCATION, &_Py_ID(__doc__), Store)); } } - for (; i < asdl_seq_LEN(stmts); i++) { + for (Py_ssize_t i = first_instr; i < asdl_seq_LEN(stmts); i++) { VISIT(c, stmt, (stmt_ty)asdl_seq_GET(stmts, i)); } return SUCCESS; @@ -2239,7 +2237,6 @@ static int compiler_function_body(struct compiler *c, stmt_ty s, int is_async, Py_ssize_t funcflags, int firstlineno) { - PyObject *docstring = NULL; arguments_ty args; identifier name; asdl_stmt_seq *body; @@ -2266,28 +2263,33 @@ compiler_function_body(struct compiler *c, stmt_ty s, int is_async, Py_ssize_t f RETURN_IF_ERROR( compiler_enter_scope(c, name, scope_type, (void *)s, firstlineno)); - /* if not -OO mode, add docstring */ - if (c->c_optimize < 2) { - docstring = _PyAST_GetDocString(body); - if (docstring) { + Py_ssize_t first_instr = 0; + PyObject *docstring = _PyAST_GetDocString(body); + if (docstring) { + first_instr = 1; + /* if not -OO mode, add docstring */ + if (c->c_optimize < 2) { docstring = _PyCompile_CleanDoc(docstring); if (docstring == NULL) { compiler_exit_scope(c); return ERROR; } } + else { + docstring = NULL; + } } if (compiler_add_const(c->c_const_cache, c->u, docstring ? docstring : Py_None) < 0) { Py_XDECREF(docstring); compiler_exit_scope(c); return ERROR; } - Py_XDECREF(docstring); + Py_CLEAR(docstring); c->u->u_metadata.u_argcount = asdl_seq_LEN(args->args); c->u->u_metadata.u_posonlyargcount = asdl_seq_LEN(args->posonlyargs); c->u->u_metadata.u_kwonlyargcount = asdl_seq_LEN(args->kwonlyargs); - for (Py_ssize_t i = docstring ? 1 : 0; i < asdl_seq_LEN(body); i++) { + for (Py_ssize_t i = first_instr; i < asdl_seq_LEN(body); i++) { VISIT_IN_SCOPE(c, stmt, (stmt_ty)asdl_seq_GET(body, i)); } if (c->u->u_ste->ste_coroutine || c->u->u_ste->ste_generator) { -- cgit v0.12