Skip to content

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

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

Open
picnixz wants to merge 3 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b2194ac
gh-145019: improve `SyntaxError` when `match` patterns bind different…
picnixz Mar 14, 2026
37d5b5c
Update Lib/test/test_syntax.py
picnixz Mar 14, 2026
5cc7b11
use correct format specified `%d` -> `%zd`
picnixz Mar 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions Lib/test/test_syntax.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1502,6 +1502,33 @@
Traceback (most recent call last):
SyntaxError: invalid syntax

Ensure that alternative patterns bind the same names

>>> match 1:
... case x | 1: pass
Traceback (most recent call last):
SyntaxError: name capture 'x' makes remaining patterns unreachable

>>> match 1:
... case x | y: pass
Traceback (most recent call last):
SyntaxError: name capture 'x' makes remaining patterns unreachable

>>> match 1:
... case 1 | x: ...
Traceback (most recent call last):
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'])
Comment on lines +1520 to +1530
Copy link
Copy Markdown
Member

@hugovk hugovk Mar 30, 2026

Choose a reason for hiding this comment

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

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'])

Copy link
Copy Markdown
Member Author

@picnixz picnixz Apr 3, 2026

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Member

@hugovk hugovk Apr 4, 2026

Choose a reason for hiding this comment

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

Could you give an example?

Copy link
Copy Markdown
Member Author

@picnixz picnixz Apr 4, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

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)

Copy link
Copy Markdown
Member

@hugovk hugovk Apr 4, 2026

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member Author

@picnixz picnixz Apr 4, 2026

Choose a reason for hiding this comment

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

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


Incomplete dictionary literals

>>> {1:2, 3:4, 5}
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core_and_Builtins/2026-03-14-11-20-52.gh-issue-145019.ls2EXo.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Improve :exc:`SyntaxError` when :keyword:`match` alternative patterns bind
different names. Patch by Bénédikt Tran.
26 changes: 24 additions & 2 deletions Python/codegen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6270,6 +6270,8 @@ codegen_pattern_or(compiler *c, pattern_ty p, pattern_context *pc)
NEW_JUMP_TARGET_LABEL(c, end);
Py_ssize_t size = asdl_seq_LEN(p->v.MatchOr.patterns);
assert(size > 1);
PyObject *mismatched_names = NULL;
Py_ssize_t mismatch_index = 0;
// We're going to be messing with pc. Keep the original info handy:
pattern_context old_pc = *pc;
Py_INCREF(pc->stores);
Expand Down Expand Up @@ -6304,6 +6306,8 @@ codegen_pattern_or(compiler *c, pattern_ty p, pattern_context *pc)
control = Py_NewRef(pc->stores);
}
else if (nstores != PyList_GET_SIZE(control)) {
mismatch_index = i;
mismatched_names = Py_NewRef(pc->stores);
goto diff;
}
else if (nstores) {
Expand All @@ -6314,6 +6318,8 @@ codegen_pattern_or(compiler *c, pattern_ty p, pattern_context *pc)
Py_ssize_t istores = PySequence_Index(pc->stores, name);
if (istores < 0) {
PyErr_Clear();
mismatch_index = i;
mismatched_names = Py_NewRef(pc->stores);
goto diff;
}
if (icontrol != istores) {
Expand Down Expand Up @@ -6405,10 +6411,26 @@ codegen_pattern_or(compiler *c, pattern_ty p, pattern_context *pc)
// Pop the copy of the subject:
ADDOP(c, LOC(p), POP_TOP);
return SUCCESS;
diff:
_PyCompile_Error(c, LOC(p), "alternative patterns bind different names");
diff:;
PyObject *no_names = NULL;
if (PyList_GET_SIZE(control) == 0 || !mismatched_names) {
Copy link
Copy Markdown
Member

@pablogsal pablogsal Apr 5, 2026

Choose a reason for hiding this comment

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

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.

no_names = PyUnicode_FromString("no names");
Copy link
Copy Markdown
Member

@pablogsal pablogsal Apr 5, 2026

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member Author

@picnixz picnixz Apr 5, 2026

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

@pablogsal pablogsal Apr 5, 2026

Choose a reason for hiding this comment

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

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.

if (no_names == NULL) {
goto error;
}
}
_PyCompile_Error(
c, LOC(p),
"alternative patterns bind different names "
"(first pattern binds %S, pattern %d binds %S)",
PyList_GET_SIZE(control) == 0 ? no_names : control,
mismatch_index + 1,
mismatched_names == NULL ? no_names : mismatched_names
);
Py_XDECREF(no_names);
error:
PyMem_Free(old_pc.fail_pop);
Py_XDECREF(mismatched_names);
Py_DECREF(old_pc.stores);
Py_XDECREF(control);
return ERROR;
Expand Down
Loading