bpo-42128: Structural Pattern Matching (PEP 634) #22917

brandtbucher · 2020-10-23T15:39:36Z

PEP 634 has now been accepted, and we'd like to get this into the next alpha.

Several people have volunteered to review the implementation, since it's so huge. Other reviews are very welcome, if anybody has a bit of time to pitch in. This touches tons of stuff: the parser, the compiler, the VM, the builtins, the stdlib, the tests... I'd like as many eyeballs as possible!

https://bugs.python.org/issue42128

This currently segfaults but I still like it

…patma

pablogsal · 2021-02-19T00:32:11Z

when's the latest you'd be comfortable merging this before the release?

Do you mean before the alpha release? Given that a lot of people would be waiting for the next alpha to play with this, we can merge it even on the same day if is required. We can stay in contact via email if it comes to that. Or are you referring to beta freeze?

In any case, I would like to make a review pass, but these weeks I have been fighting lots of stuff and it has been impossible to find some time for it. Hopefully, I can get to it at the weekend :)

brandtbucher · 2021-02-19T00:34:43Z

Gotcha. Yeah, I was referring to the next alpha, scheduled for 3/1. I thought maybe you might need a day or two between tagging and releasing or something.

pablogsal · 2021-02-19T00:48:10Z

I thought maybe you might need a day or two between tagging and releasing or something.

No, although is a multi-hour process, I normally do it on the same day as the release, at least for the alphas (assuming everything goes smoothly).

willingc

@brandtbucher I did an initial pass and it looks good for alpha. I will look at it more closely this weekend.

asottile · 2021-02-20T00:16:17Z

found a bit of an odd case while trying this out -- not sure if it's intentional:

x = 1

match x:
    case 1 as y if 0:
        print('unreachable')
print(f'{y=}')  # expect NameError

instead of NameError I get y=1

pablogsal · 2021-02-20T00:34:15Z

found a bit of an odd case while trying this out -- not sure if it's intentional:
x = 1

match x:
    case 1 as y if 0:
        print('unreachable')
print(f'{y=}')  # expect NameError
instead of NameError I get y=1

IIRC I think that is covered in https://www.python.org/dev/peps/pep-0634/#side-effects-and-undefined-behavior, although the peephole could detect the branch and optimize it away.

brandtbucher · 2021-02-20T00:53:47Z

Yeah, we're required to bind any names used by the guard before evaluating it (obviously). The current naive implementation accomplishes this by binding all names before hitting any guard. Strange example, but legal by the PEP.

I'm actually not sure the peephole/CFG could catch this (it can optimize out the body of the case, though, and it actually does here). Might be something for the AST optimizer.

pablogsal · 2021-02-20T16:42:26Z

Python/ceval.c

+    if (dummy == NULL) {
+        goto fail;
+    }
+    values = PyTuple_New(nkeys);


This is dangerous. You are preparing a tuple with nkeys but then you are filling it calling PyObject_CallFunctionObjArgs, which can execute arbitrary Python code. At this point, the tuple is tracked by the GC so code executed by PyObject_CallFunctionObjArgs could see the tuple half-initialized (the same if there is a gc pass). The same thing applies to anything done meanwhile the tuple is not ready that can call into arbitrary Python.

~~I see. So is filling it with None the right thing to do here, or can we cheat and just untrack/retrack?~~

Actually, filling it with None can lead to problems (as we've seen before). So should we just untrack it temporarily?

I guess we could just fill a list and convert it to a tuple, too.

Okay, I just went with list -> tuple for both. Let me know if you prefer something else.

Another option it to untrack the tuple after its creation and track it back when is ready. Be careful because some APIs can retrack it (like resizing the tuple). I would recommend to add some asserts to check that is untracked where is supposed to be if you decide to go this way.

pablogsal · 2021-02-20T16:42:45Z

Python/ceval.c

+        return NULL;
+    }
+    // So far so good:
+    PyObject *attrs = PyTuple_New(nargs + PyTuple_GET_SIZE(kwargs));


Same as https://github.com/python/cpython/pull/22917/files?file-filters%5B%5D=.asdl&file-filters%5B%5D=.c&file-filters%5B%5D=.h&file-filters%5B%5D=.py#r579678887

pablogsal · 2021-02-20T18:04:00Z

Python/ceval.c

+    // to raise TypeErrors for repeated lookups. On failure, return NULL (with
+    // no error set). Use _PyErr_Occurred(tstate) to disambiguate.
+    assert(PyUnicode_CheckExact(name));
+    // XXX: Why doesn't PySet_CheckExact exist?


#24598

😉

brandtbucher · 2021-02-24T18:43:28Z

I believe I have addressed all of the comments so far. If anyone still has additional feedback (or needs the weekend to review), please let me know. Otherwise, I'll plan on merging this Friday.

@pablogsal, should we hit it with the buildbots again?

bedevere-bot · 2021-02-24T22:25:20Z

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 61616fc 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

brandtbucher · 2021-02-25T16:53:17Z

The three failed jobs look unrelated to these changes; I see the same failures (with the same logs) on #24643.

brandtbucher · 2021-02-26T22:50:55Z

Okay, I'm going to merge this. 🍾

Big thanks to everyone who took time to help out here!

Co-authored-by: Guido van Rossum <guido@python.org> Co-authored-by: Talin <viridia@gmail.com> Co-authored-by: Pablo Galindo <pablogsal@gmail.com>

serhiy-storchaka · 2023-07-01T08:12:05Z

Python/ceval.c

+    PyObject *attr = PyObject_GetAttr(subject, name);
+    if (attr == NULL && _PyErr_ExceptionMatches(tstate, PyExc_AttributeError)) {
+        _PyErr_Clear(tstate);
+    }


It is better to use _PyObject_LookupAttr() here. See #106303.

serhiy-storchaka · 2023-07-01T08:12:32Z

Python/ceval.c

