gh-153568: Share expression parsing with assignment targets (#157450)

* gh-153568: Share expression parsing with assignment targets

* Clarify target parsing contracts and AST expectations

Author: pablogsal

Grammar/python.gram

@@ -157,7 +157,7 @@ assignment[stmt_ty]:
             _PyAST_AnnAssign(CHECK(expr_ty, _PyPegen_set_expr_context(p, a, Store)), b, c, 1, EXTRA)
         ) }
     | a=('(' b=single_target ')' { b }
-         | single_subscript_attribute_target) ':' b=expression c=['=' d=annotated_rhs { d }] {
+         | attribute_or_subscript_target) ':' b=expression c=['=' d=annotated_rhs { d }] {
         CHECK_VERSION(stmt_ty, 6, "Variable annotations syntax is", _PyAST_AnnAssign(a, b, c, 0, EXTRA)) }
     | a[asdl_expr_seq*]=(z=star_targets '=' { z })+ b=annotated_rhs !'=' tc=[TYPE_COMMENT] {
          _PyAST_Assign(a, b, NEW_TYPE_COMMENT(p, tc), EXTRA) }
@@ -740,16 +740,10 @@ star_expression[expr_ty] (memo):
 
 star_named_expressions[asdl_expr_seq*]: a[asdl_expr_seq*]=','.star_named_expression+ [','] { a }
 
-star_named_expressions_sequence[asdl_expr_seq*]: a[asdl_expr_seq*]=','.star_named_expression_sequence+ [','] { a }
-
 star_named_expression[expr_ty]:
     | '*' a=bitwise_or { _PyAST_Starred(a, Load, EXTRA) }
     | named_expression
 
-star_named_expression_sequence[expr_ty]:
-    | invalid_starred_expression_unpacking_sequence
-    | star_named_expression
-
 assignment_expression[expr_ty]:
     | a=NAME ':=' ~ b=expression {
         CHECK_VERSION(expr_ty, 8, "Assignment expressions are",
@@ -791,28 +785,17 @@ comparison[expr_ty]:
     | bitwise_or
 
 compare_op_bitwise_or_pair[CmpopExprPair*]:
-    | eq_bitwise_or
-    | noteq_bitwise_or
-    | lte_bitwise_or
-    | lt_bitwise_or
-    | gte_bitwise_or
-    | gt_bitwise_or
-    | notin_bitwise_or
-    | in_bitwise_or
-    | isnot_bitwise_or
-    | is_bitwise_or
-
-eq_bitwise_or[CmpopExprPair*]: '==' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, Eq, a) }
-noteq_bitwise_or[CmpopExprPair*]:
-    | (tok='!=' { _PyPegen_check_barry_as_flufl(p, tok) ? NULL : tok}) a=bitwise_or {_PyPegen_cmpop_expr_pair(p, NotEq, a) }
-lte_bitwise_or[CmpopExprPair*]: '<=' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, LtE, a) }
-lt_bitwise_or[CmpopExprPair*]: '<' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, Lt, a) }
-gte_bitwise_or[CmpopExprPair*]: '>=' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, GtE, a) }
-gt_bitwise_or[CmpopExprPair*]: '>' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, Gt, a) }
-notin_bitwise_or[CmpopExprPair*]: 'not' 'in' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, NotIn, a) }
-in_bitwise_or[CmpopExprPair*]: 'in' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, In, a) }
-isnot_bitwise_or[CmpopExprPair*]: 'is' 'not' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, IsNot, a) }
-is_bitwise_or[CmpopExprPair*]: 'is' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, Is, a) }
+    | '==' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, Eq, a) }
+    | (tok='!=' { _PyPegen_check_barry_as_flufl(p, tok) ? NULL : tok }) a=bitwise_or {
+        _PyPegen_cmpop_expr_pair(p, NotEq, a) }
+    | '<=' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, LtE, a) }
+    | '<' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, Lt, a) }
+    | '>=' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, GtE, a) }
+    | '>' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, Gt, a) }
+    | 'not' 'in' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, NotIn, a) }
+    | 'in' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, In, a) }
+    | 'is' 'not' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, IsNot, a) }
+    | 'is' a=bitwise_or { _PyPegen_cmpop_expr_pair(p, Is, a) }
 
 # Bitwise operators
 # -----------------
