feat: add IndexExpr support to constant_fold_expr [1/2]#19982
feat: add IndexExpr support to constant_fold_expr [1/2]#19982BobTheBuidler wants to merge 50 commits intopython:masterfrom
constant_fold_expr [1/2]#19982Conversation
constant_fold_exprconstant_fold_expr [1/1]
constant_fold_expr [1/1]constant_fold_expr [1/1]
This comment has been minimized.
This comment has been minimized.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
This comment has been minimized.
This comment has been minimized.
JukkaL
left a comment
There was a problem hiding this comment.
Please add some test cases -- the logic is non-trivial so it would be good to have test coverage.
| 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: |
There was a problem hiding this comment.
I think it would be better to check if base is a Sequence -- you can index an int, for example.
|
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? |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
for more information, see https://pre-commit.ci
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Sequence / dict, yeah. It isn't just Sequence because #19864 enables quasi-folding over dicts |
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
|
@hauntsaninja did I perhaps misunderstand what you were asking about? Anything still blocking this one? |
|
Yeah, I understood Jukka's suggestion to be to use |
This PR adds support for IndexExpr in
constant_fold_exprand support for constant folding intransform_index_expr