bpo-39562: Prevent collision of future and compiler flags #19230
Conversation
| @@ -24,6 +24,8 @@ PyAPI_FUNC(PyCodeObject *) PyNode_Compile(struct _node *, const char *); | |||
| #define PyCF_IGNORE_COOKIE 0x0800 | |||
| #define PyCF_TYPE_COMMENTS 0x1000 | |||
| #define PyCF_ALLOW_TOP_LEVEL_AWAIT 0x2000 | |||
There was a problem hiding this comment.
How many bits are free for incoming compiler flags? Two? 0x4000 and 0x8000?
There was a problem hiding this comment.
and 0x10000 (now that we removed that old macro)
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 |
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) |
That would be a good idea 👍 |
Co-Authored-By: Victor Stinner <vstinner@python.org>
e286f29 to
560a100
Compare
There was a problem hiding this comment.
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 |
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. @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 |
|
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 :) |
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>
https://bugs.python.org/issue39562