bpo-39562: Prevent collision of future and compiler flags by isidentical · Pull Request #19230 · python/cpython · GitHub

https://bugs.python.org/issue39562

Would it be possible to pass future and compiler flags as two different variables to avoid of global unicity? I don't think so, but I ask, just in case :-)

How many bits are free for incoming compiler flags? Two? 0x4000 and 0x8000?

and 0x10000 (now that we removed that old macro)

Would it be possible to pass future and compiler flags as two different variables to avoid of global unicity? I don't think so, but I ask, just in case :-)

It is hard because of public APIs use it in this way, including python level ones (e.g: compile function). But, I can work on a patch to do this in a period (3.9, add a new parameter to compile for future flags (compile(*args, flags, future_flags)), giving future flags from flags parameter would give a deprecation warning, for distinguishing those we still need this patch;3.10, we'll increase that deprecation's level;3.11 we'll only accept compiler flags from flags and take future flags from future_flags. Just a thought but if seen suitable, I can create a patch before the alpha cut.

It is hard because of public APIs use it in this way, including python level ones (e.g: compile function). But, I can work on a patch to do this in a period (3.9, add a new parameter to compile for future flags (compile(*args, flags, future_flags)), giving future flags from flags parameter would give a deprecation warning, for distinguishing those we still need this patch;3.10, we'll increase that deprecation's level;3.11 we'll only accept compiler flags from flags and take future flags from future_flags. Just a thought but if seen suitable, I can create a patch before the alpha cut.

That would show the deprecation warning to a lot of people and then it will break them after we deprecate it. Many users have complained before of this exact scenario so I would say is not a good idea to do this.

I think we can certainly add a second parameter to encourage people to separate them in the future but I think we will not be able to deprecate this without many users complaining. :(

Just a guess but I'm not sure much people is using compile with a future flags manually instead of inserting a future statement to first line. Still need to prove it with analyzing some pypi packages (tonight I can share some results)

Just a guess but I'm not sure much people is using compile with a future flags manually instead of inserting a future statement to first line. Still need to prove it with analyzing some pypi packages (tonight I can share some results)

That would be a good idea 👍

Co-Authored-By: Victor Stinner <vstinner@python.org>

LGTM.

@pablogsal: Would you mind to double check the PR?

@isidentical: I took the liberty to commit directly my last change request. Thanks for updating your PR multiple times, it now looks better ;-)

@isidentical: Would you mind to fix the conflict in Doc/whatsnew/3.9.rst? You may rebase your branch on master, or merge master into your branch.

Done @vstinner

Thank you for merge and reviews @vstinner. Currently working on a PoC implementation to split these 2 flags (will share about my research on usage of flags= with future flags. on the issue)

Thank you for merge and reviews @vstinner. Currently working on a PoC implementation to split these 2 flags (will share about my research on usage of flags= with future flags. on the issue)

Honestly, I don't think that it's worth it. The bug is fixed, there is no need to modify dozens of functions accepting compiler flags, just for that.

Well I dont think it will change that much, but I'm still going to create a PoC to see how much it will change.

My attempt to add -X noopt adds a new parameter to compile(): my PR modifies not less than 42 files... I closed my PR because of that, the change was too intrusive.
#13600

@pablogsal suggested me to use the value -1 to mean "disable all optimizations". I didn't try this approach, but I'm no longer interested to work on https://bugs.python.org/issue2506

Thanks @isidentical for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

Sorry, @isidentical and @vstinner, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 4454057269b995341b04d13f0bf97f96080f27d0 3.8

I'll send a manual cherry pick, tonight.

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

Oh, @pablogsal already did one :)

Oh, @pablogsal already did one :)

Fastest gun in this side of the west 🔫

🔫 🔫

…onGH-19230) The constant values of future flags in the __future__ module is updated in order to prevent collision with compiler flags. Previously PyCF_ALLOW_TOP_LEVEL_AWAIT was clashing with CO_FUTURE_DIVISION.. (cherry picked from commit 4454057) Co-authored-by: Batuhan Taşkaya <batuhanosmantaskaya@gmail.com>

…9230) (GH-19835) The constant values of future flags in the __future__ module is updated in order to prevent collision with compiler flags. Previously PyCF_ALLOW_TOP_LEVEL_AWAIT was clashing with CO_FUTURE_DIVISION.. (cherry picked from commit 4454057) Co-authored-by: Batuhan Taşkaya <batuhanosmantaskaya@gmail.com>

the-knights-who-say-ni added the CLA signed label Mar 30, 2020

bedevere-bot added the awaiting review label Mar 30, 2020

