bpo-41520: codeop no longer ignores SyntaxWarning #21838
Conversation
|
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9. |
|
@Carreau, thank you for catching this. |
|
GH-21840 is a backport of this pull request to the 3.9 branch. |
(cherry picked from commit 369a1cb) Co-authored-by: Victor Stinner <vstinner@python.org>
|
GH-21841 is a backport of this pull request to the 3.8 branch. |
(cherry picked from commit 369a1cb) Co-authored-by: Victor Stinner <vstinner@python.org>
|
Thanks for the review @csabella! |
| with warnings.catch_warnings(): | ||
| warnings.simplefilter("ignore") | ||
| warnings.simplefilter("error", SyntaxWarning) |
There was a problem hiding this comment.
Ignoring all warnings was intentional. Compile also issues DeprecationWarnings. Narrowing the filter to SyntaxWarning only reintroduces the error of DeprecationWarning being issued thrice, which was fixed by the previous code. I will write more on the issue.
Followup PR fixes regression by removing SyntaxWarning.
There was a problem hiding this comment.
Then maybe the catch warnings need to be put in there 3 times, to reset the warning filter on each subsequent compilations ?
| # bpo-41520: check SyntaxWarning treated as an SyntaxError | ||
| with self.assertRaises(SyntaxError): | ||
| warnings.simplefilter('error', SyntaxWarning) | ||
| compile_command('1 is 1\n', symbol='exec') |
There was a problem hiding this comment.
This source should not have '\n' as single lines are passed without it. That is why to recompile with \n and \n\n added. Fixed in followup PR.
There was a problem hiding this comment.
Even w/o the \n 3.8.5 does not raise the SyntaxError.
$ cat foo.py
import warnings
from codeop import compile_command
warnings.simplefilter('error', SyntaxWarning)
res = compile_command('1 is 1', symbol='exec')
print('Res', res)
$ python foo.py
Res None
There was a problem hiding this comment.
I expect that, but aside from your issue, the test if better if it calls compile_command the way it is intended to be used and is used by code.InteractiveInterpreter.
|
The title is misleading because before this patch, codeop was not ignoring the first SyntaxWarning. The test we added in bpo-40807 ensured that. The issue, rather, is that _maybe_compile requires 3 SyntaxErrors, not just 1, to raise a SyntaxError. As I note above, the fix to a regression introduces another regression. Fixed in #21848. |
https://bugs.python.org/issue41520