add vector slice

[elcritch]

No description provided.

[oakes]

This is really great. But you don't need to define in -- it already works, because nim defines it in terms of contains.

[elcritch]

Oh nice! I can never remember that ‘in’ is already defined. Ok removed the in.

[oakes]

Currently the behavior of add on slices is buggy because it still thinks it's the original vector:

var s2 = v1[1..1] # ["two"]
s2 = s2.add(0, "yo") # expected ["yo"] but I get ["two"]
s2 = s2.add("yo") # expected ["two", "yo"] but I get ["yo"]

[elcritch]

Currently the behavior of add on slices is buggy because it still thinks it's the original vector:

Good catch! Ok, I pushed fixes for add on a slice. I also found a bug in the regular vector add too where the Leaf branch didn't increase the vec size. There should probably be large vec test for these too.

Merry Christmas and thanks for a fun x-mas day puzzle! ;)

[oakes]

I don't think this is correct. At this point we are reaching a leaf that already exists and updating its value, so the size should not change.

[oakes]

However, you inspired me to look into the size stuff and I realized I am actually incrementing too often. Specifically, if you use setLen to increase the size of a vec, and then set a value in one of the new slots, it is incrementing the size when it should not:

7fc8d3a

[elcritch]

setLen isn't used when adding a new item, at least for Vec[T]. In my test case when you added an item to the seq, the length wasn't changing like it should.

Specifically, if you use setLen to increase the size of a vec, and then set a value in one of the new slots, it is incrementing the size when it should not:

To handle both a setLen and adding new items, you'll might want a capacity and a length. That's how seq works I think.

Though, perhaps you could update add to only increment if index == size? Then I think my change would be correct for cases when you're adding at the end, and fix the issue you pointed out.

Or you could re-work add to always use setLen when index >= size.

[oakes]

setLen isn't used when adding a new item

I know; I didn't mean to imply that it was. It was a separate bug that I happened to notice yesterday.

Though, perhaps you could update add to only increment if index == size? Then I think my change would be correct for cases when you're adding at the end, and fix the issue you pointed out.

That is exactly what I did. I linked to the commit in my last comment.