isidentical added the skip news label Mar 30, 2020

pablogsal reviewed Mar 30, 2020

View reviewed changes

Comment thread Lib/test/test_future.py Outdated

Comment thread Lib/__future__.py Outdated

pablogsal requested a review from serhiy-storchaka March 30, 2020 19:16

vstinner reviewed Apr 20, 2020

View reviewed changes

isidentical added 5 commits April 20, 2020 21:52

bpo-39562: Prevent collision of future and compiler flags

984c815

Add support to compile for PyCF_ALLOW_TOP_LEVEL_AWAIT

8d6d207

Pack valid compile flags to a mask

ebf0003

automatically discover compiler flags from ast

c14888d

Mention about change, describe tests, remove old macro

7cd943a

vstinner reviewed Apr 20, 2020

View reviewed changes

Comment thread Include/code.h Outdated

Comment thread Include/code.h Outdated

Comment thread Lib/test/test_future.py Outdated

Comment thread Lib/test/test_future.py Outdated

vstinner reviewed Apr 20, 2020

View reviewed changes

Comment thread Doc/whatsnew/3.9.rst Outdated

vstinner reviewed Apr 20, 2020

View reviewed changes

Comment thread Doc/whatsnew/3.9.rst Outdated

isidentical and others added 2 commits April 21, 2020 02:27

Apply suggestions

d1094d6

Co-Authored-By: Victor Stinner <vstinner@python.org>

Mention clashing variables, remove old comment

560a100

isidentical force-pushed the bpo-39562-2 branch from e286f29 to 560a100 Compare April 20, 2020 23:29

isidentical requested a review from vstinner April 20, 2020 23:33

vstinner reviewed Apr 21, 2020

View reviewed changes

Comment thread Include/code.h Outdated

Comment thread Lib/test/test_future.py Outdated

Comment thread Lib/test/test_future.py Outdated

Add a noticement header, use dicts for tests

5f8018b

vstinner reviewed Apr 21, 2020

View reviewed changes

Comment thread Include/compile.h Outdated

Update Include/compile.h

b225c61

vstinner approved these changes Apr 21, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Apr 21, 2020

Merge branch 'master' into bpo-39562-2

0b8936e

vstinner merged commit 4454057 into python:master Apr 22, 2020

bedevere-bot removed the awaiting merge label Apr 22, 2020

isidentical mentioned this pull request Apr 22, 2020

bpo-40366: Remove support for passing obsolete flags into compile #19660

Closed

pablogsal added the needs backport to 3.8 label May 1, 2020

miss-islington assigned vstinner May 1, 2020

bedevere-bot removed the needs backport to 3.8 label May 1, 2020

bpo-39562: Prevent collision of future and compiler flags by isidentical · Pull Request #19230 · python/cpython · GitHub

Uh oh!

Uh oh!

bpo-39562: Prevent collision of future and compiler flags - #19230

bpo-39562: Prevent collision of future and compiler flags#19230vstinner merged 10 commits intopython:masterpython/cpython:masterfrom isidentical:bpo-39562-2isidentical/cpython:bpo-39562-2Copy head branch name to clipboard

Conversation

isidentical commented Mar 30, 2020 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner Apr 20, 2020

Choose a reason for hiding this comment

Uh oh!

isidentical Apr 20, 2020

Choose a reason for hiding this comment

Uh oh!

isidentical commented Apr 20, 2020

Uh oh!

pablogsal commented Apr 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

isidentical commented Apr 20, 2020

Uh oh!

pablogsal commented Apr 20, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Apr 22, 2020

Uh oh!

isidentical commented Apr 22, 2020

Uh oh!

isidentical commented Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner commented Apr 22, 2020

Uh oh!

isidentical commented Apr 22, 2020

Uh oh!

vstinner commented Apr 22, 2020

Uh oh!

miss-islington commented May 1, 2020

Uh oh!

miss-islington commented May 1, 2020

Uh oh!

isidentical commented May 1, 2020

Uh oh!

bedevere-bot commented May 1, 2020

Uh oh!

isidentical commented May 1, 2020

Uh oh!

pablogsal commented May 1, 2020

Uh oh!

isidentical commented May 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

bpo-39562: Prevent collision of future and compiler flags#19230
vstinner merged 10 commits into
python:masterfrom
isidentical:bpo-39562-2

isidentical commented Mar 30, 2020 •
edited by bedevere-bot
Loading

pablogsal commented Apr 20, 2020 •
edited
Loading

isidentical commented Apr 22, 2020 •
edited
Loading