Repository: python/cpython
Pull request: gh-110309: prune empty constant in format specs
close #110309 

<!-- gh-issue-number: gh-110309 -->
* Issue: gh-110309
<!-- /gh-issue-number -->


Problem statement:
empty string constant in f-string format_spec
# Bug report

### Bug description:

The f-string `f'{1:{name}}'` has a extra `Constant(value='')` in the format_spec part in 3.12, which is not part of the AST in 3.11. This has also an effect on the generated bytecode.
    
```python
import ast
import dis

print(ast.dump(ast.parse("f'{1:{name}}'"),indent=2))

dis.dis(compile("f'{1:{name}}'","<string>","exec"))
```

output (Python 3.12.0):
```python
Module(
  body=[
    Expr(
      value=JoinedStr(
        values=[
          FormattedValue(
            value=Constant(value=1),
            conversion=-1,
            format_spec=JoinedStr(
              values=[
                FormattedValue(
                  value=Name(id='name', ctx=Load()),
                  conversion=-1),
                Constant(value='')]))]))],            # <-- extra Constant
  type_ignores=[])
  0           0 RESUME                   0

  1           2 LOAD_CONST               0 (1)
              4 LOAD_NAME                0 (name)
              6 FORMAT_VALUE             0
              8 LOAD_CONST               1 ('')        # <-- extra LOAD_CONST
             10 BUILD_STRING             2             # <-- extra BUILD_STRING
             12 FORMAT_VALUE             4 (with format)
             14 POP_TOP
             16 RETURN_CONST             2 (None)
```


output (Python 3.11.3):
```python
Module(
  body=[
    Expr(
      value=JoinedStr(
        values=[
          FormattedValue(
            value=Constant(value=1),
            conversion=-1,
            format_spec=JoinedStr(
              values=[
                FormattedValue(
                  value=Name(id='name', ctx=Load()),
                  conversion=-1)]))]))],
  type_ignores=[])
  0           0 RESUME                   0

  1           2 LOAD_CONST               0 (1)
              4 LOAD_NAME                0 (name)
              6 FORMAT_VALUE             0
              8 FORMAT_VALUE             4 (with format)
             10 POP_TOP
             12 LOAD_CONST               1 (None)
             14 RETURN_VALUE
```




### CPython versions tested on:

3.12

### Operating systems tested on:

Linux

<!-- gh-linked-prs -->
### Linked PRs
* gh-110320
<!-- /gh-linked-prs -->

Hints from issue discussion:
Confirmed also on main: 8d92b6eff3
@lysnikolaou @isidentical can you take a look? If you are not available I can take a look at this next week
I proposed a fix: apply the same empty constant prune algorithm here.

Commit message: 📜🤖 Added by blurb_it.

Patch to review:
diff --git a/Lib/test/test_fstring.py b/Lib/test/test_fstring.py
index 4f05a149a901b2..dd8c2dd628ee13 100644
--- a/Lib/test/test_fstring.py
+++ b/Lib/test/test_fstring.py
@@ -514,6 +514,54 @@ def test_ast_fstring_empty_format_spec(self):
         self.assertEqual(type(format_spec), ast.JoinedStr)
         self.assertEqual(len(format_spec.values), 0)
 
+    def test_ast_fstring_format_spec(self):
+        expr = "f'{1:{name}}'"
+
+        mod = ast.parse(expr)
+        self.assertEqual(type(mod), ast.Module)
+        self.assertEqual(len(mod.body), 1)
+
+        fstring = mod.body[0].value
+        self.assertEqual(type(fstring), ast.JoinedStr)
+        self.assertEqual(len(fstring.values), 1)
+
+        fv = fstring.values[0]
+        self.assertEqual(type(fv), ast.FormattedValue)
+
+        format_spec = fv.format_spec
+        self.assertEqual(type(format_spec), ast.JoinedStr)
+        self.assertEqual(len(format_spec.values), 1)
+
+        format_spec_value = format_spec.values[0]
+        self.assertEqual(type(format_spec_value), ast.FormattedValue)
+        self.assertEqual(format_spec_value.value.id, 'name')
+
+        expr = "f'{1:{name1}{name2}}'"
+
+        mod = ast.parse(expr)
+        self.assertEqual(type(mod), ast.Module)
+        self.assertEqual(len(mod.body), 1)
+
+        fstring = mod.body[0].value
+        self.assertEqual(type(fstring), ast.JoinedStr)
+        self.assertEqual(len(fstring.values), 1)
+
+        fv = fstring.values[0]
+        self.assertEqual(type(fv), ast.FormattedValue)
+
+        format_spec = fv.format_spec
+        self.assertEqual(type(format_spec), ast.JoinedStr)
+        self.assertEqual(len(format_spec.values), 2)
+
+        format_spec_value = format_spec.values[0]
+        self.assertEqual(type(format_spec_value), ast.FormattedValue)
+        self.assertEqual(format_spec_value.value.id, 'name1')
+
+        format_spec_value = format_spec.values[1]
+        self.assertEqual(type(format_spec_value), ast.FormattedValue)
+        self.assertEqual(format_spec_value.value.id, 'name2')
+
+
     def test_docstring(self):
         def f():
             f'''Not a docstring'''
diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-10-03-23-26-18.gh-issue-110309.Y8nDOF.rst b/Misc/NEWS.d/next/Core and Builtins/2023-10-03-23-26-18.gh-issue-110309.Y8nDOF.rst
new file mode 100644
index 00000000000000..830428730391df
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2023-10-03-23-26-18.gh-issue-110309.Y8nDOF.rst	
@@ -0,0 +1 @@
+Remove unnecessary empty constant nodes in the ast of f-string specs.
diff --git a/Parser/action_helpers.c b/Parser/action_helpers.c
index 36e0750220a30d..56f4a82d72d644 100644
--- a/Parser/action_helpers.c
+++ b/Parser/action_helpers.c
@@ -998,18 +998,38 @@ _PyPegen_setup_full_format_spec(Parser *p, Token *colon, asdl_expr_seq *spec, in
         return NULL;
     }
 
-    // This is needed to keep compatibility with 3.11, where an empty format spec is parsed
-    // as an *empty* JoinedStr node, instead of having an empty constant in it.
-    if (asdl_seq_LEN(spec) == 1) {
-        expr_ty e = asdl_seq_GET(spec, 0);
-        if (e->kind == Constant_kind
-                && PyUnicode_Check(e->v.Constant.value)
-                && PyUnicode_GetLength(e->v.Constant.value) == 0) {
-            spec = _Py_asdl_expr_seq_new(0, arena);
+    // This is needed to keep compatibility with 3.11, where an empty format
+    // spec is parsed as an *empty* JoinedStr node, instead of having an empty
+    // constant in it.
+    Py_ssize_t n_items = asdl_seq_LEN(spec);
+    asdl_expr_seq *seq = _Py_asdl_expr_seq_new(n_items, p->arena);
+    if (seq == NULL) {
+        return NULL;
+    }
+    Py_ssize_t index = 0;
+    for (Py_ssize_t i = 0; i < n_items; i++) {
+        expr_ty item = asdl_seq_GET(spec, i);
+        if (item->kind == Constant_kind &&
+            PyUnicode_CheckExact(item->v.Constant.value) &&
+            PyUnicode_GET_LENGTH(item->v.Constant.value) == 0) {
+            continue;
+        }
+        asdl_seq_SET(seq, index++, item);
+    }
+    asdl_expr_seq *resized_specs;
+    if (index != n_items) {
+        resized_specs = _Py_asdl_expr_seq_new(index, p->arena);
+        if (resized_specs == NULL) {
+            return NULL;
         }
+        for (Py_ssize_t i = 0; i < index; i++) {
+            asdl_seq_SET(resized_specs, i, asdl_seq_GET(seq, i));
+        }
+    } else {
+        resized_specs = seq;
     }
-
-    expr_ty res = _PyAST_JoinedStr(spec, lineno, col_offset, end_lineno, end_col_offset, p->arena);
+    expr_ty res = _PyAST_JoinedStr(resized_specs, lineno, col_offset,
+                                   end_lineno, end_col_offset, p->arena);
     if (!res) {
         return NULL;
     }


Code review comments:
diff_hunk: @@ -998,18 +998,38 @@ _PyPegen_setup_full_format_spec(Parser *p, Token *colon, asdl_expr_seq *spec, in
         return NULL;
     }
 
-    // This is needed to keep compatibility with 3.11, where an empty format spec is parsed
-    // as an *empty* JoinedStr node, instead of having an empty constant in it.
-    if (asdl_seq_LEN(spec) == 1) {
-        expr_ty e = asdl_seq_GET(spec, 0);
-        if (e->kind == Constant_kind
-                && PyUnicode_Check(e->v.Constant.value)
-                && PyUnicode_GetLength(e->v.Constant.value) == 0) {
-            spec = _Py_asdl_expr_seq_new(0, arena);
+    // This is needed to keep compatibility with 3.11, where an empty format
+    // spec is parsed as an *empty* JoinedStr node, instead of having an empty
+    // constant in it.
+    Py_ssize_t n_items = asdl_seq_LEN(spec);
+    asdl_expr_seq *seq = _Py_asdl_expr_seq_new(n_items, p->arena);
+    if (seq == NULL) {
+        return NULL;
+    }
+    Py_ssize_t index = 0;
+    for (Py_ssize_t i = 0; i < n_items; i++) {
path: Parser/action_helpers.c
text: @user1:
I think it would be cleaner (and anecdotally faster) to do one pass first to count the elements (no allocations), allocate just once and then do a second pass setting the elements

@author:
makes sense