Compilation emits multiple warnings in the finally block

[sobolevn]

Bug report

This code produces two SyntaxWarnings in the new REPL:

>>> def some():
... try:
... return 1
... finally:
... return 2
... 
<python-input-14>:5: SyntaxWarning: 'return' in a 'finally' block
<python-input-14>:5: SyntaxWarning: 'return' in a 'finally' block

But, in our old REPL it produces just one:

Other SyntaxWarnings also do not show the same behavior in the new REPL. Example:

So, looks like only PEP-765 is affected, only in the new REPL.

CC @ambv @iritkatriel

Linked PRs

• gh-131927: Prevent emitting optimizer warnings twice in the REPL #131993
• [3.13] gh-131927: Prevent emitting optimizer warnings twice in the REPL (GH-131993) #132463
• gh-131927: Do not emit PEP 765 warnings in ast.parse() #139642
• gh-131927: Silence PEP 765 warnings in PyREPL #139719
• [3.14] gh-131927: Do not emit PEP 765 warnings in ast.parse() (GH-139642) #140786

[tomasr8]

I think the issue is that we compile the source twice, once we compile to an AST and second time from the AST to a code object. The warning comes from the AST optimizer so it is emitted both times:

cpython/Lib/_pyrepl/console.py

Lines 190 to 209 in 6aa88a2

	try:
	tree = self.compile.compiler(
	source,
	filename,
	"exec",
	ast.PyCF_ONLY_AST,
	incomplete_input=False,
	)
	except (SyntaxError, OverflowError, ValueError):
	self.showsyntaxerror(filename, source=source)
	return False
	if tree.body:
	*_, last_stmt = tree.body
	for stmt in tree.body:
	wrapper = ast.Interactive if stmt is last_stmt else ast.Module
	the_symbol = symbol if stmt is last_stmt else "exec"
	item = wrapper([stmt])
	try:
	code = self.compile.compiler(item, filename, the_symbol)
	linecache._register_code(code, source, filename)

A very simple fix for this would be to catch this warning on the second compilation, something like this:

diff --git a/Lib/_pyrepl/console.py b/Lib/_pyrepl/console.py
index 8956fb1242..9d461980be 100644
--- a/Lib/_pyrepl/console.py
+++ b/Lib/_pyrepl/console.py
@@ -204,8 +204,26 @@ def runsource(self, source, filename="<input>", symbol="single"):
 wrapper = ast.Interactive if stmt is last_stmt else ast.Module
 the_symbol = symbol if stmt is last_stmt else "exec"
 item = wrapper([stmt])
+ import warnings
 try:
- code = self.compile.compiler(item, filename, the_symbol)
+ with warnings.catch_warnings(record=True) as caught_warnings:
+ # Enable all warnings
+ warnings.simplefilter("always")
+ code = self.compile.compiler(item, filename, the_symbol)
+ for warning in caught_warnings:
+ if issubclass(warning.category, SyntaxWarning) and "in a 'finally' block" in str(warning.message):
+ # Ignore this warning as it would've alread been raised
+ # when compiling the code above
+ pass
+ else:
+ # Re-emit other warnings
+ warnings.warn_explicit(
+ message=warning.message,
+ category=warning.category,
+ filename=warning.filename,
+ lineno=warning.lineno,
+ source=warning.source
+ )
 linecache._register_code(code, source, filename)
 except SyntaxError as e:
 if e.args[0] == "'await' outside function":

I can send a PR for this later today unless you were already working on it @sobolevn ?

[mdboom]

Cc: @iritkatriel

[iritkatriel]

Why does the repl need the ast?

[sobolevn]

No, I don't plan to work on this issue, because I don't know how to solve it :)

[iritkatriel]

Why does the repl need the ast?

Looks like it's walking the AST and then constructing an AST for each statement on its own and executing that. Not sure why.

I don't like the solution proposed above (suppressing the warnings) because I think we will add other syntax warnings to this stage later on (including moving some that are not in the compiler so that they show up in static analysis), and I don't want us to have to special case each one of them. Let's think of something else.

[tomasr8]

Indeed, the solution I proposed is pretty brittle, especially if we're planning to add more warnings in the future. I'll see if I can come up with another solution.

[iritkatriel]

One thing we could do is to define a subclass of SyntaxWarning which is only raised from the ast_opt stage, and then this type can be filtered out.

(note that ast_opt is in the process of being renamed in #131830 (comment), so maybe wait till that's merged).

[tomasr8]

That sounds like a good option! I'll wait for the other PR to land :)

[iritkatriel]

Actually, adding a new builtin warning type would need a PEP, and this is probably overkill for this problem.

Maybe we can just mark the SyntaxWarning as "coming from ast_opt" in some way that the real can query?

[tomasr8]

Or a way to turn off warnings coming from ast_opt? Though that also seems like overkill for such a niche use case.