@@ -1014,14 +997,20 @@ strings[expr_ty] (memo):
     | a[asdl_expr_seq*]=(fstring|string)+ { _PyPegen_concatenate_strings(p, a, EXTRA) }
     | a[asdl_expr_seq*]=tstring+ { _PyPegen_concatenate_tstrings(p, a, EXTRA) }
 
+display_items[asdl_expr_seq*]: a[asdl_expr_seq*]=','.display_item+ [','] { a }
+
+display_item[expr_ty]:
+    | invalid_starred_expression_unpacking_sequence
+    | star_named_expression
+
 list[expr_ty]:
-    | '[' a=[star_named_expressions_sequence] ']' { _PyAST_List(a, Load, EXTRA) }
+    | '[' a=[display_items] ']' { _PyAST_List(a, Load, EXTRA) }
 
 tuple[expr_ty]:
-    | '(' a=[y=star_named_expression_sequence ',' z=[star_named_expressions_sequence] { _PyPegen_seq_insert_in_front(p, y, z) } ] ')' {
+    | '(' a=[y=display_item ',' z=[display_items] { _PyPegen_seq_insert_in_front(p, y, z) } ] ')' {
         _PyAST_Tuple(a, Load, EXTRA) }
 
-set[expr_ty]: '{' a=star_named_expressions_sequence '}' { _PyAST_Set(a, EXTRA) }
+set[expr_ty]: '{' a=display_items '}' { _PyAST_Set(a, EXTRA) }
 
 # Dicts
 # -----
@@ -1088,8 +1077,8 @@ args[expr_ty]:
                           EXTRA) }
 
 kwargs[asdl_seq*]:
-    | a=','.kwarg_or_starred+ ',' b=','.kwarg_or_double_starred+ { _PyPegen_join_sequences(p, a, b) }
-    | ','.kwarg_or_starred+
+    | a=','.kwarg_or_starred+ b=[',' c=','.kwarg_or_double_starred+ { c }] {
+        b ? _PyPegen_join_sequences(p, a, b) : a }
     | ','.kwarg_or_double_starred+
 
 starred_expression[expr_ty]:
@@ -1115,72 +1104,35 @@ kwarg_or_double_starred[KeywordOrStarred*]:
 # Generic targets
 # ---------------
 
-# NOTE: star_targets may contain *bitwise_or, targets may not.
+# Targets reuse primary. Copying the context leaves its cached values in Load.
 star_targets[expr_ty]:
     | a=star_target !',' { a }
     | a=star_target b=(',' c=star_target { c })* [','] {
         _PyAST_Tuple(CHECK(asdl_expr_seq*, _PyPegen_seq_insert_in_front(p, a, b)), Store, EXTRA) }
 
-star_targets_list_seq[asdl_expr_seq*]: a[asdl_expr_seq*]=','.star_target+ [','] { a }
-
-star_targets_tuple_seq[asdl_expr_seq*]:
-    | a=star_target b=(',' c=star_target { c })+ [','] { (asdl_expr_seq*) _PyPegen_seq_insert_in_front(p, a, b) }
-    | a=star_target ',' { (asdl_expr_seq*) _PyPegen_singleton_seq(p, a) }
-
 star_target[expr_ty] (memo):
-    | '*' a=(!'*' star_target) {
-        _PyAST_Starred(CHECK(expr_ty, _PyPegen_set_expr_context(p, a, Store)), Store, EXTRA) }
-    | target_with_star_atom
+    | '*' a=(!'*' star_target) { _PyAST_Starred(a, Store, EXTRA) }
+    | target
 
-target_with_star_atom[expr_ty] (memo):
-    | a=t_primary '.' b=NAME !t_lookahead { _PyAST_Attribute(a, b->v.Name.id, Store, EXTRA) }
-    | a=t_primary '[' b=slices ']' !t_lookahead { _PyAST_Subscript(a, b, Store, EXTRA) }
-    | star_atom
-
-star_atom[expr_ty]:
-    | a=NAME { _PyPegen_set_expr_context(p, a, Store) }
-    | '(' a=target_with_star_atom ')' { _PyPegen_set_expr_context(p, a, Store) }
-    | '(' a=[star_targets_tuple_seq] ')' { _PyAST_Tuple(a, Store, EXTRA) }
-    | '[' a=[star_targets_list_seq] ']' { _PyAST_List(a, Store, EXTRA) }
+target[expr_ty] (memo):
+    | a=target_value { _PyPegen_make_target(p, a, STAR_TARGETS) }
 
 single_target[expr_ty]:
