From 775fda6ac34340ac2a13861d39d868e39f777d93 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 15 Jun 2019 19:01:10 +0300 Subject: [PATCH] bpo-1875, bpo-32477: Raise SyntaxError in invalid blocks that will be optimized away. Also simplify the code generator and peepholer for "if" and "while" with constant condition. Also optimize more cases of conditional jumps with constant condition. --- Lib/test/test_dis.py | 10 +- Lib/test/test_syntax.py | 22 ++++- Misc/NEWS.d/3.8.0b1.rst | 4 +- .../2019-06-15-19-01-05.bpo-1875.IfLQg1.rst | 3 + Python/compile.c | 95 ++++++------------- Python/peephole.c | 16 ---- 6 files changed, 61 insertions(+), 89 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-06-15-19-01-05.bpo-1875.IfLQg1.rst diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index 652af45d55ade9e..a05d002be27f1d0 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -140,22 +140,22 @@ def bug708901(): def bug1333982(x=[]): - assert 0, ([s for s in x] + + assert x, ([s for s in x] + 1) pass dis_bug1333982 = """\ -%3d 0 LOAD_CONST 1 (0) +%3d 0 LOAD_FAST 0 (x) 2 POP_JUMP_IF_TRUE 26 4 LOAD_GLOBAL 0 (AssertionError) - 6 LOAD_CONST 2 ( at 0x..., file "%s", line %d>) - 8 LOAD_CONST 3 ('bug1333982..') + 6 LOAD_CONST 1 ( at 0x..., file "%s", line %d>) + 8 LOAD_CONST 2 ('bug1333982..') 10 MAKE_FUNCTION 0 12 LOAD_FAST 0 (x) 14 GET_ITER 16 CALL_FUNCTION 1 -%3d 18 LOAD_CONST 4 (1) +%3d 18 LOAD_CONST 3 (1) %3d 20 BINARY_ADD 22 CALL_FUNCTION 1 diff --git a/Lib/test/test_syntax.py b/Lib/test/test_syntax.py index 8451c072f64209b..9ab026646dbf45a 100644 --- a/Lib/test/test_syntax.py +++ b/Lib/test/test_syntax.py @@ -698,17 +698,35 @@ def test_break_outside_loop(self): def test_yield_outside_function(self): self._check_error("if 0: yield", "outside function") - self._check_error("class C:\n if 0: yield", "outside function") + self._check_error("if 1: pass\nelse: yield", "outside function") + self._check_error("while 0: yield", "outside function") + self._check_error("class C:\n if 0: yield", "outside function") + self._check_error("class C:\n if 1: pass\n else: yield", + "outside function") + self._check_error("class C:\n while 0: yield", "outside function") def test_return_outside_function(self): self._check_error("if 0: return", "outside function") - self._check_error("class C:\n if 0: return", "outside function") + self._check_error("if 1: pass\nelse: return", "outside function") + self._check_error("while 0: return", "outside function") + self._check_error("class C:\n if 0: return", "outside function") + self._check_error("class C:\n if 1: pass\n else: return", + "outside function") + self._check_error("class C:\n while 0: return", "outside function") def test_break_outside_loop(self): self._check_error("if 0: break", "outside loop") + self._check_error("if 1: pass\nelse: break", "outside loop") + self._check_error("class C:\n if 0: break", "outside loop") + self._check_error("class C:\n if 1: pass\n else: break", + "outside loop") def test_continue_outside_loop(self): self._check_error("if 0: continue", "not properly in loop") + self._check_error("if 1: pass\nelse: continue", "not properly in loop") + self._check_error("class C:\n if 0: continue", "not properly in loop") + self._check_error("class C:\n if 1: pass\n else: continue", + "not properly in loop") def test_unexpected_indent(self): self._check_error("foo()\n bar()\n", "unexpected indent", diff --git a/Misc/NEWS.d/3.8.0b1.rst b/Misc/NEWS.d/3.8.0b1.rst index d083983642e7a0c..af75cc4507f234b 100644 --- a/Misc/NEWS.d/3.8.0b1.rst +++ b/Misc/NEWS.d/3.8.0b1.rst @@ -256,8 +256,8 @@ Add NamedExpression kind support to ast_unparse.c .. nonce: 9oxXFX .. section: Core and Builtins -A :exc:`SyntaxError` is now raised if a code blocks that will be optimized -away (e.g. if conditions that are always false) contains syntax errors. +A :exc:`SyntaxError` is now raised if the "if" code blocks that will be optimized +away (if the conditions is always false) contains syntax errors. Patch by Pablo Galindo. .. diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-06-15-19-01-05.bpo-1875.IfLQg1.rst b/Misc/NEWS.d/next/Core and Builtins/2019-06-15-19-01-05.bpo-1875.IfLQg1.rst new file mode 100644 index 000000000000000..a4a86e55ed9cb64 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-06-15-19-01-05.bpo-1875.IfLQg1.rst @@ -0,0 +1,3 @@ +A :exc:`SyntaxError` is now always raised if a code blocks that will be +optimized away (e.g. if conditions that are always false) contains syntax +errors. diff --git a/Python/compile.c b/Python/compile.c index 4d3ecfe5d6fc9d3..151ade5c1a1bd47 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -190,7 +190,6 @@ static int compiler_visit_slice(struct compiler *, slice_ty, expr_context_ty); static int inplace_binop(struct compiler *, operator_ty); -static int expr_constant(expr_ty); static int compiler_with(struct compiler *, stmt_ty, int); static int compiler_async_with(struct compiler *, stmt_ty, int); @@ -2448,10 +2447,22 @@ compiler_jump_if(struct compiler *c, expr_ty e, basicblock *next, int cond) /* fallback to general implementation */ break; } - default: + default: { + if (e->kind == Constant_kind) { + int is_true = PyObject_IsTrue(e->v.Constant.value); + if (is_true < 0) { + return 0; + } + if (is_true == cond) { + ADDOP_JABS(c, JUMP_ABSOLUTE, next); + NEXT_BLOCK(c); + } + return 1; + } /* fallback to general implementation */ break; } + } /* general implementation */ VISIT(c, expr, e); @@ -2539,36 +2550,23 @@ static int compiler_if(struct compiler *c, stmt_ty s) { basicblock *end, *next; - int constant; assert(s->kind == If_kind); - end = compiler_new_block(c); + next = end = compiler_new_block(c); if (end == NULL) return 0; - constant = expr_constant(s->v.If.test); - /* constant = 0: "if 0" Leave the optimizations to - * the pephole optimizer to check for syntax errors - * in the block. - * constant = 1: "if 1", "if 2", ... - * constant = -1: rest */ - if (constant == 1) { - VISIT_SEQ(c, stmt, s->v.If.body); - } else { - if (asdl_seq_LEN(s->v.If.orelse)) { - next = compiler_new_block(c); - if (next == NULL) - return 0; - } - else - next = end; - if (!compiler_jump_if(c, s->v.If.test, next, 0)) + if (asdl_seq_LEN(s->v.If.orelse)) { + next = compiler_new_block(c); + if (next == NULL) return 0; - VISIT_SEQ(c, stmt, s->v.If.body); - if (asdl_seq_LEN(s->v.If.orelse)) { - ADDOP_JREL(c, JUMP_FORWARD, end); - compiler_use_next_block(c, next); - VISIT_SEQ(c, stmt, s->v.If.orelse); - } + } + if (!compiler_jump_if(c, s->v.If.test, next, 0)) + return 0; + VISIT_SEQ(c, stmt, s->v.If.body); + if (asdl_seq_LEN(s->v.If.orelse)) { + ADDOP_JREL(c, JUMP_FORWARD, end); + compiler_use_next_block(c, next); + VISIT_SEQ(c, stmt, s->v.If.orelse); } compiler_use_next_block(c, end); return 1; @@ -2658,38 +2656,24 @@ compiler_async_for(struct compiler *c, stmt_ty s) static int compiler_while(struct compiler *c, stmt_ty s) { - basicblock *loop, *orelse, *end, *anchor = NULL; - int constant = expr_constant(s->v.While.test); + basicblock *loop, *orelse = NULL, *end, *anchor; - if (constant == 0) { - if (s->v.While.orelse) - VISIT_SEQ(c, stmt, s->v.While.orelse); - return 1; - } loop = compiler_new_block(c); end = compiler_new_block(c); - if (constant == -1) { - anchor = compiler_new_block(c); - if (anchor == NULL) - return 0; - } - if (loop == NULL || end == NULL) + anchor = compiler_new_block(c); + if (loop == NULL || end == NULL || anchor == NULL) return 0; if (s->v.While.orelse) { orelse = compiler_new_block(c); if (orelse == NULL) return 0; } - else - orelse = NULL; compiler_use_next_block(c, loop); if (!compiler_push_fblock(c, WHILE_LOOP, loop, end)) return 0; - if (constant == -1) { - if (!compiler_jump_if(c, s->v.While.test, anchor, 0)) - return 0; - } + if (!compiler_jump_if(c, s->v.While.test, anchor, 0)) + return 0; VISIT_SEQ(c, stmt, s->v.While.body); ADDOP_JABS(c, JUMP_ABSOLUTE, loop); @@ -2697,8 +2681,7 @@ compiler_while(struct compiler *c, stmt_ty s) if there is no else clause ? */ - if (constant == -1) - compiler_use_next_block(c, anchor); + compiler_use_next_block(c, anchor); compiler_pop_fblock(c, WHILE_LOOP, loop); if (orelse != NULL) /* what if orelse is just pass? */ @@ -4508,22 +4491,6 @@ compiler_visit_keyword(struct compiler *c, keyword_ty k) return 1; } -/* Test whether expression is constant. For constants, report - whether they are true or false. - - Return values: 1 for true, 0 for false, -1 for non-constant. - */ - -static int -expr_constant(expr_ty e) -{ - if (e->kind == Constant_kind) { - return PyObject_IsTrue(e->v.Constant.value); - } - return -1; -} - - /* Implements the async with statement. diff --git a/Python/peephole.c b/Python/peephole.c index 3e56e788b0079b5..94bb4de57ae86a8 100644 --- a/Python/peephole.c +++ b/Python/peephole.c @@ -300,24 +300,8 @@ PyCode_Optimize(PyObject *code, PyObject* consts, PyObject *names, cumlc = 0; switch (opcode) { - /* Skip over LOAD_CONST trueconst - POP_JUMP_IF_FALSE xx. This improves - "while 1" performance. */ case LOAD_CONST: cumlc = lastlc + 1; - if (nextop != POP_JUMP_IF_FALSE || - !ISBASICBLOCK(blocks, op_start, i + 1)) { - break; - } - PyObject* cnt = PyList_GET_ITEM(consts, get_arg(codestr, i)); - int is_true = PyObject_IsTrue(cnt); - if (is_true == -1) { - goto exitError; - } - if (is_true == 1) { - fill_nops(codestr, op_start, nexti + 1); - cumlc = 0; - } break; /* Try to fold tuples of constants.