Fix translation of ArgumentError for filters in Liquid::Variable#render

ext/liquid_c/context.c

@@ -1,6 +1,7 @@
 #include "liquid.h"
 #include "context.h"
 #include "variable_lookup.h"
+#include "variable.h"
 #include "vm.h"
 #include "expression.h"
 
@@ -17,9 +18,11 @@ static VALUE context_evaluate(VALUE self, VALUE expression)
 
     switch (RB_BUILTIN_TYPE(expression)) {
         case T_DATA:
-            if (RBASIC_CLASS(expression) == cLiquidCExpression)
+        {
+            if (RTYPEDDATA_P(expression) && RTYPEDDATA_TYPE(expression) == &expression_data_type)
                 return internal_expression_evaluate(DATA_PTR(expression), self);
             break; // e.g. BigDecimal
+        }
         case T_OBJECT: // may be Liquid::VariableLookup or Liquid::RangeLookup
         {
             VALUE result = rb_check_funcall(expression, id_evaluate, 1, &self);

ext/liquid_c/expression.c

@@ -31,10 +31,10 @@ const rb_data_type_t expression_data_type = {
     NULL, NULL, RUBY_TYPED_FREE_IMMEDIATELY
 };
 
-VALUE expression_new(expression_t **expression_ptr)
+VALUE expression_new(VALUE klass, expression_t **expression_ptr)
 {
     expression_t *expression;
-    VALUE obj = TypedData_Make_Struct(cLiquidCExpression, expression_t, &expression_data_type, expression);
+    VALUE obj = TypedData_Make_Struct(klass, expression_t, &expression_data_type, expression);
     *expression_ptr = expression;
     vm_assembler_init(&expression->code);
     return obj;
@@ -51,7 +51,7 @@ static VALUE internal_expression_parse(parser_t *p)
         return const_obj;
 
     expression_t *expression;
-    VALUE expr_obj = expression_new(&expression);
+    VALUE expr_obj = expression_new(cLiquidCExpression, &expression);
 
     parse_and_compile_expression(p, &expression->code);
     assert(expression->code.stack_size == 1);
@@ -77,7 +77,7 @@ static VALUE expression_strict_parse(VALUE klass, VALUE markup)
 
 #define Expression_Get_Struct(obj, sval) TypedData_Get_Struct(obj, expression_t, &expression_data_type, sval)
 
-static VALUE expression_evaluate(VALUE self, VALUE context)
+VALUE expression_evaluate(VALUE self, VALUE context)
 {
     expression_t *expression;
     Expression_Get_Struct(self, expression);

ext/liquid_c/expression.h

@@ -5,14 +5,16 @@
 #include "parser.h"
 
 extern VALUE cLiquidCExpression;
+extern const rb_data_type_t expression_data_type;
 
 typedef struct expression {
     vm_assembler_t code;
 } expression_t;
 
 void init_liquid_expression();
 
-VALUE expression_new(expression_t **expression_ptr);
+VALUE expression_new(VALUE klass, expression_t **expression_ptr);
+VALUE expression_evaluate(VALUE self, VALUE context);
 VALUE internal_expression_evaluate(expression_t *expression, VALUE context);
 
 #endif

ext/liquid_c/variable.c

@@ -2,6 +2,8 @@
 #include "variable.h"
 #include "parser.h"
 #include "expression.h"
+#include "vm.h"
+
 #include <stdio.h>
 
 static ID id_rescue_strict_parse_syntax_error;
@@ -10,6 +12,8 @@ static ID id_ivar_parse_context;
 static ID id_ivar_name;
 static ID id_ivar_filters;
 
+VALUE cLiquidCVariableExpression;
+
 static VALUE frozen_empty_array;
 
 static VALUE try_variable_strict_parse(VALUE uncast_args)
@@ -46,7 +50,7 @@ static VALUE try_variable_strict_parse(VALUE uncast_args)
                     if (push_keywords_obj == Qnil) {
                         expression_t *push_keywords_expr;
                         // use an object to automatically free on an exception
-                        push_keywords_obj = expression_new(&push_keywords_expr);
+                        push_keywords_obj = expression_new(cLiquidCExpression, &push_keywords_expr);
                         rb_obj_hide(push_keywords_obj);
                         push_keywords_code = &push_keywords_expr->code;
                     }
@@ -146,7 +150,7 @@ static VALUE variable_strict_parse_method(VALUE self, VALUE markup)
     VALUE parse_context = rb_ivar_get(self, id_ivar_parse_context);
 
     expression_t *expression;
-    VALUE expression_obj = expression_new(&expression);
+    VALUE expression_obj = expression_new(cLiquidCVariableExpression, &expression);
 
     variable_parse_args_t parse_args = {
         .markup = RSTRING_PTR(markup),
@@ -166,6 +170,31 @@ static VALUE variable_strict_parse_method(VALUE self, VALUE markup)
     return Qnil;
 }
 
+typedef struct {
+    VALUE self;
+    VALUE context;
+} variable_expression_evaluate_args_t;
+
+static VALUE try_variable_expression_evaluate(VALUE uncast_args)
+{
+    variable_expression_evaluate_args_t *args = (void *)uncast_args;
+    return expression_evaluate(args->self, args->context);
+}
+
+static VALUE rescue_variable_expression_evaluate(VALUE uncast_args, VALUE exception)
+{
+    variable_expression_evaluate_args_t *args = (void *)uncast_args;
+    vm_t *vm = vm_from_context(args->context);
+    exception = vm_translate_if_filter_argument_error(vm, exception);
+    rb_exc_raise(exception);
+}
+
+static VALUE variable_expression_evaluate(VALUE self, VALUE context)
+{
+    variable_expression_evaluate_args_t args = { self, context };
+    return rb_rescue(try_variable_expression_evaluate, (VALUE)&args, rescue_variable_expression_evaluate, (VALUE)&args);
+}
+
 void init_liquid_variable(void)
 {
     id_rescue_strict_parse_syntax_error = rb_intern("rescue_strict_parse_syntax_error");
@@ -179,5 +208,9 @@ void init_liquid_variable(void)
     rb_global_variable(&frozen_empty_array);
 
     rb_define_method(cLiquidVariable, "c_strict_parse", variable_strict_parse_method, 1);
+
+    cLiquidCVariableExpression = rb_define_class_under(mLiquidC, "VariableExpression", cLiquidCExpression);
+    rb_global_variable(&cLiquidCVariableExpression);
+    rb_define_method(cLiquidCVariableExpression, "evaluate", variable_expression_evaluate, 1);
 }
 

ext/liquid_c/variable.h

@@ -4,6 +4,8 @@
 #include "vm_assembler.h"
 #include "block.h"
 
+extern VALUE cLiquidCVariableExpression;
+
 typedef struct variable_parse_args {
     const char *markup;
     const char *markup_end;

ext/liquid_c/vm.c

@@ -19,7 +19,7 @@ ID id_global_filter;
 
 static VALUE cLiquidCVM;
 
-typedef struct vm {
+struct vm {
     c_buffer_t stack;
     VALUE strainer;
     VALUE filter_methods;
@@ -29,7 +29,7 @@ typedef struct vm {
     VALUE global_filter;
     bool strict_filters;
     bool invoking_filter;
-} vm_t;
+};
 
 static void vm_mark(void *ptr)
 {
@@ -86,7 +86,7 @@ static VALUE vm_internal_new(VALUE context)
     return obj;
 }
 
-static vm_t *vm_from_context(VALUE context)
+vm_t *vm_from_context(VALUE context)
 {
     VALUE vm_obj = rb_attr_get(context, id_vm);
     if (vm_obj == Qnil) {
@@ -506,6 +506,18 @@ void liquid_vm_next_instruction(const uint8_t **ip_ptr, const size_t **const_ptr
     *ip_ptr = ip;
 }
 
+VALUE vm_translate_if_filter_argument_error(vm_t *vm, VALUE exception)
+{
+    if (vm->invoking_filter) {
+        if (rb_obj_is_kind_of(exception, rb_eArgError)) {
+            VALUE cLiquidStrainerTemplate = rb_const_get(mLiquid, rb_intern("StrainerTemplate"));
+            exception = rb_funcall(cLiquidStrainerTemplate, rb_intern("arg_exc_to_liquid_exc"), 1, exception);
+        }
+        vm->invoking_filter = false;
+    }
+    return exception;
+}
+
 typedef struct vm_render_rescue_args {
     vm_render_until_error_args_t *render_args;
     size_t old_stack_byte_size;
@@ -533,13 +545,7 @@ static VALUE vm_render_rescue(VALUE uncast_args, VALUE exception)
         vm->stack.data_end = vm->stack.data + args->old_stack_byte_size;
     }
 
-    if (vm->invoking_filter) {
-        if (rb_obj_is_kind_of(exception, rb_eArgError)) {
-            VALUE cLiquidStrainerTemplate = rb_const_get(mLiquid, rb_intern("StrainerTemplate"));
-            exception = rb_funcall(cLiquidStrainerTemplate, rb_intern("arg_exc_to_liquid_exc"), 1, exception);
-        }
-        vm->invoking_filter = false;
-    }
+    exception = vm_translate_if_filter_argument_error(vm, exception);
 
     VALUE line_number = Qnil;
     if (render_args->node_line_number) {

ext/liquid_c/vm.h

@@ -4,12 +4,17 @@
 #include <ruby.h>
 #include "block.h"
 
+typedef struct vm vm_t;
+
 void init_liquid_vm();
 void liquid_vm_render(block_body_t *block, VALUE context, VALUE output);
 void liquid_vm_next_instruction(const uint8_t **ip_ptr, const size_t **const_ptr_ptr);
 bool liquid_vm_filtering(VALUE context);
 VALUE liquid_vm_evaluate(VALUE context, vm_assembler_t *code);
 
+vm_t *vm_from_context(VALUE context);
+VALUE vm_translate_if_filter_argument_error(vm_t *vm, VALUE exception);
+
 static inline unsigned int decode_node_line_number(const uint8_t *node_line_number)
 {
     return (node_line_number[0] << 16) | (node_line_number[1] << 8) | node_line_number[2];

test/unit/variable_test.rb

@@ -215,6 +215,20 @@ def test_filter_error
     assert_equal('before (Liquid error: concat filter requires an array argument) after', output)
   end
 
+  def test_render_variable_object
+    variable = Liquid::Variable.new("ary | concat: ary2", Liquid::ParseContext.new)
+    assert_instance_of(Liquid::C::VariableExpression, variable.name)
+
+    context = Liquid::Context.new('ary' => [1], 'ary2' => [2])
+    assert_equal([1, 2], variable.render(context))
+
+    context['ary2'] = 2
+    exc = assert_raises(Liquid::ArgumentError) do
+      variable.render(context)
+    end
+    assert_equal('Liquid error: concat filter requires an array argument', exc.message)
+  end
+
   private
 
   def variable_strict_parse(markup)