-    | single_subscript_attribute_target
-    | a=NAME { _PyPegen_set_expr_context(p, a, Store) }
-    | '(' a=single_target ')' { a }
-
-single_subscript_attribute_target[expr_ty]:
-    | a=t_primary '.' b=NAME !t_lookahead { _PyAST_Attribute(a, b->v.Name.id, Store, EXTRA) }
-    | a=t_primary '[' b=slices ']' !t_lookahead { _PyAST_Subscript(a, b, Store, EXTRA) }
-
-t_primary[expr_ty]:
-    | a=t_primary '.' b=NAME &t_lookahead { _PyAST_Attribute(a, b->v.Name.id, Load, EXTRA) }
-    | a=t_primary '[' b=slices ']' &t_lookahead { _PyAST_Subscript(a, b, Load, EXTRA) }
-    | a=t_primary b=genexp &t_lookahead {
-        _PyAST_Call(a, CHECK(asdl_expr_seq*, (asdl_expr_seq*)_PyPegen_singleton_seq(p, b)), NULL, EXTRA) }
-    | a=t_primary '(' b=[arguments] ')' &t_lookahead {
-        _PyAST_Call(a,
-                 (b) ? ((expr_ty) b)->v.Call.args : NULL,
-                 (b) ? ((expr_ty) b)->v.Call.keywords : NULL,
-                 EXTRA) }
-    | a=atom &t_lookahead { a }
+    | a=target_value { _PyPegen_make_target(p, a, SINGLE_TARGETS) }
+
+attribute_or_subscript_target[expr_ty]:
+    | a=target_value { _PyPegen_make_target(p, a, ATTRIBUTE_OR_SUBSCRIPT_TARGETS) }
 
-t_lookahead: '(' | '[' | '.'
+target_value[expr_ty]:
+    | a=primary !('(' | '[' | '.') { a }
 
 # Targets for del statements
 # --------------------------
 
 del_targets[asdl_expr_seq*]: a[asdl_expr_seq*]=','.del_target+ [','] { a }
 
 del_target[expr_ty] (memo):
-    | a=t_primary '.' b=NAME !t_lookahead { _PyAST_Attribute(a, b->v.Name.id, Del, EXTRA) }
-    | a=t_primary '[' b=slices ']' !t_lookahead { _PyAST_Subscript(a, b, Del, EXTRA) }
-    | del_t_atom
-
-del_t_atom[expr_ty]:
-    | a=NAME { _PyPegen_set_expr_context(p, a, Del) }
-    | '(' a=del_target ')' { _PyPegen_set_expr_context(p, a, Del) }
-    | '(' a=[del_targets] ')' { _PyAST_Tuple(a, Del, EXTRA) }
-    | '[' a=[del_targets] ']' { _PyAST_List(a, Del, EXTRA) }
+    | a=target_value { _PyPegen_make_target(p, a, DEL_TARGETS) }
 
 # TYPING ELEMENTS
 # ---------------

Lib/test/test_ast/test_ast.py

@@ -696,6 +696,51 @@ def test_issue18374_binop_col_offset(self):
         self.assertEqual(grandchild_binop.end_col_offset, 3)
         self.assertEqual(grandchild_binop.end_lineno, 1)
 
