Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpo-43897: Reject "_" captures and top-level MatchStar in the AST validator #27432

Merged
merged 3 commits into from
Jul 29, 2021
Merged

bpo-43897: Reject "_" captures and top-level MatchStar in the AST validator #27432

merged 3 commits into from
Jul 29, 2021

Conversation

brandtbucher
Copy link
Member

@brandtbucher brandtbucher commented Jul 28, 2021 •
edited by bedevere-bot
Loading

Also, remove an old comment and fix some recursion depth issues.

https://bugs.python.org/issue43897

@brandtbucher brandtbucher changed the title bpo-43897: Reject "_" captures and top-level MatchStar in the AST validator bpo-43897: Reject "_" captures and top-level MatchStar in the AST validator Jul 28, 2021
isidentical
isidentical reviewed Jul 28, 2021
View reviewed changes
Python/ast.c Outdated
}

static int
validate_pattern(struct validator *state, pattern_ty p, int star_okay)
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Suggested change
validate_pattern(struct validator *state, pattern_ty p, int star_okay)
validate_pattern(struct validator *state, pattern_ty p, int star_ok)

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch!

isidentical
isidentical approved these changes Jul 28, 2021
View reviewed changes
Copy link
Sponsor Member

@isidentical isidentical left a comment

Choose a reason for hiding this comment

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

LGTM (beside star_ok)

@isidentical
Copy link
Sponsor Member

Generally, these issues must be severe enough (e.g. crashes) that they deserve fixing before the final release. All other issues should be deferred to the next development cycle (Python 3.10.1) since stability is the strongest concern at this point

Regarding the backport to 3.10, I think it might make sense to hold this off until 3.10.1 what do you think? Also we should probably notify pablo on the backport PR

@brandtbucher
Copy link
Member Author

That's only after the first RC, right? I think a backport for this would be fine as long as it happened soon.

@pablogsal?

@pablogsal
Copy link
Member

That's only after the first RC, right? I think a backport for this would be fine as long as it happened soon.

@pablogsal?

That's correct, that is only after the first RC. Please, merge this as soon as possible so we can test it enough before we release the RC.

Ideally someone has a fuzzer powerful enough to check the validator 😉

@brandtbucher
Copy link
Member Author

Windows (x86) failure looks unrelated.

@brandtbucher brandtbucher merged commit 8d06474 into python:main Jul 29, 2021
@miss-islington
Copy link
Contributor

Thanks @brandtbucher for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@bedevere-bot
Copy link

GH-27435 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jul 29, 2021
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 29, 2021
@brandtbucher @miss-islington
bpo-43897: Reject "_" captures and top-level MatchStar in the AST val…
796f816 
…idator (pythonGH-27432)

(cherry picked from commit 8d06474)

Co-authored-by: Brandt Bucher <[email protected]>
miss-islington added a commit that referenced this pull request Jul 29, 2021
@miss-islington
[3.10] bpo-43897: Reject "_" captures and top-level MatchStar in the …
405f5c5 
…AST validator (GH-27432) (GH-27435)

(cherry picked from commit 8d06474)


Co-authored-by: Brandt Bucher <[email protected]>

Automerge-Triggered-By: GH:brandtbucher
@brandtbucher brandtbucher deleted the patma-ast-validation branch July 21, 2022 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@isidentical isidentical isidentical approved these changes

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@brandtbucher @isidentical @pablogsal @miss-islington @bedevere-bot @the-knights-who-say-ni