bpo-44589: raise SyntaxError when mapping pattern has duplicate key by jdevries3133 · Pull Request #27131 · python/cpython

jdevries3133 · 2021-07-14T03:35:16Z

match case pattern cannot have duplicate keys per PEP 634.
when duplicate literal values are present, a syntax error should be
raised; not a runtime ValueError, which was the prior behavior.

Tests have been updated to test the new behavior. Docs were not changed, because the behavior is now more closely aligned with the documentation and the PEP.

https://bugs.python.org/issue44589

* match case pattern cannot have duplicate keys per PEP 634. * when duplicate literal values are present, a syntax error should be raised; not a runtime ValueError, which was the prior behavior.

Fidget-Spinner

Looks good to me in general. I'm not a compiler expert though. Thanks for taking this up :).

Python/compile.c

Fidget-Spinner · 2021-07-14T09:39:01Z

The docs at https://docs.python.org/3.11/reference/compound_stmts.html#mapping-patterns also need fixing.
Specifically the lines:

Duplicate key values in mapping patterns are disallowed. (If all key patterns are literal patterns this is considered a syntax error; otherwise this is a runtime error and will raise ValueError.)

and

If duplicate keys are detected in the mapping pattern, the pattern is considered invalid and ValueError is raised.

require more clarification.

The file for that doc is at https://github.com/python/cpython/blob/main/Doc/reference/compound_stmts.rst

jdevries3133 · 2021-07-14T16:02:02Z

@Fidget-Spinner re: Documentation, I tweaked the docs. See aeb4c42

Fidget-Spinner

LGTM. Great job!

Python/compile.c

bedevere-bot · 2021-07-14T16:46:47Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>

jdevries3133 · 2021-07-14T18:08:09Z

Thank you very much for your feedback!! I have made the requested changes; please review again.

bedevere-bot · 2021-07-14T18:08:13Z

Thanks for making the requested changes!

@brandtbucher: please review the changes made to this pull request.

brandtbucher

Nice, thanks for the quick changes.

I have a few minor suggestions, but I consider this merge-ready and will land it EOD regardless.

Python/compile.c

Misc/NEWS.d/next/Core and Builtins/2021-07-13-23-19-41.bpo-44589.59OH8T.rst

jdevries3133 · 2021-07-14T19:14:27Z

@brandtbucher ok, those final changes have all been made in 00886a5

Python/compile.c

bedevere-bot · 2021-07-14T19:49:07Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

jdevries3133 · 2021-07-14T20:14:47Z

I have made the requested changes; please review again.

bedevere-bot · 2021-07-14T20:14:50Z

Thanks for making the requested changes!

@brandtbucher: please review the changes made to this pull request.

miss-islington · 2021-07-15T00:38:45Z

Thanks @jdevries3133 for the PR, and @brandtbucher for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

bedevere-bot · 2021-07-15T00:38:53Z

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

…iteral keys (pythonGH-27131) (cherry picked from commit 2693132) Co-authored-by: Jack DeVries <58614260+jdevries3133@users.noreply.github.com>

…icate literal keys (GH-27131) (GH-27157) (cherry picked from commit 2693132) Co-authored-by: Jack DeVries <58614260+jdevries3133@users.noreply.github.com> Automerge-Triggered-By: GH:brandtbucher

bpo-44589: raise SyntaxError when mapping pattern has duplicate key

78c93dc

* match case pattern cannot have duplicate keys per PEP 634. * when duplicate literal values are present, a syntax error should be raised; not a runtime ValueError, which was the prior behavior.

jdevries3133 requested a review from markshannon as a code owner July 14, 2021 03:35

bedevere-bot added the awaiting review label Jul 14, 2021

the-knights-who-say-ni added the CLA signed label Jul 14, 2021

jdevries3133 marked this pull request as draft July 14, 2021 03:43

add edge case test cases where zero is the key

082ded0

jdevries3133 marked this pull request as ready for review July 14, 2021 03:58

Fidget-Spinner reviewed Jul 14, 2021

View reviewed changes

Python/compile.c Outdated Show resolved Hide resolved

jdevries3133 added 2 commits July 14, 2021 12:00

fix potential memory leak in exit branches

3a96d32

update documentation

aeb4c42

Fidget-Spinner approved these changes Jul 14, 2021

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Jul 14, 2021

fix: undo accidental reformatting of code

1219598

brandtbucher self-requested a review July 14, 2021 16:13

brandtbucher added the needs backport to 3.10 only security fixes label Jul 14, 2021

brandtbucher requested changes Jul 14, 2021

View reviewed changes

Python/compile.c Outdated Show resolved Hide resolved

Python/compile.c Outdated Show resolved Hide resolved

Python/compile.c Outdated Show resolved Hide resolved

Python/compile.c Outdated Show resolved Hide resolved

Python/compile.c Outdated Show resolved Hide resolved

bedevere-bot added awaiting changes and removed awaiting core review labels Jul 14, 2021

jdevries3133 and others added 5 commits July 14, 2021 10:40

Update Python/compile.c

850a68c

Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>

Update Python/compile.c

07b038f

Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>

Update Python/compile.c

becefd8

Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>

Update Python/compile.c

394d28a

Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>

refactor to use goto statements

cd57712

bedevere-bot added awaiting change review and removed awaiting changes labels Jul 14, 2021

bedevere-bot requested a review from brandtbucher July 14, 2021 18:08

brandtbucher approved these changes Jul 14, 2021

View reviewed changes

Python/compile.c Outdated Show resolved Hide resolved

Python/compile.c Outdated Show resolved Hide resolved

Python/compile.c Show resolved Hide resolved

Misc/NEWS.d/next/Core and Builtins/2021-07-13-23-19-41.bpo-44589.59OH8T.rst Outdated Show resolved Hide resolved

bedevere-bot added awaiting merge and removed awaiting change review labels Jul 14, 2021

cleanup and finalization

00886a5

brandtbucher requested changes Jul 14, 2021

View reviewed changes

Python/compile.c Outdated Show resolved Hide resolved

bedevere-bot removed the awaiting merge label Jul 14, 2021

bedevere-bot added the awaiting changes label Jul 14, 2021

fix: hidden possible memory leak in macro

a8e6751

bedevere-bot added awaiting change review and removed awaiting changes labels Jul 14, 2021

bedevere-bot requested a review from brandtbucher July 14, 2021 20:14

brandtbucher merged commit 2693132 into python:main Jul 15, 2021

bedevere-bot removed awaiting change review needs backport to 3.10 only security fixes labels Jul 15, 2021

pablogsal mentioned this pull request Jul 18, 2021

[3.10] Correct the order of regen-abidump #27228

Closed

Uh oh!

Conversation

jdevries3133 commented Jul 14, 2021 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fidget-Spinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Fidget-Spinner commented Jul 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdevries3133 commented Jul 14, 2021

Uh oh!

Fidget-Spinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented Jul 14, 2021

Uh oh!

jdevries3133 commented Jul 14, 2021

Uh oh!

bedevere-bot commented Jul 14, 2021

Uh oh!

brandtbucher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jdevries3133 commented Jul 14, 2021

Uh oh!

Uh oh!

bedevere-bot commented Jul 14, 2021

Uh oh!

jdevries3133 commented Jul 14, 2021

Uh oh!

bedevere-bot commented Jul 14, 2021

Uh oh!

miss-islington commented Jul 15, 2021

Uh oh!

bedevere-bot commented Jul 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jdevries3133 commented Jul 14, 2021 •

edited by bedevere-bot

Loading

Fidget-Spinner commented Jul 14, 2021 •

edited

Loading