Repository navigation
[mypyc] feat: support constant folding in translate_index_expr [1/1] - #19972
Merged
ilevkivskyi merged 22 commits intoOct 10, 2025
Merged
ilevkivskyi merged 22 commits into
ilevkivskyi merged 22 commits into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
JukkaL
reviewed
Oct 2, 2025
Comment thread
mypyc/irbuild/expression.py
Outdated
| if ( | ||
| isinstance(base.type, RTuple) | ||
| and isinstance(folded_index := constant_fold_expr(builder, index), int) | ||
| and folded_index >= 0 |
Copy link
Copy Markdown
Collaborator
There was a problem hiding this comment.
Check for index overflow (it may generate an error elsewhere, but we don't want to crash the compiler if it's ignored).
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment.
Done.
I also just thought about reverse indexing, I'll implement that too
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment.
We will need to adapt TupleGet before we can pass negative values here.
I've implemented that in a separate PR #19990 , lets pause this one until we get that merged in so we don't have to come back to this section later
Sorry, something went wrong.
maybe I should just compute the correct positive index value when we encounter a negative index, rather than doing this? Where's the best place to do that? Should that be with this or in a separate PR?
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
BobTheBuidler
added a commit
to BobTheBuidler/mypy
that referenced
this pull request
Oct 2, 2025
This PR modifies `TupleGet.__init__` to automatically convert negative indexes to positive indexes instead of crashing at the assert This won't change functionality on its own, but will allow us to pass negative values in python#19972 so I think we should consider this PR a prerequisite to that one
BobTheBuidler
added a commit
to BobTheBuidler/mypy
that referenced
this pull request
Oct 2, 2025
This PR modifies `TupleGet.__init__` to automatically convert negative indexes to positive indexes instead of crashing at the assert This won't change functionality on its own, since none of the existing calling locations can pass a negative value, but will allow us to pass negative values in python#19972 so I think we should consider this PR a prerequisite to that one
This comment was marked as off-topic.
p-sawicki
added a commit
that referenced
this pull request
Oct 7, 2025
This PR modifies `TupleGet.__init__` to automatically convert negative indexes to positive indexes instead of crashing at the assert This won't change functionality on its own, since none of the existing calling locations can pass a negative value, but will allow us to pass negative values in #19972 so I think we should consider this PR a prerequisite to that one --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Piotr Sawicki <sawickipiotr@outlook.com>
Copy link
Copy Markdown
Contributor
Author
|
Now that #19990 is merged, this PR is ready for review. |
Sorry, something went wrong.
Closed
ilevkivskyi
approved these changes
Oct 10, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR attempts to constant fold the index value in translate_index_expr
I'm not sure any test changes are warranted for a small PR of this nature.