Skip to content

feat: add IndexExpr support to constant_fold_expr [1/2]#19982

Open
BobTheBuidler wants to merge 50 commits intopython:masterfrom
BobTheBuidler:patch-12
Open

feat: add IndexExpr support to constant_fold_expr [1/2]#19982
BobTheBuidler wants to merge 50 commits intopython:masterfrom
BobTheBuidler:patch-12

Conversation

@BobTheBuidler
Copy link
Copy Markdown
Contributor

@BobTheBuidler BobTheBuidler commented Oct 2, 2025

This PR adds support for IndexExpr in constant_fold_expr and support for constant folding in transform_index_expr

@BobTheBuidler BobTheBuidler changed the title [mypyc] feat: add IndexOp support to constant_fold_expr [mypyc] feat: add IndexExpr support to constant_fold_expr [1/1] Oct 2, 2025
@BobTheBuidler BobTheBuidler changed the title [mypyc] feat: add IndexExpr support to constant_fold_expr [1/1] feat: add IndexExpr support to constant_fold_expr [1/1] Oct 2, 2025
@BobTheBuidler BobTheBuidler marked this pull request as ready for review October 2, 2025 06:51
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Please add some test cases -- the logic is non-trivial so it would be good to have test coverage.

Comment thread mypy/constant_fold.py
return constant_fold_unary_op(expr.op, value)
elif isinstance(expr, IndexExpr):
base = constant_fold_expr(expr.base, cur_mod_id)
if base is not None:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it would be better to check if base is a Sequence -- you can index an int, for example.

Comment thread mypyc/irbuild/constant_fold.py
@BobTheBuidler
Copy link
Copy Markdown
Contributor Author

If we can merge #19930 first I would be able to re-use the same run test, I think that's probably best for the sake of deduplication. Wdyt?

@github-actions

This comment has been minimized.

@BobTheBuidler BobTheBuidler marked this pull request as draft October 2, 2025 19:04
@github-actions

This comment has been minimized.

Comment thread mypyc/irbuild/constant_fold.py
Comment thread mypyc/test-data/irbuild-constant-fold.test
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Jukka's comment about checking base is a sequence still applies, right?

@BobTheBuidler
Copy link
Copy Markdown
Contributor Author

Sequence / dict, yeah. It isn't just Sequence because #19864 enables quasi-folding over dicts

@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@BobTheBuidler
Copy link
Copy Markdown
Contributor Author

BobTheBuidler commented Feb 8, 2026

@hauntsaninja did I perhaps misunderstand what you were asking about? Anything still blocking this one?

@hauntsaninja
Copy link
Copy Markdown
Collaborator

hauntsaninja commented Feb 8, 2026

Yeah, I understood Jukka's suggestion to be to use if isinstance(base, (Sequence, Mapping)) etc to avoid the type ignore comments on L102 and L109 #19982 (comment)

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.

3 participants