[proxy] web.archive.org — ← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ cpython Public

Conversation

Copy link
Member

pablogsal commented Mar 18, 2019 •
edited by bedevere-bot
Loading

Copy link
Member Author

pablogsal commented Mar 18, 2019

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) {
     ^~~~~~

vstinner reviewed Mar 18, 2019
View reviewed changes
Python/ast.c Outdated
Copy link
Member

vstinner Mar 18, 2019

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 :-)

Copy link
Member Author

pablogsal Mar 18, 2019

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

Copy link
Member

vstinner Mar 18, 2019

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.

serhiy-storchaka approved these changes Mar 18, 2019
View reviewed changes
Lib/test/test_ast.py Outdated
Copy link
Member

serhiy-storchaka Mar 18, 2019

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?

Copy link
Member Author

pablogsal Mar 18, 2019

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.

pablogsal force-pushed the bpo36332 branch 3 times, most recently from 3fd32ec to b71484a Compare March 18, 2019 11:09
serhiy-storchaka reviewed Mar 18, 2019
View reviewed changes
Lib/test/test_ast.py Outdated
# Decorator with generator argument
"@deco(a for a in b)\ndef f(): pass",
# Simple assignment expression
"if a:=1:\n True"
Copy link
Member

serhiy-storchaka Mar 18, 2019

Choose a reason for hiding this comment

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

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

vstinner approved these changes Mar 18, 2019
View reviewed changes
Copy link
Member

vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

pablogsal merged commit 0c9258a into python:master Mar 18, 2019
pablogsal deleted the bpo36332 branch March 18, 2019 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

serhiy-storchaka serhiy-storchaka approved these changes

vstinner vstinner approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants