Hi Brandt, On 02/04/2021 7:19 am, Brandt Bucher wrote:
Guido van Rossum wrote:
Well, now I have egg on my face, because the current implementation does reject multiple occurrences of the same identifier in __match_args__. We generate an error like "TypeError: C() got multiple sub-patterns for attribute 'a'". However, I cannot find this uniqueness requirement in PEP 634, so I think it was a mistake to implement it.
Researching this led me to find another issue where PEP 634 and the implementation differ, but this time it's the other way around: PEP 634 says about types which accept a single positional subpattern (int(x), str(x) etc.) "for these types no keyword patterns are accepted." Mark's example `case int(real=0, imag=0):` makes me think this requirement is wrong and I would like to amend PEP 634 to strike this requirement. Fortunately, this is not what is implemented. E.g. `case int(1, real=1):` is accepted and works, as does `case int(real=0):`.
Calling out Brandt to get his opinion. And thanks to Mark for finding these!
The current implementation will reject any attribute being looked up more than once, by position *or* keyword. It's actually a bit tricky to do, which is why the `MATCH_CLASS` op is such a beast... it needs to look up positional and keyword attributes all in one go, keeping track of everything it's seen and checking for duplicates.
I believe this behavior is a holdover from PEP 622:
The interpreter will check that two match items are not targeting the same attribute, for example `Point2d(1, 2, y=3)` is an error.
(https://www.python.org/dev/peps/pep-0622/#overlapping-sub-patterns)
PEP 634 explicitly disallows duplicate keywords, but as far as I can tell it says nothing about duplicate `__match_args__` or keywords that also appear in `__match_args__`. It looks like an accidental omission during the 622 -> 634 rewrite.
(I guess I figured that if somebody matches `Spam(foo, y=bar)`, where `Spam.__match_args__` is `("y",)`, that's probably a bug in the user's code. Ditto for `Spam(y=foo, y=bar)` and `Spam(foo, bar)` where `Spam.__match_args__` is `("y", "y")` But it's not a hill I'm willing to die on.)
Repeated keywords do seem likely to be a bug. Most checks are cheap though. Checking for duplicates in `__match_args__` can be done at class creation time, and checking for duplicates in the pattern can be done at compile time. So how about explicitly disallowing those, but not checking that the intersection of `__match_args__` and keywords is empty? We would get most of the error checking without the performance impact.
I agree that self-matching classes should absolutely allow keyword matches. I had no idea the PEP forbade it.
PEP 634 allows it. PEP 653 currently forbids it, mainly for consistency reasons. The purpose of self-matching is to prevent deconstruction, so it seems inconsistent to allow it for keyword arguments. Are there are any use-cases? The test-case `int(real=0+0j, imag=0-0j)` is contrived, but I'm struggling to come up with less contrived examples for any of float, list, dict, tuple, str. Cheers, Mark.