gh-150737: Optimize bytecode for empty unpack cases such as {*()}. - #150812
Conversation
Includes the branch-local test expectations for the leading-only behavior.
gesslerpd
changed the title
ast.unparse empty set representation.ast.unparse empty set representation.
|
This PR is stale because it has been open for 90 days with no activity. |
|
ping to "unstale" cc: @JelleZijlstra |
gesslerpd
changed the title
ast.unparse empty set representation.ast.unparse empty set representation.
| ('SET_UPDATE', 1), | ||
| ('RETURN_VALUE', None), | ||
| ], | ||
| '{1, 2, 3, *()}': [ |
There was a problem hiding this comment.
hm would it be much harder to optimize this too? I feel that would be more consistent, and probably not make the code much more complex. We would just ignore empty unpacks everywhere.
There was a problem hiding this comment.
Thanks for feedback, I can try to add that if you think it's worth it. Started with this since it's really only useful idiom at the start (or end) of collection literal. Current implementation is a balance to not slow down bytecode "compile" times overall. Maybe it doesn't matter much since it only affects compile-time?
There was a problem hiding this comment.
Yeah in the first place it's probably not measurable, in the second place bytecode compilation time isn't that important.
| return false; | ||
| } | ||
| expr_ty value = elt->v.Starred.value; | ||
| return value->kind == Tuple_kind && |
There was a problem hiding this comment.
Why not also lists and dicts?
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Added, nvm recursive flattening since it's probably not worth added complication for contrived forms.
gesslerpd
changed the title
ast.unparse empty set representation.ast.unparse empty set representation.
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
gesslerpd
changed the title
ast.unparse empty set representation.{*()}.
2 participants
This is preferred implementation of the proposal in issue.
This change strives to be as self contained as possible but requires adding
starunpack_helper_impl(..., start, ...)argument to supporteltsslice to ignore leading AST starred node.Less intrusive alternate implementation can be viewed here (suboptimal since it operates without AST, slight behavioral differences shown in the tests): f857d88#diff-3cbf15668c31488528b7ab0f903c674a0ecf550f4f53c1be6cf8cd965246c2a0
BUILD_SET 0#150737