Skip to content

improve nongeneric methods of Base.rest#59549

Merged
oscardssmith merged 7 commits intoJuliaLang:masterfrom
nsajko:better-dispatch-for-rest-and-indexed_iterate
Feb 1, 2026
Merged

improve nongeneric methods of Base.rest#59549
oscardssmith merged 7 commits intoJuliaLang:masterfrom
nsajko:better-dispatch-for-rest-and-indexed_iterate

Conversation

@nsajko
Copy link
Copy Markdown
Member

@nsajko nsajko commented Sep 13, 2025

Allow non-Int in dispatch for the second argument (iteration state), but typeassert Int in the method body. This improves abstract inference because the compiler can know the second argument must be Int to avoid a throw, when previously such a call would have dispatched to the generic method.

@nsajko nsajko added arrays [a, r, r, a, y, s] iteration Involves iteration or the iteration protocol labels Sep 13, 2025
@nsajko nsajko marked this pull request as ready for review September 13, 2025 17:03
@nsajko nsajko marked this pull request as draft January 25, 2026 10:23
@nsajko nsajko force-pushed the better-dispatch-for-rest-and-indexed_iterate branch from 94f21aa to 2dc15ce Compare January 25, 2026 21:04
@nsajko
Copy link
Copy Markdown
Member Author

nsajko commented Jan 25, 2026

Parts of this PR had been split into PR #60808 and PR #60809 and are now merged. Fixing merge conflicts.

@nsajko nsajko force-pushed the better-dispatch-for-rest-and-indexed_iterate branch from 2dc15ce to 278212f Compare January 25, 2026 21:11
Allow non-`Int` in dispatch for the second argument (iteration state),
but `typeassert` `Int` in the method body. This improves abstract
inference because the compiler can know the second argument must be
`Int` to avoid a throw, when previously such a call would have
dispatched to the generic method.
@nsajko nsajko force-pushed the better-dispatch-for-rest-and-indexed_iterate branch from c2f7aac to 25c21c7 Compare January 26, 2026 15:05
@nsajko nsajko marked this pull request as ready for review January 26, 2026 15:25
@simeonschaub
Copy link
Copy Markdown
Member

I can see how this could help inference for Base.rest, but in indexed_iterate i is always an Int literal inserted by lowering, are there any realistic use cases where this would matter?

@nsajko
Copy link
Copy Markdown
Member Author

nsajko commented Jan 27, 2026

in indexed_iterate i is always an Int literal inserted by lowering

A JuliaHub search indicates that several packages implement an indexed_iterate method. In some cases indexed_iterate is also called from that method. So if constrop bails, which happens easily in any case, i is not a compile-time constant any more.

While, indexed_iterate is not public from Base, so the packages that use this function are technically buggy, it seems to me it would indeed make sense to make indexed_iterate function a public interface, so packages could implement it for custom collection/iterator types they own in a supported manner.

I believe that justifies this PR. Of course, I could also split the indexed_iterate changes from the rest changes, if the rest changes will be easier to get merged.

@simeonschaub
Copy link
Copy Markdown
Member

So if constrop bails, which happens easily in any case, i is not a compile-time constant any more.

Sure, but it will still be inferred as Int, even without constant propagation. For this PR to be useful, indexed_iterate would have to be called from a context where the index can't even be inferred to be an Int and in such a case you might as well call iterate directly. I don't think we should make this code uglier without any concrete real-world motivation

@nsajko nsajko changed the title improve nongeneric methods of Base.rest, Base.indexed_iterate improve nongeneric methods of Base.rest Jan 28, 2026
nsajko added a commit to nsajko/julia that referenced this pull request Jan 29, 2026
nsajko added a commit to nsajko/julia that referenced this pull request Jan 30, 2026
@nsajko nsajko added merge me PR is reviewed. Merge when all tests are passing and removed status: waiting for PR reviewer labels Jan 31, 2026
@DilumAluthge
Copy link
Copy Markdown
Member

CI is all green now. @oscardssmith Do you want to merge this?

@oscardssmith oscardssmith merged commit 78e95d2 into JuliaLang:master Feb 1, 2026
10 checks passed
@oscardssmith oscardssmith removed the merge me PR is reviewed. Merge when all tests are passing label Feb 1, 2026
@nsajko nsajko deleted the better-dispatch-for-rest-and-indexed_iterate branch February 1, 2026 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrays [a, r, r, a, y, s] iteration Involves iteration or the iteration protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants