Skip to content

Fix iterable resolution, prefer iterator overloads#25679

Open
ZoomRmc wants to merge 7 commits intonim-lang:develfrom
ZoomRmc:iterableres
Open

Fix iterable resolution, prefer iterator overloads#25679
ZoomRmc wants to merge 7 commits intonim-lang:develfrom
ZoomRmc:iterableres

Conversation

@ZoomRmc
Copy link
Copy Markdown
Contributor

@ZoomRmc ZoomRmc commented Mar 28, 2026

This fixes type resolution for iterable[T].

I want to proceed with RFC #562 and this is the main blocker for composability.

Fixes #22098 and, arguably, #19206

import std/strutils

template collect[T](it: iterable[T]): seq[T] =
  block:
    var res: seq[T] = @[]
    for x in it:
      res.add x
    res

const text = "a b c d"

let words = text.split.collect()
doAssert words == @[ "a", "b", "c", "d" ]

In cases like strutils.split, where both proc and iterator overload exists, the compiler resolves to the func overload causing a type mismatch.

The old mode resolved text.split to seq[string] before the surrounding iterable[T] requirement was applied, so the argument no longer matched this template.

It should be noted that, compared to older sequtils templates, composable chains based on iterable[T] require an iterator-producing expression, e.g. "foo".items.iterableTmpl() rather than just "foo".iterableTmpl(). This is actually desirable: it keeps the iteration boundary explicit and makes iterable-driven templates intentionally not directly interchangeable with older untyped/loosely-typed templates like those in sequtils, whose internal iterator setup we have zero control over (e.g. hard-coding adapters like items).

Also, I noticed in semstmts that anonymous iterators are always closure, which is not that surprising if you think about it, but still I added a paragraph to the manual.

Regarding implementation:

From what I gathered, the root cause is that semOpAux eagerly pre-types all arguments with plain flags before overload resolution begins, so by the time prepareOperand processes split against the iterable[T], the wrong overload has already won.

The fix touches a few places:

  • prepareOperand in sigmatch.nim:
    When formal.kind == tyIterable and the argument was already typed as something else, it's re-semchecked with the efPreferIteratorForIterable flag. The recheck is limited to direct calls (a[0].kind in {nkIdent, nkAccQuoted, nkSym, nkOpenSym}) to avoid recursing through semIndirectOp/semOpAux again.

  • iteratorPreference field TCandidate, checked before genericMatches in cmpCandidates, gives the iterator overload a win without touching the existing iterator heuristic used by for loops.

Limitations:

The implementation is still flag-driven rather than purely formal-driven, so the behaviour is a bit too broad efWantIterable can cause iterator results to be wrapped as tyIterable in iterable-admitting contexts, not only when iterable[T] match is being processed.

iterable[T] still does not accept closure iterator values such asiterator(): T {.closure.}. It only matches the compiler's internal tyIterable, not arbitrary iterator-typed values.

The existing iterator-preference heuristic is still in place, because when I tried to remove it, some loosely-related regressions happened. In particular, ordinary iterator-admitting contexts and iterator chains still rely on early iterator preference during semchecking, before the compiler has enough surrounding context to distinguish between value/iterator producing overloads. Full heuristic removal would require a broader refactor of dot-chain/intermediate-expression semchecking, which is just too much for me ATM. This PR narrows only the tyIterable-specific cases.

Future work:

Rework overload resolution to preserve additional information of matching iterator overloads for calls up to the point where the iterator-requiring context is established, to avoid re-sem in prepareOperand.

Currently there's no good channel to store that information. Nodes can get rewritten, TCandidate doesn't live long enough, storing in Context or some side-table raises the question how to properly key that info.

Recheck direct-call operands for tyIterable, propagate iterable
preference through template re-semchecking,
and rank iterator candidates with a dedicated
iteratorPreference bucket.

This fixes cases like text.split.collect() where split previously
resolved to the seq-returning overload before the surrounding
iterable[T] requirement was applied.
@ZoomRmc ZoomRmc marked this pull request as draft March 28, 2026 02:55
@ZoomRmc ZoomRmc marked this pull request as ready for review March 28, 2026 14:47
`iterBase` is now wrong here when the loop source was wrapped as tyIterable,
because the loop variable should not get the wrapper

`iterAfterVarLent` would be too stripped, because it removes tyGenericInst

`iterType` is the middle ground:
  - removes outer tyIterable / alias-like wrappers
  - preserves tyGenericInst
ZoomRmc added 3 commits March 31, 2026 02:53
`prepareOperand` no longer retries every already-typed direct call for
iterable[T], only calls where overload resolution has seen a plausible iterator
candidate

fixes the new call check against nkCallKinds, not just nkCall

sem.nim: remove `iprefersIteratorForIterable`
@Araq
Copy link
Copy Markdown
Member

Araq commented Mar 31, 2026

@metagn review it one more time please.

@metagn
Copy link
Copy Markdown
Collaborator

metagn commented Mar 31, 2026

No other objections to the implementation, and the design is unlikely to cause regressions from what I understand, should be good except the 1 comment above.

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.

Broken overload resolution between proc and iterator for iterable

3 participants