Fix invalid IR when flattening nested `BestFitting` elements (#25398)
Summary
--
This PR fixes #25397 by dropping `StartBestFittingEntry` and
`EndBestFittingEntry` tags in
`RemoveSoftLinesBuffer`. #25144 changed `RemoveSoftLinesBuffer` to
inline the most-flat variant of
`BestFitting` elements, but it inlined them with their start and end
tags, which caused the problem
observed in #25397 when these tags became nested in another
`BestFitting` element, which interpreted
the first `End` tag as ending the outer `BestFitting` rather than the
nested one. You can see this
clearly in the IR. First, the diff between the 0.15.13 and 0.15.14 IR,
which reveals the unmatched
start tags:
<details>
<summary>0.15.13 vs main</summary>
```diff
@@ -38,29 +38,13 @@
"lambda ",
group(["arg"]),
": ",
- best_fitting([
- [
- [
- <interned 1> [
- "str(",
- group([
- indent([soft_line_break, group(["arg"])]),
- soft_line_break
- ]),
- ")"
- ]
- ]
- ]
- [[group(expand: true, [<ref interned *1>])]]
- [
- [
- "(",
- indent([hard_line_break, <ref interned *1>]),
- hard_line_break,
- ")"
- ]
+ [
+ <interned 1> [
+ "str(",
+ group([indent([group(["arg"])])]),
+ ")"
]
- ]),
+ ],
", self.args"
])
])
@@ -105,27 +89,9 @@
"lambda ",
group(["arg"]),
": ",
- best_fitting([
- [
- [
- <interned 2> [
- "str(",
- group([
- indent([soft_line_break, group(["arg"])]),
- soft_line_break
- ]),
- ")"
- ]
- ]
- ]
- [[group(expand: true, [<ref interned *2>])]]
- [
- [
- "(",
- indent([hard_line_break, <ref interned *2>]),
- hard_line_break,
- ")"
- ]
+ [
+ <interned 2> [
+ "str(",
+ group([indent([group(["arg"])])]),
+ ")"
]
- ]),
- ", self.args"
+ ]
])
+ <START_WITHOUT_END<Group>>
])
- ]),
- ")"
+ <START_WITHOUT_END<Indent>>
+ ])
+ <START_WITHOUT_END<Group>>
])
+ <START_WITHOUT_END<Group>>
])
- ]),
- ")})\""
- ]),
- hard_line_break,
- ")"
+ <START_WITHOUT_END<Indent>>
+ ])
+ <START_WITHOUT_END<Group>>
+ ])
+ <START_WITHOUT_END<Indent>>
])
- ]
+ <START_WITHOUT_END<Group>>
+ ])
+ <START_WITHOUT_END<BestFittingEntry>>
]
])
]),
```
</details>
and then the diff between main and this PR, which shows the removed
brackets that represent the
nested tags:
<details>
<summary>main vs PR</summary>
```diff
@@ -38,12 +38,10 @@
"lambda ",
group(["arg"]),
": ",
- [
- <interned 1> [
- "str(",
- group([indent([group(["arg"])])]),
- ")"
- ]
+ <interned 1> [
+ "str(",
+ group([indent([group(["arg"])])]),
+ ")"
],
", self.args"
])
@@ -89,24 +87,15 @@
"lambda ",
group(["arg"]),
": ",
- [
- <interned 2> [
- "str(",
- group([indent([group(["arg"])])]),
- ")"
- ]
- ]
+ <interned 2> [
+ "str(",
+ group([indent([group(["arg"])])]),
+ ")"
+ ],
+ ", self.args"
])
- <START_WITHOUT_END<Group>>
])
- <START_WITHOUT_END<Indent>>
- ])
- <START_WITHOUT_END<Group>>
+ ]),
+ ")"
])
- <START_WITHOUT_END<Group>>
])
- <START_WITHOUT_END<Indent>>
- ])
- <START_WITHOUT_END<Group>>
- ])
- <START_WITHOUT_END<Indent>>
+ ]),
+ ")})\""
+ ]),
+ hard_line_break,
+ ")"
])
- <START_WITHOUT_END<Group>>
- ])
- <START_WITHOUT_END<BestFittingEntry>>
+ ]
]
])
]),
```
</details>
I think the diff between 0.15.13 and this PR is also nice to see because
it makes it a bit more
clear that the `BestFitting` element has been inlined:
<details>
<summary>0.15.13 vs this PR</summary>
```diff
@@ -38,29 +38,11 @@
"lambda ",
group(["arg"]),
": ",
- best_fitting([
- [
- [
- <interned 1> [
- "str(",
- group([
- indent([soft_line_break, group(["arg"])]),
- soft_line_break
- ]),
- ")"
- ]
- ]
- ]
- [[group(expand: true, [<ref interned *1>])]]
- [
- [
- "(",
- indent([hard_line_break, <ref interned *1>]),
- hard_line_break,
- ")"
- ]
- ]
- ]),
+ <interned 1> [
+ "str(",
+ group([indent([group(["arg"])])]),
+ ")"
+ ],
", self.args"
])
])
@@ -105,29 +87,11 @@
"lambda ",
group(["arg"]),
": ",
- best_fitting([
- [
- [
- <interned 2> [
- "str(",
- group([
- indent([soft_line_break, group(["arg"])]),
- soft_line_break
- ]),
- ")"
- ]
- ]
- ]
- [[group(expand: true, [<ref interned *2>])]]
- [
- [
- "(",
- indent([hard_line_break, <ref interned *2>]),
- hard_line_break,
- ")"
- ]
- ]
- ]),
+ <interned 2> [
+ "str(",
+ group([indent([group(["arg"])])]),
+ ")"
+ ],
", self.args"
])
])
@@ -155,7 +119,6 @@
def __str__(self) -> str:
return (
- f"first stringlist of args long={list(map(lambda arg: str(
- arg
- ), self.args))})"
+ "first string"
+ f"list of args long={list(map(lambda arg: str(arg), self.args))})"
)
```
</details>
And you can also see at the bottom here that the actual formatting that
#25144 sought to resolve
is still fixed.
Test Plan
--
New regression test based on the code in the issue B
Brent Westbrook committed
9c6536e73c10c1aabea72a2b11b218e67c4d090c
Parent: 0f3ea5a
Committed by GitHub <noreply@github.com>
on 5/26/2026, 8:54:41 PM