Skip to content

Distinguish empty call defaults from comprehension accumulators - #8

Open
EiNSTeiN- wants to merge 2 commits into
ejfkdev:mainfrom
EiNSTeiN-:fix/comprehension-default-arguments
Open

EiNSTeiN- wants to merge 2 commits into
ejfkdev:mainfrom
EiNSTeiN-:fix/comprehension-default-arguments

Conversation

@EiNSTeiN-

Copy link
Copy Markdown

I am opening this PR to fix nested comprehension reconstruction when an iterator-producing call takes an empty container as a default argument. The code below currently loses its comprehension structure and is reconstructed correctly with this change:

def select(rows):
    return [field for row in rows for field in row.get("values", [])]

An empty list or map preceding an iterator need not be a new comprehension's result container: it may already have been consumed as a method argument. The prologue detector currently accepts an earlier empty BUILD_LIST, BUILD_SET or BUILD_MAP without distinguishing those uses. The fix requires the modern accumulator's immediate SWAP 2 placement before treating it as a new comprehension level.

The same distinction matters for set and dictionary results:

{field for row in rows for field in row.get("values", {})}
{field: len(field) for row in rows for field in row.get("values", {})}

Separately nested comprehensions must keep their own accumulator, including:

[[field for field in row.get("values", [])] for row in rows]
[{field for field in row.get("values", {})} for row in rows]
[{field: len(field) for field in row.get("values", {})} for row in rows]

Direct row["values"], a call without a default, and a tuple default retain their existing interpretation. The surrounding function's saved row and field bindings must also survive comprehension cleanup.

This accumulator layout begins with comprehension inlining in Python 3.12. The additional SWAP 2 requirement therefore applies from 3.12, preserving the older prologue handling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant