fix(vim.iter): enable optimizations for arrays (lists with holes)

[gpanders]

The optimizations that vim.iter uses for array-like tables don't strictly require that the underlying table has no holes. The only thing that needs to change is the determination if a table is "list-like": rather than requiring consecutive, integer keys, we can simply test for integer keys only, and remove any holes in the original array when we make a copy for the iterator.

This allows vim.iter(t):flatten():totable() to work as a proper replacement for vim.tbl_flatten.

Fixes #28777.

[clason]

The only thing that needs to change is the determination if a table is "list-like": rather than requiring consecutive, integer keys, we can simply test for integer keys only.

Luckily, we have a function for that as well: vim.isarray() :)

[justinmk]

Fantastic!

Does this potentially (in the future) expand the capabilities of pipelines to work on array-like stuff? For reference, currently we have defined :help list-iterator and various pipeline operations say that they require that, in their docstrings.

[gpanders]

Does this potentially (in the future) expand the capabilities of pipelines to work on array-like stuff? For reference, currently we have defined :help list-iterator and various pipeline operations say that they require that, in their docstrings.

This enables the whole vim.iter API to work with "array like" tables (i.e. lists with holes). If everything eventually uses vim.iter under the hood we could probably even relax the distinction between "list" and "array" (since both would be treated the same by the vim.iter API). It's still an important distinction though because Lua itself treats the two differently.

[justinmk]

IIUC, the length parameter is the key improvement. But it looks like we're also changing t[k] to store the original (non-contiguous) number keys; will that cause any problems for ArrayIter:next()?

Do we actually need the original number keys, or would it make sense to store t as a 1..n continguous "list" (as we did before)? Possibly I'm missing something, and maybe we need the number keys for get()-like functionality, or maybe we want to pass the number keys as k arg to the pipeline functions.

[gpanders]

I think this could go either way. If we keep the non-contiguous keys in the new array, then we have the property that vim.iter(t):totable() == t (holes and all). If we "strip" holes implicitly, then vim.iter(t):totable() serves the same purpose that people are using vim.tbl_flatten() for today in some cases (i.e. removing holes from an array).

I think I slightly favor stripping holes, if for no other reason than arrays with holes are so fraught in Lua that it's easier to avoid them as much as possible (eliminates all kinds of edge cases).

[justinmk]

If we keep the non-contiguous keys in the new array, then we have the property that vim.iter(t):totable() == t (holes and all)

That seems desirable. OTOH, to preserve the non-contiguous keys the caller also has the option of passing vim.iter(pairs(t)), right?

[gpanders]

That's right. Another benefit of stripping holes is that any functions used in the iterator can behave consistently for both arrays and function iterators. A function iterator will never return nil (nil is the signal that the function iterator is done), so if we strip holes in arrays then we preserve that property.

vim.iter(s):filter(function(v)
  -- v can never be nil, regardless of the value of s
end)

I think it also makes the mental model more consistent. Right now if a pipeline stage returns nil (e.g. :map()) then that acts as removing an item from the iterator (future pipeline stages won't see that value). But if arrays were allowed to have nil values that would be inconsistent.

The more I think about it the more I do think that stripping nil is the right call, but we can always change it later if I'm wrong.

[wookayin]

So the behavior is like the latter, i.e. vim.iter(t) always strips holes, NOT vim.iter(t):totable() == t; e.g. vim.iter({nil, 1, 2, nil, 3}):totable() == {1, 2, 3}.

While I agree that what's proposed makes sense and has many benefits, I'm not 100% convinced whether this is the best choice, because: (1) vim.iter(t):totable() == t also sounds sensible; (2) what if one wants to keep nil for any reason (instead, use filter explicitly to filter nil)?

[wookayin]

Due to L135:

-- short-circuit: this is not a list like table

flatten() on lists with hole still might fail:

vim.iter({nil, {1, nil, 2}, 3}):flatten(math.huge):totable()
-- ERROR: flatten() requires an array-like table
-- Expected: {1, 2, 3}
-- BTW: vim.tbl_flatten { { 1, nil, 2 }, 3 } == { 1, 3 }. ?!?!??!?!????

For the following, an expected output could be {{1, nil, 2}, 3} instead of an error, according to the array-like semantics:

vim.iter({nil, {1, nil, 2}, 3}):flatten():totable()
-- ERROR: flatten() requires an array-like table

where depth = 1.

[gpanders]

Expected output of vim.iter({nil, {1, nil, 2}, 3}):flatten():totable() should be {1, 2, 3}.

vim.iter({nil, {1, nil, 2}, 3}):totable() (no flatten) returns {{1, nil, 2}, 3}. Calling flatten on that would result in {1, 2, 3}.

[wookayin]

Oh that's right. My mistake; I was actually thinking about vim.iter(**{** {nil, {1, nil, 2}, 3} **}** ):flatten(1):totable() (one more depth) == { {1, nil, 3}, 3 }. So the current implementation after the update seems correct: no longer errors.

[github-actions]

Successfully created backport PR for release-0.10:

• fix(vim.iter): enable optimizations for arrays (lists with holes) #28816

[lewis6991]

Opened an issue #28941 to discuss splitting out ArrayIter.

[lewis6991]

I forgot that vim.ter did a full scan and table allocation of the source.

Surely this means for single (or even few) operations on a very large tables, that vim.iter is unfavourable when compared to the existing vim.tbl_* functions?

I'm thinking about this in context to treesitter, that needs to be able to scan large region tables.