Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-131927: Prevent emitting optimizer warnings twice in the REPL#131993
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
2bd058208eab522049da3a922e16742a4b1bbefbf8d047dd3c4a27bc7f8d1afc15635b97d028ea8fa07fFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import contextlib | ||
| import io | ||
| import unittest | ||
| import warnings | ||
| from unittest.mock import patch | ||
| from textwrap import dedent | ||
| @@ -273,3 +274,28 @@ def test_incomplete_statement(self): | ||
| code = "if foo:" | ||
| console = InteractiveColoredConsole(namespace, filename="<stdin>") | ||
| self.assertTrue(_more_lines(console, code)) | ||
| class TestWarnings(unittest.TestCase): | ||
| def test_pep_765_warning(self): | ||
| """ | ||
| Test that a SyntaxWarning emitted from the | ||
| AST optimizer is only shown once in the REPL. | ||
| """ | ||
| # gh-131927 | ||
| console = InteractiveColoredConsole() | ||
| code = dedent("""\ | ||
| def f(): | ||
| try: | ||
| return 1 | ||
| finally: | ||
| return 2 | ||
| """) | ||
| with warnings.catch_warnings(record=True) as caught: | ||
| warnings.simplefilter("default") | ||
| console.runsource(code) | ||
| count = sum("'return' in a 'finally' block" in str(w.message) | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can use "'return' in a 'finally' block" may be an error in future versions. Maybe use some more long living warnings, like MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Hmm I don't think I can, unless I want to match the warning message exactly?
I can't if I want to test the new repl. The PEP 765 warning is the only one emitted from the ast optimizer which is needed to trigger this behaviour in the repl. However this does change the behaviour of all warnings that use | ||
| for w in caught) | ||
| self.assertEqual(count, 1) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1479,6 +1479,28 @@ PyErr_WarnExplicitObject(PyObject *category, PyObject *message, | ||
| return 0; | ||
| } | ||
| /* Like PyErr_WarnExplicitObject, but automatically sets up context */ | ||
| int | ||
| _PyErr_WarnExplicitObjectWithContext(PyObject *category, PyObject *message, | ||
| PyObject *filename, int lineno) | ||
| { | ||
| PyObject *unused_filename, *module, *registry; | ||
| int unused_lineno; | ||
| int stack_level = 1; | ||
| if (!setup_context(stack_level, NULL, &unused_filename, &unused_lineno, | ||
| &module, ®istry)){ | ||
| return -1; | ||
| } | ||
| int rc = PyErr_WarnExplicitObject(category, message, filename, lineno, | ||
| module, registry); | ||
| Py_DECREF(unused_filename); | ||
| Py_DECREF(registry); | ||
| Py_DECREF(module); | ||
tomasr8 marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| return rc; | ||
| } | ||
| int | ||
| PyErr_WarnExplicit(PyObject *category, const char *text, | ||
| const char *filename_str, int lineno, | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if set it to "always"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It emits twice, I added a test to
test_compilefor it