+    def test_primary_target_context(self):
+        for expression in ('factory().field[index]', 'factory()[index].field'):
+            for context in (ast.Load, ast.Store, ast.Del):
+                with self.subTest(expression=expression, context=context):
+                    if context is ast.Load:
+                        node = ast.parse(expression).body[0].value
+                    elif context is ast.Store:
+                        node = ast.parse(f'{expression} = value').body[0].targets[0]
+                    else:
+                        node = ast.parse(f'del {expression}').body[0].targets[0]
+                    self.assertIsInstance(node.ctx, context)
+                    for child in ast.walk(node):
+                        if child is not node and hasattr(child, 'ctx'):
+                            self.assertIsInstance(child.ctx, ast.Load)
+
+    def test_container_target_context(self):
+        cases = (
+            ('(a, [b, c])', '(a, [b, c])'),
+            ('[a, (b, c)]', '[a, (b, c)]'),
+            ('((a))', 'a'),
+            ('()', '()'),
+            ('[]', '[]'),
+        )
+        for expression, expected_segment in cases:
+            for context in (ast.Load, ast.Store, ast.Del):
+                with self.subTest(expression=expression, context=context):
+                    if context is ast.Load:
+                        source = expression
+                    elif context is ast.Store:
+                        source = f'{expression} = value'
+                    else:
+                        source = f'del {expression}'
+                    statement = ast.parse(source).body[0]
+                    node = statement.value if context is ast.Load else statement.targets[0]
+                    for child in ast.walk(node):
+                        if hasattr(child, 'ctx'):
+                            self.assertIsInstance(child.ctx, context)
+                    self.assertEqual(ast.get_source_segment(source, node),
+                                     expected_segment)
+
+        node = ast.parse('[a, *[b, *c]] = value').body[0].targets[0]
+        for child in ast.walk(node):
+            if hasattr(child, 'ctx'):
+                self.assertIsInstance(child.ctx, ast.Store)
+
     def test_issue39579_dotted_name_end_col_offset(self):
         tree = ast.parse('@a.b.c\ndef f(): pass')
         attr_b = tree.body[0].decorator_list[0].value

Lib/test/test_syntax.py

@@ -3400,7 +3400,7 @@ def test_invalid_line_continuation_error_position(self):
 
     def test_invalid_line_continuation_left_recursive(self):
         # Check bpo-42218: SyntaxErrors following left-recursive rules
-        # (t_primary_raw in this case) need to be tested explicitly
+        # (primary_raw in this case) need to be tested explicitly
         self._check_error("A.\u018a\\ ",
                           "unexpected character after line continuation character")
         self._check_error("A.\u03bc\\\n",

Misc/NEWS.d/next/Core_and_Builtins/2026-09-13-17-00-00.gh-issue-153568.target-grammar.rst

@@ -0,0 +1,2 @@
+Speed up parsing by sharing expression rules with assignment and deletion
+targets, and simplify comparison and keyword argument rules.

Parser/action_helpers.c

@@ -346,6 +346,20 @@ _PyPegen_set_expr_context(Parser *p, expr_ty expr, expr_context_ty ctx)
     return new;
 }
 
+/* Invalid targets return NULL without raising, allowing parsing to backtrack.
+   Copy valid targets to preserve the memoized expression's context. */
+expr_ty
+_PyPegen_make_target(Parser *p, expr_ty expr, TARGETS_TYPE targets_type)
+{
+    assert(expr != NULL);
+    assert(targets_type != FOR_TARGETS);
+    if (_PyPegen_get_invalid_target(expr, targets_type) != NULL) {
+        return NULL;
+    }
+    return _PyPegen_set_expr_context(
+        p, expr, targets_type == DEL_TARGETS ? Del : Store);
+}
+
 /* Constructs a KeyValuePair that is used when parsing a dict's key value pairs */
 KeyValuePair *
 _PyPegen_key_value_pair(Parser *p, expr_ty key, expr_ty value)
@@ -1236,6 +1250,14 @@ _PyPegen_get_invalid_target(expr_ty e, TARGETS_TYPE targets_type)
         return NULL;
     }
 
+    if (targets_type == SINGLE_TARGETS ||
+        targets_type == ATTRIBUTE_OR_SUBSCRIPT_TARGETS) {
+        if (targets_type == SINGLE_TARGETS && e->kind == Name_kind) {
+            return NULL;
+        }
+        return e->kind == Attribute_kind || e->kind == Subscript_kind ? NULL : e;
+    }
+
 #define VISIT_CONTAINER(CONTAINER, TYPE) do { \
         Py_ssize_t len = asdl_seq_LEN((CONTAINER)->v.TYPE.elts);\
         for (Py_ssize_t i = 0; i < len; i++) {\