Temporarily use liquid-c branch for its corresponding change

Gemfile

@@ -22,6 +22,6 @@ group :test do
   gem 'rubocop-performance', require: false
 
   platform :mri, :truffleruby do
-    gem 'liquid-c', github: 'Shopify/liquid-c', branch: 'symbol-filter-names'
+    gem 'liquid-c', github: 'Shopify/liquid-c', ref: 'optimize-invoke-filter-refactor'
   end
 end

lib/liquid/block_body.rb

@@ -186,8 +186,8 @@ module Liquid
     end
 
     def create_variable(token, parse_context)
-      token.scan(ContentOfVariable) do |content|
-        markup = content.first
+      if token =~ ContentOfVariable
+        markup = Regexp.last_match(1)
         return Variable.new(markup, parse_context)
       end
       raise_missing_variable_terminator(token, parse_context)

lib/liquid/context.rb

@@ -93,8 +93,13 @@ module Liquid
       exception_renderer.call(e).to_s
     end
 
-    def invoke(method, *args)
-      strainer.invoke(method, *args).to_liquid
+    def invoke(method, input, *args)
+      strainer.invoke(method, input, *args).to_liquid
+    end
+
+    # @api private
+    def invoke_filter(method, input, args)
+      strainer.invoke_filter(method, input, args).to_liquid
     end
 
     # Push new local scope on the stack. use <tt>Context#stack</tt> instead

lib/liquid/strainer_template.rb

@@ -24,27 +24,32 @@ module Liquid
 
         include(filter)
 
-        filter.public_instance_methods.each { |name| filter_methods[name] = true }
+        filter_methods.merge(filter.public_instance_methods)
       end
 
       def invokable?(method)
-        filter_methods.key?(method.to_sym)
+        filter_methods.include?(method.to_sym)
       end
 
       private
 
       def filter_methods
-        @filter_methods ||= {}
+        @filter_methods ||= Set.new
       end
     end
 
-    def invoke(method, *args)
+    def invoke(method, input, *args)
+      invoke_filter(method.to_sym, input, args)
+    end
+
+    # @api private
+    def invoke_filter(method, input, args)
       if self.class.invokable?(method)
-        send(method, *args)
+        send(method, input, *args)
       elsif @context.strict_filters
         raise Liquid::UndefinedFilter, "undefined filter #{method}"
       else
-        args.first
+        input
       end
     rescue ::ArgumentError => e
       raise Liquid::ArgumentError, e.message, e.backtrace

lib/liquid/variable.rb

