bpo-36332: Allow compile() to handle AST objects with assignment expressions by pablogsal · Pull Request #12398 · python/cpython

bpo-36332: Allow compile() to handle AST objects with assignment expressions by pablogsal · Pull Request #12398 · python/cpython

As @serhiy-storchaka suggested, I have moved the default cause of the switch outside so the compiler now emits warnings if there are missing cases. For example:

Python/ast.c: In function ‘validate_expr’:
Python/ast.c:226:5: warning: enumeration value ‘NamedExpr_kind’ not handled in switch [-Wswitch]
     switch (exp->kind) {
     ^~~~~~

Choose a reason for hiding this comment

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

nitpick: I prefer the current "default:" case coding style :-)

Choose a reason for hiding this comment

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

The advantage of having it outside the switch is that the compiler complains about missing cases. Check my previous comment and Serhiy's suggestion in the bpo

Choose a reason for hiding this comment

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

Ah ok, I see. It makes sense.

Choose a reason for hiding this comment

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

Why not add it to exec_tests or single_tests?

Choose a reason for hiding this comment

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

I was afraid that the diff will be very difficult to read because the regenerated exec_results, but it turned out to be very simple. I have added the test to exec_results instead of creating a new extra_validation_snippets.

		# Decorator with generator argument
		"@deco(a for a in b)\ndef f(): pass",
		# Simple assignment expression
		"if a:=1:\n True"

Choose a reason for hiding this comment

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

Minimal example: "(a := 1)".

Choose a reason for hiding this comment

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

LGTM.