Optimize range iteration

Gemfile

@@ -18,6 +18,6 @@ group :test do
   gem 'rubocop', '~> 0.53.0'
 
   platform :mri do
-    gem 'liquid-c', github: 'Shopify/liquid-c', ref: '9168659de45d6d576fce30c735f857e597fa26f6'
+    gem 'liquid-c', github: 'Shopify/liquid-c', ref: 'reversable-range'
   end
 end

lib/liquid.rb

@@ -74,6 +74,7 @@ module Liquid
 require 'liquid/utils'
 require 'liquid/tokenizer'
 require 'liquid/parse_context'
+require 'liquid/reversable_range'
 
 # Load all the tags of the standard library
 #

lib/liquid/range_lookup.rb

@@ -6,7 +6,7 @@ def self.parse(start_markup, end_markup)
       if start_obj.respond_to?(:evaluate) || end_obj.respond_to?(:evaluate)
         new(start_obj, end_obj)
       else
-        start_obj.to_i..end_obj.to_i
+        ReversableRange.new(start_obj.to_i, end_obj.to_i)
       end
     end
 
@@ -18,7 +18,7 @@ def initialize(start_obj, end_obj)
     def evaluate(context)
       start_int = to_integer(context.evaluate(@start_obj))
       end_int = to_integer(context.evaluate(@end_obj))
-      start_int..end_int
+      ReversableRange.new(start_int, end_int)
     end
 
     private

lib/liquid/reversable_range.rb

@@ -0,0 +1,77 @@
+module Liquid
+  class ReversableRange
+    include Enumerable
+
+    def initialize(min, max)
+      @min = min
+      @max = max
+      @reversed = false
+    end
+
+    def each
+      if reversed
+        index = max
+        while index >= min
+          yield index
+          index -= 1
+        end
+      else
+        index = min
+        while index <= max
+          yield index
+          index += 1
+        end
+      end
+    end
+
+    def reverse!
+      @reversed = !reversed
+      self
+    end
+
+    def empty?
+      max < min
+    end
+
+    def size
+      difference = max - min
+      if difference > 0
+        difference + 1
+      else
+        0
+      end
+    end
+
+    def load_slice(from, to = nil)
+      to ||= max
+      slice_max = [max, to].min
+      slice_min = [min, from].max
+      range = ReversableRange.new(slice_min, slice_max)
+      range.reverse! if reversed
+      range
+    end
+
+    def ==(other)
+      other.is_a?(self.class) &&
+        other.min == min &&
+        other.max == max &&
+        other.reversed == reversed
+    end
+
+    def to_s
+      if reversed
+        "#{max}..#{min}"
+      else
+        "#{min}..#{max}"
+      end
+    end
+
+    def to_liquid
+      self
+    end
+
+    protected
+
+    attr_reader :min, :max, :reversed
+  end
+end

lib/liquid/tags/for.rb

@@ -133,7 +133,6 @@ def collection_segment(context)
       end
 
       collection = context.evaluate(@collection_name)
-      collection = collection.to_a if collection.is_a?(Range)
 
       limit_value = context.evaluate(@limit)
       to = if limit_value.nil?
@@ -145,14 +144,14 @@ def collection_segment(context)
       segment = Utils.slice_collection(collection, from, to)
       segment.reverse! if @reversed
 
-      offsets[@name] = from + segment.length
+      offsets[@name] = from + segment.size
 
       segment
     end
 
     def render_segment(context, segment)
       for_stack = context.registers[:for_stack] ||= []
-      length = segment.length
+      length = segment.size
 
       result = ''
 

lib/liquid/utils.rb

@@ -1,6 +1,8 @@
 module Liquid
   module Utils
     def self.slice_collection(collection, from, to)
+      return collection.load_slice(from, to) if collection.is_a?(ReversableRange)
+
       if (from != 0 || !to.nil?) && collection.respond_to?(:load_slice)
         collection.load_slice(from, to)
       else

test/integration/tags/for_tag_test.rb

@@ -36,6 +36,10 @@ def test_for_reversed
     assert_template_result('321', '{%for item in array reversed %}{{item}}{%endfor%}', assigns)
   end
 
+  def test_for_range_reversed
+    assert_template_result('321', '{%for item in (1..3) reversed %}{{item}}{%endfor%}', {})
+  end
+
   def test_for_with_range
     assert_template_result(' 1  2  3 ', '{%for item in (1..3) %} {{item}} {%endfor%}')
 