@@ -18,7 +18,7 @@ module Liquid
     JustTagAttributes        = /\A#{TagAttributes}\z/o
     MarkupWithQuotedFragment = /(#{QuotedFragment})(.*)/om
 
-    attr_accessor :filters, :name, :line_number
+    attr_accessor :name, :line_number
     attr_reader :parse_context
     alias_method :options, :parse_context
 
@@ -81,12 +81,15 @@ module Liquid
     end
 
     def render(context)
-      obj = @filters.inject(context.evaluate(@name)) do |output, (filter_name, filter_args, filter_kwargs)|
-        filter_args = evaluate_filter_expressions(context, filter_args, filter_kwargs)
-        context.invoke(filter_name, output, *filter_args)
+      result = context.evaluate(@name)
+      @filters.each do |filter_name, constant_args, filter_args, filter_kwargs|
+        unless constant_args
+          filter_args = evaluate_filter_expressions(context, filter_args, filter_kwargs)
+        end
+        result = context.invoke_filter(filter_name, result, filter_args)
       end
 
-      context.apply_global_filter(obj)
+      context.apply_global_filter(result)
     end
 
     def render_to_output_buffer(context, output)
@@ -110,21 +113,49 @@ module Liquid
       []
     end
 
+    def filters
+      @filters.map do |filter_name, constant_args, *filter_args_and_kwargs|
+        filter_name = filter_name.to_s
+        if constant_args
+          filter_args = filter_args_and_kwargs.first
+          if filter_args.last.is_a?(Hash)
+            filter_args = filter_args.dup
+            [filter_name, filter_args, filter_args.pop]
+          else
+            [filter_name, *filter_args_and_kwargs]
+          end
+        else
+          [filter_name, *filter_args_and_kwargs]
+        end
+      end
+    end
+
     private
 
     def parse_filter_expressions(filter_name, unparsed_args)
-      filter_args  = []
+      constant_args = true
+      filter_args = []
       keyword_args = nil
       unparsed_args.each do |a|
         if (matches = a.match(JustTagAttributes))
-          keyword_args           ||= {}
-          keyword_args[matches[1]] = Expression.parse(matches[2])
+          keyword_args ||= {}
+          expression = Expression.parse(matches[2])
+          constant_args &&= !expression.is_a?(VariableLookup)
+          keyword_args[matches[1]] = expression
         else
-          filter_args << Expression.parse(a)
+          expression = Expression.parse(a)
+          constant_args &&= !expression.is_a?(VariableLookup)
+          filter_args << expression
+        end
+      end
+      result = [filter_name.to_sym, constant_args, filter_args]
+      if keyword_args
+        if constant_args
+          filter_args << keyword_args
+        else
+          result << keyword_args
         end
       end
-      result = [filter_name.to_sym, filter_args]
-      result << keyword_args if keyword_args
       result
     end
 

test/integration/security_test.rb

@@ -43,20 +43,14 @@ class SecurityTest < Minitest::Test
     assert_equal(expected, Template.parse(text).render!(@assigns, filters: SecurityFilter))
   end
 
-  def test_does_not_permanently_add_filters_to_symbol_table
+  def test_does_not_add_filters_to_symbol_table
     current_symbols = Symbol.all_symbols
 
-    # MRI imprecisely marks objects found on the C stack, which can result
-    # in uninitialized memory being marked. This can even result in the test failing
-    # deterministically for a given compilation of ruby. Using a separate thread will
-    # keep these writes of the symbol pointer on a separate stack that will be garbage
-    # collected after Thread#join.
-    Thread.new do
-      test = %( {{ "some_string" | a_bad_filter }} )
-      Template.parse(test).render!
-      nil
-    end.join
+    test = %( {{ "some_string" | a_bad_filter }} )
 
+    Template.parse(test).render!
+
+    GC.start
     GC.start
 
     assert_equal([], (Symbol.all_symbols - current_symbols))

test/unit/strainer_factory_unit_test.rb

@@ -6,11 +6,11 @@ class StrainerFactoryUnitTest < Minitest::Test
   include Liquid
 
   module AccessScopeFilters
-    def public_filter
+    def public_filter(_input)
       "public"
     end
 
-    def private_filter
+    def private_filter(_input)
       "private"
     end
     private :private_filter
@@ -31,13 +31,13 @@ class StrainerFactoryUnitTest < Minitest::Test
   def test_strainer
     strainer = StrainerFactory.create(@context)
     assert_equal(5, strainer.invoke('size', 'input'))
-    assert_equal("public", strainer.invoke("public_filter"))
+    assert_equal("public", strainer.invoke("public_filter", 'input'))
   end
 
   def test_stainer_raises_argument_error
     strainer = StrainerFactory.create(@context)
     assert_raises(Liquid::ArgumentError) do
-      strainer.invoke("public_filter", 1)
+      strainer.invoke("public_filter", 'input', 1)
     end
   end
 
@@ -45,11 +45,11 @@ class StrainerFactoryUnitTest < Minitest::Test
     strainer = StrainerFactory.create(@context)
 
     exception = assert_raises(Liquid::ArgumentError) do
-      strainer.invoke("public_filter", 1)
+      strainer.invoke("public_filter", 'input', 1)
     end
 
     assert_match(
-      /\ALiquid error: wrong number of arguments \((1 for 0|given 1, expected 0)\)\z/,
+      /\ALiquid error: wrong number of arguments \((2 for 1|given 2, expected 1)\)\z/,
       exception.message
     )
     assert_equal(exception.backtrace[0].split(':')[0], __FILE__)
@@ -66,8 +66,8 @@ class StrainerFactoryUnitTest < Minitest::Test
 
   def test_strainer_returns_nil_if_no_filter_method_found
     strainer = StrainerFactory.create(@context)
-    assert_nil(strainer.invoke("private_filter"))
-    assert_nil(strainer.invoke("undef_the_filter"))
+    assert_nil(strainer.invoke("private_filter", nil))
+    assert_nil(strainer.invoke("undef_the_filter", nil))
   end
 
   def test_strainer_returns_first_argument_if_no_method_and_arguments_given

test/unit/variable_unit_test.rb

@@ -13,75 +13,75 @@ class VariableUnitTest < Minitest::Test
   def test_filters
     var = create_variable('hello | textileze')
     assert_equal(VariableLookup.new('hello'), var.name)
-    assert_equal([[:textileze, []]], var.filters)
+    assert_equal([['textileze', []]], var.filters)
 
     var = create_variable('hello | textileze | paragraph')
     assert_equal(VariableLookup.new('hello'), var.name)
-    assert_equal([[:textileze, []], [:paragraph, []]], var.filters)
+    assert_equal([['textileze', []], ['paragraph', []]], var.filters)
 
     var = create_variable(%( hello | strftime: '%Y'))
     assert_equal(VariableLookup.new('hello'), var.name)
-    assert_equal([[:strftime, ['%Y']]], var.filters)
+    assert_equal([['strftime', ['%Y']]], var.filters)
 
     var = create_variable(%( 'typo' | link_to: 'Typo', true ))
     assert_equal('typo', var.name)
-    assert_equal([[:link_to, ['Typo', true]]], var.filters)
+    assert_equal([['link_to', ['Typo', true]]], var.filters)
 
     var = create_variable(%( 'typo' | link_to: 'Typo', false ))
     assert_equal('typo', var.name)
-    assert_equal([[:link_to, ['Typo', false]]], var.filters)
+    assert_equal([['link_to', ['Typo', false]]], var.filters)
 
     var = create_variable(%( 'foo' | repeat: 3 ))
     assert_equal('foo', var.name)
-    assert_equal([[:repeat, [3]]], var.filters)
+    assert_equal([['repeat', [3]]], var.filters)
 
     var = create_variable(%( 'foo' | repeat: 3, 3 ))
     assert_equal('foo', var.name)
-    assert_equal([[:repeat, [3, 3]]], var.filters)
+    assert_equal([['repeat', [3, 3]]], var.filters)
 
     var = create_variable(%( 'foo' | repeat: 3, 3, 3 ))
     assert_equal('foo', var.name)
-    assert_equal([[:repeat, [3, 3, 3]]], var.filters)
+    assert_equal([['repeat', [3, 3, 3]]], var.filters)
 
     var = create_variable(%( hello | strftime: '%Y, okay?'))
     assert_equal(VariableLookup.new('hello'), var.name)
-    assert_equal([[:strftime, ['%Y, okay?']]], var.filters)
+    assert_equal([['strftime', ['%Y, okay?']]], var.filters)
 
     var = create_variable(%( hello | things: "%Y, okay?", 'the other one'))
     assert_equal(VariableLookup.new('hello'), var.name)
-    assert_equal([[:things, ['%Y, okay?', 'the other one']]], var.filters)
+    assert_equal([['things', ['%Y, okay?', 'the other one']]], var.filters)
   end
 
   def test_filter_with_date_parameter
     var = create_variable(%( '2006-06-06' | date: "%m/%d/%Y"))
     assert_equal('2006-06-06', var.name)
-    assert_equal([[:date, ['%m/%d/%Y']]], var.filters)
+    assert_equal([['date', ['%m/%d/%Y']]], var.filters)
   end
 
   def test_filters_without_whitespace
     var = create_variable('hello | textileze | paragraph')
     assert_equal(VariableLookup.new('hello'), var.name)
-    assert_equal([[:textileze, []], [:paragraph, []]], var.filters)
+    assert_equal([['textileze', []], ['paragraph', []]], var.filters)
 
     var = create_variable('hello|textileze|paragraph')
     assert_equal(VariableLookup.new('hello'), var.name)
-    assert_equal([[:textileze, []], [:paragraph, []]], var.filters)
+    assert_equal([['textileze', []], ['paragraph', []]], var.filters)
 
     var = create_variable("hello|replace:'foo','bar'|textileze")
     assert_equal(VariableLookup.new('hello'), var.name)
-    assert_equal([[:replace, ['foo', 'bar']], [:textileze, []]], var.filters)
+    assert_equal([['replace', ['foo', 'bar']], ['textileze', []]], var.filters)
   end
 
   def test_symbol
     var = create_variable("http://disney.com/logo.gif | image: 'med' ", error_mode: :lax)
     assert_equal(VariableLookup.new('http://disney.com/logo.gif'), var.name)
-    assert_equal([[:image, ['med']]], var.filters)
+    assert_equal([['image', ['med']]], var.filters)
   end
 
   def test_string_to_filter
     var = create_variable("'http://disney.com/logo.gif' | image: 'med' ")
     assert_equal('http://disney.com/logo.gif', var.name)
-    assert_equal([[:image, ['med']]], var.filters)
+    assert_equal([['image', ['med']]], var.filters)
   end
 
   def test_string_single_quoted
@@ -128,13 +128,13 @@ class VariableUnitTest < Minitest::Test
   def test_filter_with_keyword_arguments
     var = create_variable(%( hello | things: greeting: "world", farewell: 'goodbye'))
     assert_equal(VariableLookup.new('hello'), var.name)
-    assert_equal([[:things, [], { 'greeting' => 'world', 'farewell' => 'goodbye' }]], var.filters)
+    assert_equal([['things', [], { 'greeting' => 'world', 'farewell' => 'goodbye' }]], var.filters)
   end
 
   def test_lax_filter_argument_parsing
     var = create_variable(%( number_of_comments | pluralize: 'comment': 'comments' ), error_mode: :lax)
     assert_equal(VariableLookup.new('number_of_comments'), var.name)
-    assert_equal([[:pluralize, ['comment', 'comments']]], var.filters)
+    assert_equal([['pluralize', ['comment', 'comments']]], var.filters)
   end
 
   def test_strict_filter_argument_parsing