Skip to content

Refactor `array_expand` (and squash bug)

Michael Aaron Murphy requested to merge huntergoldstein:select_touchups into master

Created by: huntergoldstein

Changes introduced by this pull request:

  • Refactored array_expand to be the entry point for any Selection into an array of words, DRYing up the code along the way
  • Squashed a subtle bug where reverse indexing into an array that expanded to a larger array (for example the array @[[foo bar] [baz quux]] would incorrectly use the original length of the array and not the expanded length. This has been resolved by getting a reversed iterator over the array instead of using the size to resolve the true length.
  • Added a regression test for the above

State: Good to go!

Other: The majority of my time spent on this PR was trying to figure out the best way to expand Select code reuse:

  • I'm probably overthinking this. Just having functions like array_expand work beautifully; they aren't as pretty as foobar.select(..) but they work and are as efficient as possible, generally.
  • It will be much easier if I pass around Box<Iterator>s as I don't need to commit to a particular iterator type and instead use composition of iterators. I have noticed there is / was a clear effort to reduce the use of heap allocated structures as well as references to removing the allocation increasing performance. Are there benchmark tests lying around that I could make use of?

Merge request reports