From 4d592c1f6484fa0ce638fe6ca853999d3f2d3522 Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Mon, 8 Feb 2021 16:42:00 -0500 Subject: [PATCH 1/2] Refactor to provide internal_variable_expression_evaluate for context.c for expression evaluation --- ext/liquid_c/expression.c | 2 -- ext/liquid_c/expression.h | 3 +++ ext/liquid_c/variable.c | 17 ++++++++++++----- ext/liquid_c/variable.h | 2 ++ 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/ext/liquid_c/expression.c b/ext/liquid_c/expression.c index 7f492504..cc29970b 100644 --- a/ext/liquid_c/expression.c +++ b/ext/liquid_c/expression.c @@ -75,8 +75,6 @@ static VALUE expression_strict_parse(VALUE klass, VALUE markup) return expr_obj; } -#define Expression_Get_Struct(obj, sval) TypedData_Get_Struct(obj, expression_t, &expression_data_type, sval) - VALUE expression_evaluate(VALUE self, VALUE context) { expression_t *expression; diff --git a/ext/liquid_c/expression.h b/ext/liquid_c/expression.h index f981a146..5db05b99 100644 --- a/ext/liquid_c/expression.h +++ b/ext/liquid_c/expression.h @@ -11,6 +11,9 @@ typedef struct expression { vm_assembler_t code; } expression_t; +extern const rb_data_type_t expression_data_type; +#define Expression_Get_Struct(obj, sval) TypedData_Get_Struct(obj, expression_t, &expression_data_type, sval) + void init_liquid_expression(); VALUE expression_new(VALUE klass, expression_t **expression_ptr); diff --git a/ext/liquid_c/variable.c b/ext/liquid_c/variable.c index e2035dd2..cffb4211 100644 --- a/ext/liquid_c/variable.c +++ b/ext/liquid_c/variable.c @@ -171,14 +171,14 @@ static VALUE variable_strict_parse_method(VALUE self, VALUE markup) } typedef struct { - VALUE self; + expression_t *expression; 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); + return liquid_vm_evaluate(args->context, &args->expression->code); } static VALUE rescue_variable_expression_evaluate(VALUE uncast_args, VALUE exception) @@ -189,12 +189,19 @@ static VALUE rescue_variable_expression_evaluate(VALUE uncast_args, VALUE except rb_exc_raise(exception); } -static VALUE variable_expression_evaluate(VALUE self, VALUE context) +VALUE internal_variable_expression_evaluate(expression_t *expression, VALUE context) { - variable_expression_evaluate_args_t args = { self, context }; + variable_expression_evaluate_args_t args = { expression, context }; return rb_rescue(try_variable_expression_evaluate, (VALUE)&args, rescue_variable_expression_evaluate, (VALUE)&args); } +static VALUE variable_expression_evaluate_method(VALUE self, VALUE context) +{ + expression_t *expression; + Expression_Get_Struct(self, expression); + return internal_variable_expression_evaluate(expression, context); +} + void init_liquid_variable(void) { id_rescue_strict_parse_syntax_error = rb_intern("rescue_strict_parse_syntax_error"); @@ -211,6 +218,6 @@ void init_liquid_variable(void) cLiquidCVariableExpression = rb_define_class_under(mLiquidC, "VariableExpression", cLiquidCExpression); rb_global_variable(&cLiquidCVariableExpression); - rb_define_method(cLiquidCVariableExpression, "evaluate", variable_expression_evaluate, 1); + rb_define_method(cLiquidCVariableExpression, "evaluate", variable_expression_evaluate_method, 1); } diff --git a/ext/liquid_c/variable.h b/ext/liquid_c/variable.h index f802ac0a..6ac9a5dd 100644 --- a/ext/liquid_c/variable.h +++ b/ext/liquid_c/variable.h @@ -3,6 +3,7 @@ #include "vm_assembler.h" #include "block.h" +#include "expression.h" extern VALUE cLiquidCVariableExpression; @@ -17,6 +18,7 @@ typedef struct variable_parse_args { void init_liquid_variable(void); void internal_variable_compile(variable_parse_args_t *parse_args, unsigned int line_number); void internal_variable_compile_evaluate(variable_parse_args_t *parse_args); +VALUE internal_variable_expression_evaluate(expression_t *expression, VALUE context); #endif From a07651109421ce186c18eef63d9e5d0f3e9b0b0b Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Mon, 8 Feb 2021 14:30:23 -0500 Subject: [PATCH 2/2] Fix filter error translation for context evaluate of variable expression --- ext/liquid_c/context.c | 10 ++++++++-- test/unit/variable_test.rb | 7 +++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/ext/liquid_c/context.c b/ext/liquid_c/context.c index fef1ba55..35cbf906 100644 --- a/ext/liquid_c/context.c +++ b/ext/liquid_c/context.c @@ -19,8 +19,14 @@ static VALUE context_evaluate(VALUE self, VALUE expression) switch (RB_BUILTIN_TYPE(expression)) { case T_DATA: { - if (RTYPEDDATA_P(expression) && RTYPEDDATA_TYPE(expression) == &expression_data_type) - return internal_expression_evaluate(DATA_PTR(expression), self); + if (RTYPEDDATA_P(expression) && RTYPEDDATA_TYPE(expression) == &expression_data_type) { + if (RBASIC_CLASS(expression) == cLiquidCExpression) { + return internal_expression_evaluate(DATA_PTR(expression), self); + } else { + assert(RBASIC_CLASS(expression) == cLiquidCVariableExpression); + return internal_variable_expression_evaluate(DATA_PTR(expression), self); + } + } break; // e.g. BigDecimal } case T_OBJECT: // may be Liquid::VariableLookup or Liquid::RangeLookup diff --git a/test/unit/variable_test.rb b/test/unit/variable_test.rb index 87533326..e980dd46 100644 --- a/test/unit/variable_test.rb +++ b/test/unit/variable_test.rb @@ -225,6 +225,13 @@ def test_render_variable_object assert_equal('Liquid error: concat filter requires an array argument', exc.message) end + def test_filter_argument_error_translation + variable = Liquid::Variable.new("'some words' | split", Liquid::ParseContext.new) + context = Liquid::Context.new + exc = assert_raises(Liquid::ArgumentError) { variable.render(context) } + assert_equal('Liquid error: wrong number of arguments (given 1, expected 2)', exc.message) + end + private def variable_strict_parse(markup)