+        else if (_PyErr_ExceptionMatches(tstate, PyExc_AttributeError)) {
+            _PyErr_Clear(tstate);


Same as above.

gvanrossum and others added 30 commits May 25, 2020 16:32

Tentative (incomplete) grammar for match statements

1d39be6

This currently segfaults but I still like it

Add tests for or-patterns

67354ad

Add support for or-patterns

5d609c7

Merge branch 'patma' of https://github.com/brandtbucher/cpython into …

a222351

…patma

Clean up pattern matching compiler

1f8bcbd

Add attribute/name support

eb704d4

Validate name context

5a361bf

Clean up pattern compiler

9c2e61c

Don't bind _

dddcca8

Add sequence tests

af70c74

Add basic sequence support

4191858

Add support for starred sub-patterns in sequences

33633c1

Format tests

a36542e

Merge branch 'master' of https://github.com/python/cpython into patma

d62a426

Simplify control-flow

76d108c

Rename blocks

34c54a0

Add GET_MATCH_ITER opcode

8bc9ae5

Add string tests

2586ee6

Rename and add patma tests

adb04fd

Cleanup

6eb6783

Use Mapping/Sequence _collections_abc

f3c513d

Add missing error handling

fc472c7

Add GET_MATCH_MAP

26dd4fd

Simplify new opcodes

6ce6c34

Bump MAGIC_NUMBER

ea0de16

Add mapping support

880a7c2

Add a bunch of mapping tests, plus cleanup

297ceb2

Reorganize pattern compiler

1902b21

Handle errors when creating new blocks

09e8fdc

Break out _collections_abc stuff in ceval

f612a27

willingc reviewed Feb 19, 2021

View reviewed changes

gvanrossum mentioned this pull request Feb 19, 2021

bpo-42128: Add what's new section for PEP 634 (pattern matching) #24588

Closed

Fix NULL pointer error

69fa421

pablogsal reviewed Feb 20, 2021

View reviewed changes

Don't keep uninitialized tuples hanging around

1711207

pablogsal reviewed Feb 20, 2021

View reviewed changes

brandtbucher added 5 commits February 22, 2021 10:58

Catch up with master

f7c833e

Fix new refleaks ("-R 3:3" is clean again)

7762099

Use PySet_CheckExact (thanks Pablo!)

5e269fb

Move some comments

ca6e332

Catch up with master

61616fc

pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 24, 2021

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 24, 2021

brandtbucher merged commit 145bf26 into python:master Feb 26, 2021

bedevere-bot removed the awaiting changes label Feb 26, 2021

brandtbucher mentioned this pull request Mar 17, 2021

Literal comparison for True, False, None gvanrossum/patma#139

Open

brandtbucher deleted the patma branch July 21, 2022 20:20

serhiy-storchaka reviewed Jul 1, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-42128: Structural Pattern Matching (PEP 634) #22917

bpo-42128: Structural Pattern Matching (PEP 634) #22917

brandtbucher commented Oct 23, 2020 •

edited by gvanrossum

Loading

pablogsal commented Feb 19, 2021

brandtbucher commented Feb 19, 2021 •

edited

Loading

pablogsal commented Feb 19, 2021 •

edited

Loading

willingc left a comment

asottile commented Feb 20, 2021

pablogsal commented Feb 20, 2021

brandtbucher commented Feb 20, 2021 •

edited

Loading

pablogsal Feb 20, 2021 •

edited

Loading

brandtbucher Feb 20, 2021 •

edited

Loading

brandtbucher Feb 20, 2021 •

edited

Loading

brandtbucher Feb 20, 2021

pablogsal Feb 20, 2021

pablogsal Feb 20, 2021

pablogsal Feb 20, 2021

brandtbucher commented Feb 24, 2021

bedevere-bot commented Feb 24, 2021

brandtbucher commented Feb 25, 2021 •

edited

Loading

brandtbucher commented Feb 26, 2021

serhiy-storchaka Jul 1, 2023

serhiy-storchaka Jul 1, 2023

		else if (_PyErr_ExceptionMatches(tstate, PyExc_AttributeError)) {
		_PyErr_Clear(tstate);

bpo-42128: Structural Pattern Matching (PEP 634) #22917

bpo-42128: Structural Pattern Matching (PEP 634) #22917

Conversation

brandtbucher commented Oct 23, 2020 • edited by gvanrossum Loading

pablogsal commented Feb 19, 2021

brandtbucher commented Feb 19, 2021 • edited Loading

pablogsal commented Feb 19, 2021 • edited Loading

willingc left a comment

Choose a reason for hiding this comment

asottile commented Feb 20, 2021

pablogsal commented Feb 20, 2021

brandtbucher commented Feb 20, 2021 • edited Loading

pablogsal Feb 20, 2021 • edited Loading

Choose a reason for hiding this comment

brandtbucher Feb 20, 2021 • edited Loading

Choose a reason for hiding this comment

brandtbucher Feb 20, 2021 • edited Loading

Choose a reason for hiding this comment

brandtbucher Feb 20, 2021

Choose a reason for hiding this comment

pablogsal Feb 20, 2021

Choose a reason for hiding this comment

pablogsal Feb 20, 2021

Choose a reason for hiding this comment

pablogsal Feb 20, 2021

Choose a reason for hiding this comment

brandtbucher commented Feb 24, 2021

bedevere-bot commented Feb 24, 2021

brandtbucher commented Feb 25, 2021 • edited Loading

brandtbucher commented Feb 26, 2021

serhiy-storchaka Jul 1, 2023

Choose a reason for hiding this comment

serhiy-storchaka Jul 1, 2023

Choose a reason for hiding this comment

brandtbucher commented Oct 23, 2020 •

edited by gvanrossum

Loading

brandtbucher commented Feb 19, 2021 •

edited

Loading

pablogsal commented Feb 19, 2021 •

edited

Loading

brandtbucher commented Feb 20, 2021 •

edited

Loading

pablogsal Feb 20, 2021 •

edited

Loading

brandtbucher Feb 20, 2021 •

edited

Loading

brandtbucher Feb 20, 2021 •

edited

Loading

brandtbucher commented Feb 25, 2021 •

edited

Loading