Skip to content

gh-148817: Fold long lists/sets of constant elements into constant tuples/frozensets#149016

Open
Eclips4 wants to merge 2 commits intopython:mainfrom
Eclips4:remove-check-for-size
Open

gh-148817: Fold long lists/sets of constant elements into constant tuples/frozensets#149016
Eclips4 wants to merge 2 commits intopython:mainfrom
Eclips4:remove-check-for-size

Conversation

@Eclips4
Copy link
Copy Markdown
Member

@Eclips4 Eclips4 commented Apr 26, 2026

@itamaro
Copy link
Copy Markdown
Contributor

itamaro commented Apr 27, 2026

thanks for the PR @Eclips4 !
it looks good to me, and I tested it locally and confirmed it behaves as expected!
since this isn't my area of expertise, I'll leave further review and approval to relevant experts (like Irit)

Comment thread Python/flowgraph.c
LIST_APPEND 1
CALL_INTRINSIC_1 INTRINSIC_LIST_TO_TUPLE
LIST_APPEND/SET_ADD 1
[CALL_INTRINSIC_1 INTRINSIC_LIST_TO_TUPLE] <-- when intrinsic_at_i is true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can intrinsic_at_i be renamed to something clearer? Maybe just expected_append?

Comment thread Python/flowgraph.c

if (expect_append) {
if (opcode != LIST_APPEND || oparg != 1) {
if (opcode != append_op || oparg != 1) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this need to change?

Comment thread Python/flowgraph.c
if (oparg == INTRINSIC_LIST_TO_TUPLE) {
if (nextop == GET_ITER) {
RETURN_IF_ERROR(fold_constant_intrinsic_list_to_tuple(bb, i, consts, const_cache, consts_index));
/* If folding didn't apply, the list-to-tuple conversion
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this related to the current PR? It seems like a separate optimisation.

Also, why is the to-tuple redundant only when the folding did not apply? If it's folded it's already a tuple, right?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants