Skip to content

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Oct 6, 2025

Revert GH-131993.

Fix swallowing some syntax warnings in different modules if they accidentally have the same message and are emitted from the same line.

ast.parse() no longer emits syntax warnings for
return/break/continue in finally (see PEP-765) -- they are only emitted during compilation.

Revert pythonGH-131993.

Fix swallowing some syntax warnings in different modules if they accidentally
have the same message and are emitted from the same line.

ast.parse() no longer emits syntax warnings for
return/break/continue in finally (see PEP-765) -- they are only
emitted during compilation.
@serhiy-storchaka serhiy-storchaka force-pushed the compile-syntax-warnings branch from 2fa4266 to c04d874 Compare October 6, 2025 07:34
Copy link
Member

@Eclips4 Eclips4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AST changes are LGTM.
Thank you Serhiy.

@ncoghlan
Copy link
Contributor

ncoghlan commented Oct 7, 2025

Would it be possible to avoid the side effect of delaying the PEP 765 syntax warnings to the code generation stage? Emitting those syntax warnings during AST construction is one of the key reasons I was comfortable suggesting turning them off globally in #139658 when static code analysis was in use.

@serhiy-storchaka
Copy link
Member Author

Only if you are fine with double warnings in REPL or other places that use ast.parse() + compile() instead of just compile().

There are three kinds of warnings, by the emitter:

  • Emitted in the lexer/tokenizer. They are always emitted when you parse the Python sources.
  • Emitted in the AST optimizer. They may be emitted twice, in ast.parse() and compile(). Currently, this is only the PEP 765 warnings.
  • Emitted in the code generator. They are only emitted if you use compile().

So, you already see not all warnings if you only use ast.parse(), without code generation. The fact that the PEP 765 warnings are emitted in the AST optimizer instead of the code generator is just an implementation detail. It could perhaps even be more convenient to emit them in the code generator.

@iritkatriel
Copy link
Member

The fact that the PEP 765 warnings are emitted in the AST optimizer instead of the code generator is just an implementation detail.

Actually I think this was a deliberate decision, we want this to happen during static analysis.

Can we add a kwarg to ast.parse to tell it whether to emit syntax warnings or not?

@serhiy-storchaka
Copy link
Member Author

We can, but

  • This is a new feature. It can perhaps be backported to 3.14.1 as a special exception.
  • Every user of ast.parse() and compile() will need to modify their code to pass the new option. And they will not get all warnings, they will only get the PEP 765 warning. Is it worth a new option?

It is safer to disable that warning by default in ast.parse().

@iritkatriel
Copy link
Member

Every user of ast.parse() and compile() will need to modify their code to pass the new option. And they will not get all warnings, they will only get the PEP 765 warning.

The default should be to give the warning. Turn it off in cpython when we know that the code was just ast.parse'ed (rather than an ast given by the user - we do know that).

@serhiy-storchaka
Copy link
Member Author

What is the name of the option you propose to add?

@iritkatriel
Copy link
Member

Could be with_warnings or something like that.

@serhiy-storchaka
Copy link
Member Author

No, because it should not control all warnings. Maybe _enable_pep_765_warnings?

@serhiy-storchaka
Copy link
Member Author

This would complicated, and I need to revert #131993 because it is a blocker for my other changes.

See yet one option in #139719. It is based on the original solution by @tomasr8.

@iritkatriel
Copy link
Member

Why not all warnings?

@ncoghlan
Copy link
Contributor

ncoghlan commented Oct 7, 2025

The early PEP 765 warnings were indeed specified in the last paragraph of https://peps.python.org/pep-0765/#specification

Edit: that said, I'm open to revisiting that decision and instead letting linters make the call on how to present syntax warnings in their ruleset. My suspicion is that they would end up turning off the parse-time warnings anyway, in which case we don't actually lose anything important by handling the PEP 765 warnings in the code generation step instead of the AST parsing step.

@serhiy-storchaka
Copy link
Member Author

serhiy-storchaka commented Oct 8, 2025

Why not all warnings?

There are three kinds of warnings. #139642 (comment)

If we silence all warnings in ast.parse() or compile(), we will silence not only the PEP 765 warnings, but also warnings emitted in the tokenizer or in the code generator.

Anyway, adding an option for selective silencing requires complex changes will be a complex change comparable to #139652, but with less benefit. I would rather not backport it to 3.14.

So we have a choice for 3.14:

@serhiy-storchaka
Copy link
Member Author

The early PEP 765 warnings were indeed specified in the last paragraph of https://peps.python.org/pep-0765/#specification

This contrasts AST construction and execution of pre-compiled code. But there is a missed step -- the code generation from AST. You cannot normally execute AST.

@serhiy-storchaka serhiy-storchaka changed the title gh-139640: Fix swallowing syntax warnings in different modules gh-139640: Fix swallowing syntax warnings in different modules (no PEP 765 warnings in ast.parse()) Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting merge needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants