Implement range/3 in C for consistent error handling (#3267)

src/builtin.c

@@ -2117,21 +2117,19 @@ static block bind_bytecoded_builtins(block b) {
     }
   }
   {
-    // Note that we can now define `range` as a jq-coded function
     block rangevar = gen_op_var_fresh(STOREV, "rangevar");
-    block rangestart = gen_op_var_fresh(STOREV, "rangestart");
-    block range = BLOCK(gen_op_simple(DUP),
-                        gen_call("start", gen_noop()),
-                        rangestart,
-                        gen_call("end", gen_noop()),
-                        gen_op_simple(DUP),
-                        gen_op_bound(LOADV, rangestart),
-                        // Reset rangevar for every value generated by "end"
-                        rangevar,
-                        gen_op_bound(RANGE, rangevar));
-    builtins = BLOCK(builtins, gen_function("range",
-                                            BLOCK(gen_param("start"), gen_param("end")),
-                                            range));
+    builtins = BLOCK(builtins,
+        gen_function("range",
+                     BLOCK(gen_param_regular("start"),
+                           gen_param_regular("end"),
+                           gen_param_regular("step")),
+                     BLOCK(gen_op_unbound(LOADV, "step"),
+                           gen_op_simple(DUP),
+                           gen_op_unbound(LOADV, "end"),
+                           gen_op_simple(DUP),
+                           gen_op_unbound(LOADV, "start"),
+                           rangevar,
+                           gen_op_bound(RANGE, rangevar))));
   }
   return BLOCK(builtins, b);
 }

src/builtin.jq

@@ -42,7 +42,7 @@ def to_entries: [keys_unsorted[] as $k | {key: $k, value: .[$k]}];
 def from_entries: map({ (.key // .Key // .name // .Name):
   if has("value") then .value else .Value end }) | add // {};
 def with_entries(f): to_entries | map(f) | from_entries;
-def reverse: [.[length - 1 - range(0;length)]];
+def reverse: [.[range(length-1;-1;-1)]];
 def indices($i): if type == "array" and ($i|type) == "array" then .[$i]
   elif type == "array" then .[[$i]]
   elif type == "string" and ($i|type) == "string" then _strindices($i)
@@ -70,6 +70,7 @@ def join($x): reduce .[] as $i (null;
 def _flatten($x): reduce .[] as $i ([]; if $i | type == "array" and $x != 0 then . + ($i | _flatten($x-1)) else . + [$i] end);
 def flatten($x): if $x < 0 then error("flatten depth must not be negative") else _flatten($x) end;
 def flatten: _flatten(-1);
+def range($x;$y): range($x;$y;1);
 def range($x): range(0;$x);
 def fromdateiso8601: strptime("%Y-%m-%dT%H:%M:%SZ")|mktime;
 def todateiso8601: strftime("%Y-%m-%dT%H:%M:%SZ");
@@ -148,11 +149,6 @@ def skip($n; expr):
   if $n > 0 then foreach expr as $item ($n; . - 1; if . < 0 then $item else empty end)
   elif $n == 0 then expr
   else error("skip doesn't support negative count") end;
-# range/3, with a `by` expression argument
-def range($init; $upto; $by):
-    if $by > 0 then $init|while(. < $upto; . + $by)
-  elif $by < 0 then $init|while(. > $upto; . + $by)
-  else empty end;
 def first(g): label $out | g | ., break $out;
 def isempty(g): first((g|false), true);
 def all(generator; condition): isempty(generator|condition and empty);

src/execute.c

@@ -516,24 +516,29 @@ jv jq_next(jq_state *jq) {
       uint16_t v = *pc++;
       jv* var = frame_local_var(jq, v, level);
       jv max = stack_pop(jq);
+      jv step = stack_pop(jq);
       if (raising) {
         jv_free(max);
+        jv_free(step);
         goto do_backtrack;
-      } 
-      if (jv_get_kind(*var) != JV_KIND_NUMBER ||
-          jv_get_kind(max) != JV_KIND_NUMBER) {
-        set_error(jq, jv_invalid_with_msg(jv_string_fmt("Range bounds must be numeric")));
+      }
+
+      if (jv_get_kind(*var) != JV_KIND_NUMBER || jv_get_kind(max) != JV_KIND_NUMBER ||
+          jv_get_kind(step) != JV_KIND_NUMBER) {
+        set_error(jq, jv_invalid_with_msg(jv_string("Range bounds and step must be numeric")));
         jv_free(max);
+        jv_free(step);
         goto do_backtrack;
-      } else if (jv_number_value(*var) >= jv_number_value(max)) {
-        /* finished iterating */
+      } else if (jv_cmp(jv_copy(*var), jv_copy(max)) * jv_cmp(jv_copy(step), jv_number(0)) >= 0) {
         jv_free(max);
+        jv_free(step);
         goto do_backtrack;
       } else {
         jv curr = *var;
-        *var = jv_number(jv_number_value(*var) + 1);
+        *var = jv_number(jv_number_value(curr) + jv_number_value(step));
 
         struct stack_pos spos = stack_get_pos(jq);
+        stack_push(jq, step);
         stack_push(jq, max);
         stack_save(jq, pc - 3, spos);
 

tests/jq.test

@@ -304,10 +304,30 @@ null
 null
 [0,-1,-2,-3,-4]
 
+[range(0;-10;-3)]
+null
+[0,-3,-6,-9]
+
+[range(0;10;0)]
+null
+[]
+
 [range(0,1;4,5;1,2)]
 null
 [0,1,2,3,0,2, 0,1,2,3,4,0,2,4, 1,2,3,1,3, 1,2,3,4,1,3]
 
+try range(.) catch .
+null
+"Range bounds and step must be numeric"
+
+try range(0;"") catch .
+null
+"Range bounds and step must be numeric"
+
+try range(0;1;[]) catch .
+null
+"Range bounds and step must be numeric"
+
 [while(.<100; .*2)]
 1
 [1,2,4,8,16,32,64]