From ead10138ef78fd8a47ca8bdbb1c4bc2fb2390f5d Mon Sep 17 00:00:00 2001 From: Mark Dominus Date: Mon, 3 Jun 2024 22:01:50 -0400 Subject: [PATCH 1/2] Remove unused branch in _make_posargs `names_with_default` is never `NULL`, even if there are no names with defaults. In that case it points to a structure with `size` zero. Rather than eliminating the branch, we leave it behind with an `assert(0)` in case a future change to the grammar exercises the branch. --- Parser/action_helpers.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Parser/action_helpers.c b/Parser/action_helpers.c index 3f6c282ffa7a68..c6f1a6b00e0cef 100644 --- a/Parser/action_helpers.c +++ b/Parser/action_helpers.c @@ -555,7 +555,10 @@ _make_posargs(Parser *p, *posargs = _get_names(p, names_with_default); } else if (plain_names != NULL && names_with_default == NULL) { - *posargs = plain_names; + // With the current grammar, we never get here. + // If that has changed, remove the assert, and test thoroughly. + assert(0); + *posargs = plain_names; } else { *posargs = _Py_asdl_arg_seq_new(0, p->arena); From 3cf3f84bf7b2e004e702ca5b9b5fc59eb2d75998 Mon Sep 17 00:00:00 2001 From: Mark Dominus Date: Mon, 3 Jun 2024 22:04:15 -0400 Subject: [PATCH 2/2] Reorganize four-way if-elsif-elsif-elsif as nested if-elses --- Parser/action_helpers.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/Parser/action_helpers.c b/Parser/action_helpers.c index c6f1a6b00e0cef..91b7e2f1058423 100644 --- a/Parser/action_helpers.c +++ b/Parser/action_helpers.c @@ -543,25 +543,30 @@ _make_posargs(Parser *p, asdl_arg_seq *plain_names, asdl_seq *names_with_default, asdl_arg_seq **posargs) { - if (plain_names != NULL && names_with_default != NULL) { - asdl_arg_seq *names_with_default_names = _get_names(p, names_with_default); - if (!names_with_default_names) { - return -1; + + if (names_with_default != NULL) { + if (plain_names != NULL) { + asdl_arg_seq *names_with_default_names = _get_names(p, names_with_default); + if (!names_with_default_names) { + return -1; + } + *posargs = (asdl_arg_seq*)_PyPegen_join_sequences( + p,(asdl_seq*)plain_names, (asdl_seq*)names_with_default_names); + } + else { + *posargs = _get_names(p, names_with_default); } - *posargs = (asdl_arg_seq*)_PyPegen_join_sequences( - p,(asdl_seq*)plain_names, (asdl_seq*)names_with_default_names); - } - else if (plain_names == NULL && names_with_default != NULL) { - *posargs = _get_names(p, names_with_default); - } - else if (plain_names != NULL && names_with_default == NULL) { - // With the current grammar, we never get here. - // If that has changed, remove the assert, and test thoroughly. - assert(0); - *posargs = plain_names; } else { - *posargs = _Py_asdl_arg_seq_new(0, p->arena); + if (plain_names != NULL) { + // With the current grammar, we never get here. + // If that has changed, remove the assert, and test thoroughly. + assert(0); + *posargs = plain_names; + } + else { + *posargs = _Py_asdl_arg_seq_new(0, p->arena); + } } return *posargs == NULL ? -1 : 0; }