bpo-37500: Make sure dead code does not generate bytecode but also detect syntax errors #14612

pablogsal · 2019-07-05T22:31:44Z

https://bugs.python.org/issue37500

Add a new field to the compiler structure that allows to be configured
so no bytecode is emitted. In this way is possible to detect errors by
walking the nodes while preserving optimizations.

https://bugs.python.org/issue37500

Python/peephole.c

vstinner

Does this change still mark a function as a generator when using "if 0: yield"? Does it still detect syntax errors?

Python/compile.c

tim-one · 2019-07-06T00:48:50Z

Does this change still mark a function as a generator when using "if 0: yield"?

Victor, test_generators.py has always had a test to make sure that's so - so the test suite will catch that if it breaks.

pablogsal · 2019-07-06T00:52:01Z

Does this change still mark a function as a generator when using "if 0: yield"? Does it still detect syntax errors?

Yes and yes.

…tted Add a new field to the compiler structure that allows to be configured so no bytecode is emitted. In this way is possible to detect errors by walking the nodes while preserving optimizations. Add a specific tests for dead blocks being optimized.

pablogsal · 2019-07-06T01:30:24Z

I have updated the PR adding more tests

Python/compile.c

bedevere-bot · 2019-07-14T11:13:01Z

Thanks for making the requested changes!

@markshannon: please review the changes made to this pull request.

serhiy-storchaka

I am not sure that this is the best way, at least it is not simple, so I suggest to wait with its merging until we try other approaches.

serhiy-storchaka · 2019-07-14T17:21:11Z

Python/compile.c

+ * while visiting nodes.
+*/
+
+#define BEGIN_DO_NOT_EMIT_BYTECODE { \


BEGIN_DO_NOT_EMIT_BYTECODE and END_DO_NOT_EMIT_BYTECODE always surround VISIT_SEQ. I would add a new macros which expands to

c->c_do_not_emit_bytecode++; VISIT_SEQ(...); c->c_do_not_emit_bytecode--;

instead of BEGIN_DO_NOT_EMIT_BYTECODE and END_DO_NOT_EMIT_BYTECODE.

Will update the PR soon.

pablogsal · 2019-07-14T18:22:49Z

so I suggest to wait with its merging until we try other approaches.

If you think there are better ways, I am happy to try them out :)

@serhiy-storchaka Could you elaborate a bit more what you don't like about this approach?

vstinner

LGTM. I dislike the macros, but you keep them if you prefer.

This change looks like a nice way to fix https://bugs.python.org/issue37500

Python/compile.c

miss-islington · 2019-07-15T09:15:04Z

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

…tect syntax errors (pythonGH-14612) https://bugs.python.org/issue37500 Add a new field to the compiler structure that allows to be configured so no bytecode is emitted. In this way is possible to detect errors by walking the nodes while preserving optimizations. https://bugs.python.org/issue37500 (cherry picked from commit 18c5f9d) Co-authored-by: Pablo Galindo <[email protected]>

bedevere-bot · 2019-07-15T09:15:23Z

GH-14780 is a backport of this pull request to the 3.8 branch.

vstinner · 2019-07-15T09:16:19Z

