gh-145019: improve SyntaxError when match patterns bind different names

[picnixz]

• Issue: Improve SyntaxError message for inconsistent name binding in OR-patterns #145019

[hugovk]

Nit: rather than "first pattern... pattern X...", let's be consistent.

"second/third/etc pattern" is tricky, so:

Suggested change

	"(first pattern binds %S, pattern %zd binds %S)",
	"(pattern 1 binds %S, pattern %zd binds %S)",

[hugovk]

Suggested change

	SyntaxError: alternative patterns bind different names (first pattern binds no names, pattern 2 binds ['x'])
	>>> match 1:
	... case ("user", {"id": id}) | ("admin", {"name": name}): pass
	Traceback (most recent call last):
	SyntaxError: alternative patterns bind different names (first pattern binds ['id'], pattern 2 binds ['name'])
	>>> match 1:
	... case ("user", {"id": id}) | ("admin", {"id": id}) | ("other", {"ip": ip}): pass
	Traceback (most recent call last):
	SyntaxError: alternative patterns bind different names (first pattern binds ['id'], pattern 3 binds ['ip'])
	SyntaxError: alternative patterns bind different names (pattern 1 binds no names, pattern 2 binds ['x'])
	>>> match 1:
	... case ("user", {"id": id}) | ("admin", {"name": name}): pass
	Traceback (most recent call last):
	SyntaxError: alternative patterns bind different names (pattern 1 binds ['id'], pattern 2 binds ['name'])
	>>> match 1:
	... case ("user", {"id": id}) | ("admin", {"id": id}) | ("other", {"ip": ip}): pass
	Traceback (most recent call last):
	SyntaxError: alternative patterns bind different names (pattern 1 binds ['id'], pattern 3 binds ['ip'])

[picnixz]

I actually wondered whether it was better to display the list as is or use some join to have a string. How do you feel about it?

[hugovk]

Could you give an example?

[picnixz]

Currently:l (I think)

• pattern 1 binds ["a", "b"] but pattern k binds ["u", "v"]

Suggestion:

1. (formatted) pattern 1 binds "a" and "b" but pattern k binds "u" and "v"
2. (minimal) pattern k binds "u' and "v' but pattern 1 does not
3. (minimal swapped) pattern 1 binds "a" and "b" but pattern k does not.
4. If pattern 1 binds a and b and pattern k binds a and c, only report that patten k does not bind b (or vice-versa)

[hugovk]

I think either current or number 1. Current will be easier :)

[picnixz]

Yes current is simple and easier to parse. It may however be better if we remove common entries (unless it is already done).

[picnixz]

Thanks Hugo for the review! I am unavailable this week-end so I will address this next week. If you want to apply your changes, feel free to do so! Otherwise I will do it once I have my laptop.

[pablogsal]

Unless I am missing something here the condition if (PyList_GET_SIZE(control) == 0 || !mismatched_names) has a permanently dead second operand. Both paths to goto diff (lines 6309-6311 and 6321-6323) unconditionally execute mismatched_names = Py_NewRef(pc->stores) before jumping, so mismatched_names is always non-NULL at the diff: label.

Similarly, the ternary at line 6428 (mismatched_names == NULL ? no_names : mismatched_names) always evaluates to mismatched_names.

[pablogsal]

This guy is only substituted when the first pattern (control) has 0 names. When a later pattern binds nothing (e.g., case (a, b) | 1:), mismatched_names is a valid empty list, so the error message would read:

alternative patterns bind different names (first pattern binds ['a', 'b'], pattern 2 binds [])

Showing [] instead of no names is inconsistent with the reverse case

[picnixz]

Oh good catch. I should have increased test coverage. I will put more cases to exercise this path. Thanks!

[pablogsal]

This is freed later before the error: label, but error: itself doesn't free it. Currently safe because the only goto error from diff: is when no_names is NULL. But this is fragile as if future code between allocation and Py_XDECREF(no_names) adds a goto error, it would leak. Other variables (mismatched_names) use the pattern of cleanup in error:. Consider moving no_names to function scope and adding Py_XDECREF(no_names) to the error: block.