test/unit/context_unit_test.rb

@@ -349,9 +349,9 @@ def test_nested_context_from_within_drop
 
   def test_ranges
     @context.merge("test" => '5')
-    assert_equal (1..5), @context['(1..5)']
-    assert_equal (1..5), @context['(1..test)']
-    assert_equal (5..5), @context['(test..test)']
+    assert_equal ReversableRange.new(1, 5), @context['(1..5)']
+    assert_equal ReversableRange.new(1, 5), @context['(1..test)']
+    assert_equal ReversableRange.new(5, 5), @context['(test..test)']
   end
 
   def test_cents_through_drop_nestedly

test/unit/reversable_range_unit_test.rb

@@ -0,0 +1,116 @@
+require 'test_helper'
+
+class ReversableRangeTest < Minitest::Test
+  include Liquid
+
+  def test_each_iterates_through_items_in_the_range
+    actual_items = []
+    ReversableRange.new(1, 10).each { |item| actual_items << item }
+
+    expected_items = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
+    assert_equal expected_items, actual_items
+  end
+
+  def test_implements_enumerable
+    actual_items = ReversableRange.new(1, 10).select(&:even?)
+
+    expected_items = [2, 4, 6, 8, 10]
+    assert_equal expected_items, actual_items
+  end
+
+  def test_is_not_empty_max_greater_than_min
+    range = ReversableRange.new(9, 10)
+
+    refute_predicate range, :empty?
+  end
+
+  def test_is_not_empty_max_equal_to_min
+    range = ReversableRange.new(10, 10)
+
+    refute_predicate range, :empty?
+  end
+
+  def test_is_empty_if_not_reversed_and_max_less_than_min
+    range = ReversableRange.new(10, 9)
+
+    assert_predicate range, :empty?
+  end
+
+  def test_ranges_with_the_same_max_and_min_have_one_element
+    actual_items = ReversableRange.new(1337, 1337).to_a
+    expected_items = [1337]
+    assert_equal expected_items, actual_items
+  end
+
+  def test_load_slice_returns_a_sub_range
+    actual_items = ReversableRange.new(1, 10).load_slice(5, 8).to_a
+
+    expected_items = [5, 6, 7, 8]
+    assert_equal expected_items, actual_items
+  end
+
+  def test_load_slice_returns_a_reversed_sub_range_if_reversed
+    range = ReversableRange.new(1, 10)
+    range.reverse!
+    actual_items = range.load_slice(5, 8).to_a
+
+    expected_items = [8, 7, 6, 5]
+    assert_equal expected_items, actual_items
+  end
+
+  def test_is_equal_to_other_if_also_a_reversable_range_and_has_same_properties
+    one = ReversableRange.new(1, 10)
+    one.reverse!
+
+    two = ReversableRange.new(1, 10)
+    two.reverse!
+
+    assert_equal one, two
+  end
+
+  def test_is_not_equal_to_a_non_reversable_range
+    range = ReversableRange.new(1, 10)
+    range.reverse!
+
+    refute_equal range, :something_else
+  end
+
+  def test_is_not_equal_if_ranges_have_different_mins
+    one = ReversableRange.new(1, 10)
+    two = ReversableRange.new(2, 10)
+
+    refute_equal one, two
+  end
+
+  def test_is_not_equal_if_ranges_have_different_maxes
+    one = ReversableRange.new(1, 10)
+    two = ReversableRange.new(1, 11)
+
+    refute_equal one, two
+  end
+
+  def test_is_not_equal_if_only_one_is_reversed
+    one = ReversableRange.new(1, 10)
+
+    two = ReversableRange.new(1, 10)
+    two.reverse!
+
+    refute_equal one, two
+  end
+
+  def test_to_s_mirrors_rubys_range_syntax
+    range = ReversableRange.new(1, 10)
+    assert_equal '1..10', range.to_s
+  end
+
+  def test_to_s_reverses_when_reversed
+    range = ReversableRange.new(1, 10)
+    range.reverse!
+    assert_equal '10..1', range.to_s
+  end
+
+  def test_size
+    range = ReversableRange.new(1, 10)
+    assert_equal 10, range.size
+  end
+end