Oh, I didn't want to merge directly the change, only say that it LGTM. I didn't want to merge it since @serhiy-storchaka seems to want to experiment another approach (that he didn't mention). I dislike automerge :-(

vstinner · 2019-07-15T09:17:04Z

I closed the 3.8 backport (without merging it), until we agree on what should be done.

vstinner · 2019-07-15T13:15:54Z

I am not sure that this is the best way, at least it is not simple, so I suggest to wait with its merging until we try other approaches.

@serhiy-storchaka: Would you mind to elaborate why you consider that this change is "not simple"? It sounds like a simple and working solution to me. It don't see how we can produce SyntaxError without this approach (visit all nodes even the ones that we remove).

markshannon · 2019-07-21T10:25:31Z

I see no reason not to merge this into 3.8.
We need to fix https://bugs.python.org/issue37500 and I can't envisage any simpler fix.
Longer term we will want to improve the compiler, but that would be too big a change for 3.8.
@vstinner any objections to reopening #14780?

miss-islington · 2019-07-29T14:22:27Z

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

bedevere-bot · 2019-07-29T14:22:43Z

GH-15002 is a backport of this pull request to the 3.8 branch.

…tect syntax errors (pythonGH-14612) https://bugs.python.org/issue37500 Add a new field to the compiler structure that allows to be configured so no bytecode is emitted. In this way is possible to detect errors by walking the nodes while preserving optimizations. https://bugs.python.org/issue37500 (cherry picked from commit 18c5f9d) Co-authored-by: Pablo Galindo <[email protected]>

pablogsal · 2019-07-29T14:23:51Z

I am going to reopen the backport to 3.8 as I think is very important to test this before b4 so we can react accordingly if something goes wrong. If we discover a better version to do this, we can always backport it on top, but I think is very important to give the solution some exposure before the release candidates and the last beta.

I will keep investigating for better solutions and maybe will do some refactors with some of the proposed ideas (like creating a wrapper for VISIT_SEQ instead).

…tect syntax errors (GH-14612) https://bugs.python.org/issue37500 Add a new field to the compiler structure that allows to be configured so no bytecode is emitted. In this way is possible to detect errors by walking the nodes while preserving optimizations. https://bugs.python.org/issue37500 (cherry picked from commit 18c5f9d) Co-authored-by: Pablo Galindo <[email protected]>

…tect syntax errors (pythonGH-14612) https://bugs.python.org/issue37500 Add a new field to the compiler structure that allows to be configured so no bytecode is emitted. In this way is possible to detect errors by walking the nodes while preserving optimizations. https://bugs.python.org/issue37500

the-knights-who-say-ni added the CLA signed label Jul 5, 2019

bedevere-bot added the awaiting core review label Jul 5, 2019

pablogsal added skip issue skip news labels Jul 5, 2019

pablogsal requested a review from tim-one July 5, 2019 22:38

pablogsal changed the title ~~bpo-37500: Revert commit 85ed1712e428f93408f56fc684816f9a85b0ebc0~~ bpo-37500: Make sure bytecode for always false conditional is not emmited Jul 5, 2019

pablogsal force-pushed the fix_opt2 branch from ced2e13 to 3746b10 Compare July 5, 2019 23:07

pablogsal requested review from serhiy-storchaka and vstinner July 5, 2019 23:10

pablogsal commented Jul 5, 2019

View reviewed changes

Python/peephole.c Outdated Show resolved Hide resolved

pablogsal force-pushed the fix_opt2 branch 3 times, most recently from bf70fa2 to 40530df Compare July 6, 2019 00:15

pablogsal added 🤖 automerge and removed skip issue labels Jul 6, 2019

pablogsal force-pushed the fix_opt2 branch from 40530df to c7a8607 Compare July 6, 2019 00:34

vstinner reviewed Jul 6, 2019

View reviewed changes

Python/compile.c Outdated Show resolved Hide resolved

pablogsal force-pushed the fix_opt2 branch from c7a8607 to 2041319 Compare July 6, 2019 00:48

pablogsal force-pushed the fix_opt2 branch from 2041319 to adf20fa Compare July 6, 2019 00:51

pablogsal self-assigned this Jul 6, 2019

pablogsal force-pushed the fix_opt2 branch from adf20fa to 23efd8e Compare July 6, 2019 01:00

Add more tests and skip more dead code

b58ad27

pablogsal changed the title ~~bpo-37500: Make sure bytecode for always false conditional is not emmited~~ bpo-37500: Make sure dead code does not generate bytecode Jul 6, 2019

pablogsal changed the title ~~bpo-37500: Make sure dead code does not generate bytecode~~ bpo-37500: Make sure dead code does not generate bytecode but also detect syntax errors Jul 6, 2019

pablogsal added the needs backport to 3.8 label Jul 6, 2019

vstinner reviewed Jul 6, 2019

View reviewed changes

Python/compile.c Outdated Show resolved Hide resolved

bedevere-bot removed the awaiting changes label Jul 14, 2019

bedevere-bot added the awaiting change review label Jul 14, 2019

serhiy-storchaka reviewed Jul 14, 2019

View reviewed changes

vstinner approved these changes Jul 15, 2019

View reviewed changes

Python/compile.c Show resolved Hide resolved

bedevere-bot added awaiting merge and removed awaiting change review labels Jul 15, 2019

miss-islington merged commit 18c5f9d into python:master Jul 15, 2019

bedevere-bot removed the awaiting merge label Jul 15, 2019

bedevere-bot removed the needs backport to 3.8 label Jul 15, 2019

vstinner mentioned this pull request Jul 15, 2019

[3.8] bpo-37500: Make sure dead code does not generate bytecode but also detect syntax errors (GH-14612) #14780

Closed

pablogsal added the needs backport to 3.8 label Jul 29, 2019

bedevere-bot removed the needs backport to 3.8 label Jul 29, 2019

pablogsal deleted the fix_opt2 branch May 19, 2021 18:57

Uh oh!

bpo-37500: Make sure dead code does not generate bytecode but also detect syntax errors #14612

bpo-37500: Make sure dead code does not generate bytecode but also detect syntax errors #14612

Uh oh!

Conversation

pablogsal commented Jul 5, 2019 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tim-one commented Jul 6, 2019

Uh oh!

pablogsal commented Jul 6, 2019

Uh oh!

pablogsal commented Jul 6, 2019

Uh oh!

Uh oh!

bedevere-bot commented Jul 14, 2019

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Jul 14, 2019

Choose a reason for hiding this comment

Uh oh!

pablogsal Jul 14, 2019

Choose a reason for hiding this comment

Uh oh!

pablogsal commented Jul 14, 2019

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miss-islington commented Jul 15, 2019

Uh oh!

bedevere-bot commented Jul 15, 2019

Uh oh!

vstinner commented Jul 15, 2019

Uh oh!

vstinner commented Jul 15, 2019

Uh oh!

vstinner commented Jul 15, 2019

Uh oh!

markshannon commented Jul 21, 2019

Uh oh!

miss-islington commented Jul 29, 2019

Uh oh!

bedevere-bot commented Jul 29, 2019

Uh oh!

pablogsal commented Jul 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

pablogsal commented Jul 5, 2019 •

edited by bedevere-bot

Loading

pablogsal commented Jul 29, 2019 •

edited

Loading