Improve type stability of array_subpadding slightly #48136
Merged
Conversation
This file contains 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
This should be slightly more efficient as the compiler now only tries to call `iterate` on `t` and `s` once, and will not try to destructure the result if the `iterate` call returns `nothing`. This change reduce spurious JET warnings.
vtjnash
reviewed
Jan 5, 2023
| checked_size = 0 | ||
| ps, sstate = iterate(s) # use of Stateful harms inference and makes this vulnerable to invalidation | ||
| pad, tstate = iterate(t) | ||
| # use of Stateful harms inference and makes this vulnerable to invalidation |
There was a problem hiding this comment.
This does not seem to use Stateful, but just the normal iterate algorithm?
Suggested change
| # use of Stateful harms inference and makes this vulnerable to invalidation |
There was a problem hiding this comment.
I guess it was written with Stateful before and got changed to not use it and this is here as a hint for future developers to not put that back?
There was a problem hiding this comment.
Correct - it was changed some time before 1.9 by Tim Holy in hunt of invalidations. I left the comment.
There was a problem hiding this comment.
timholy
approved these changes
Jan 5, 2023
|
Test failures are unrelated. |
aviatesk
pushed a commit
that referenced
this pull request
May 25, 2023
This should be slightly more efficient as the compiler now only tries to call `iterate` on `t` and `s` once, and will not try to destructure the result if the `iterate` call returns `nothing`. This change reduce spurious JET warnings.
KristofferC
pushed a commit
that referenced
this pull request
May 27, 2023
This should be slightly more efficient as the compiler now only tries to call `iterate` on `t` and `s` once, and will not try to destructure the result if the `iterate` call returns `nothing`. This change reduce spurious JET warnings.
kpamnany
pushed a commit
that referenced
this pull request
Jun 21, 2023
This should be slightly more efficient as the compiler now only tries to call `iterate` on `t` and `s` once, and will not try to destructure the result if the `iterate` call returns `nothing`. This change reduce spurious JET warnings.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 should be slightly more efficient as the compiler now only tries to call
iterateontandsonce, and will not try to destructure the result if theiteratecall returnsnothing.This change reduce spurious JET warnings.