bpo-45834: Move runtime 'except:' check to the parser by pablogsal · Pull Request #29625 · python/cpython
LGTM based on the tests but I’m not a good reviewer for the parser.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
| if (!handler->v.ExceptHandler.type && i < n-1) { | ||
| return compiler_error(c, "default 'except:' must be last"); | ||
| } | ||
| assert(handler->v.ExceptHandler.type || i >= n-1); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we also need to add this check to the AST validator? (since it might lead to crashes when compiling manually crafted AST objects).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, what makes me think that maybe this change is not worth it because I find this PR much more complex than what we currently have
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before making a decision about this PR, could you please look at the parser change for except* on #29581 ?
Let's close this if it's not worth doing.