Skip to content

Conversation

@utdemir
Copy link
Contributor

@utdemir utdemir commented Sep 10, 2020

Depends on #180, As part of #165.

Previously, vectors could only grow. This PR adds functions around resizing and slicing vectors and arrays.

  • Adds a pop function, and renames snoc to push, just because it sounds more natural to me than unsnoc. Happy to revert this change if you prefer the former.
  • Adds slice functions to arrays and vectors that can return a sub-range of elements.
  • Adds a shrinkToFit function to vectors to reduce the capacity of a vector.

Alongside with tests for each, of course.

Array newSize (Unsafe.resizeMutArr mut seed newSize)

-- XXX: Replace with toVec
-- | Copy a slice of the array, starting from given offset and copying given
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Document which of the returned arrays is the slice.


size :: HashMap k v #-> (HashMap k v, Int)
size (HashMap sz ct@(Count c) arr) = (HashMap sz ct arr, c)
size :: HashMap k v #-> (HashMap k v, Unrestricted Int)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically should be in a different PR but don't bother right now. I think it's cumbersome so I'll approve this one but going forward that's a good idea and cleaner.

where
fromHashMapSize :: (Linear.HashMap a (), Int) #-> (Set a, Int)
fromHashMapSize (hmap, size) = (Set hmap, size)
size :: Keyed a => Set a #-> (Set a, Unrestricted Int)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comment on hashmaps.


-- | Write to an element already written to before. Note: this will not write
-- to elements beyond the current size of the array and will error instead.
-- | Number of elements inside the vector.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention that this is different from the capacity.

let -- Calculate the closest power of growth factor
-- larger than required size.
newSize =
constGrowthFactor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment mentioning this is a constant defined in this file above.

Copy link
Contributor Author

@utdemir utdemir Sep 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels a bit odd to add a comment like -- this is defined at the top of this file. I would say that it is the IDE's responsibility (or grep's). Anyway, a comment won't harm anyone, so I'm happy to put the it there (unless you suggest a better one :)).


-- | Same as 'read', but does not do bounds-checking. The behaviour is undefined
-- when passed an invalid index.
unsafeRead :: HasCallStack => Vector a #-> Int -> (Vector a, Unrestricted a)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, could you change read and write to use the same elegant design as the arrays that call unsafeRead and unsafeWrite with assertIndexInRange ix arr that panics if the index is not in range?

@dpulls
Copy link

dpulls bot commented Sep 17, 2020

🎉 All dependencies have been resolved !

Previously, vectors could only grow. This commit adds functions around
resizing and slicing vectors and arrays.

* Adds a `pop` function, and renames `snoc` to `push`, just because
they sound (to me) more natural than `unsnoc`.

* Adds `slice` functions to arrays and vectors that can return a
sub-range of elements.

* Adds a `resizeToFit` function to vectors to reduce the capacity of
a vector.
@utdemir utdemir merged commit 5ec871d into master Sep 17, 2020
@utdemir utdemir deleted the ud/vector-pop branch September 17, 2020 03:01
@utdemir utdemir mentioned this pull request Sep 28, 2020
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.

3 participants