Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix array deletion bug #2662

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions docs/content/manual/manual.yml
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,27 @@ sections:
Like `.[]`, but no errors will be output if . is not an array
or object.

- title: "Reverse Array/Object Value Iterator: `.[-]`, `.[-]?`"
body: |

Just like `.[]` and `.[]?`, but when iterating an array do it
from the last element to the first.

examples:
- program: '.[-]'
input: '["a","b","c"][-]'
output:
- '"c"'
- '"b"'
- '"a"'

- title: "Array/Object Value Iterator (forward): `.[+]`, `.[+]?`"
body: |

Just like `.[]` and `.[]?`, but always iterating arrays from the
first element to the last even when invoked as part of the
`path_expression` argument to `path_reverse()`.

- title: "Comma: `,`"
body: |

Expand Down Expand Up @@ -957,6 +978,21 @@ sections:
input: '{"a":[{"b":1}]}'
output: ['[[],["a"],["a",0],["a",0,"b"]]']

- title: "`path_reverse(path_expression)`"
body: |

Just like `path(path_expression)`, but the iteration order for
any `.[]` in `path_expression` is reversed, thus
`[[range(5)]|path_reverse(.[])]` will output
`[[4],[3],[2],[1],[0]]`.

For objects, however, the iteration order of `.[]` is not reversed.

examples:
- program: '[path_reverse(.[])]'
input: '[0,1,2,3,4]'
output: ['[[4],[3],[2],[1],[0]]']

- title: "`del(path_expression)`"
body: |

Expand Down
12 changes: 12 additions & 0 deletions src/builtin.c
Original file line number Diff line number Diff line change
Expand Up @@ -1788,6 +1788,18 @@ static block bind_bytecoded_builtins(block b) {
builtin_def_1arg[i].code));
}
}
{
struct bytecoded_builtin builtin_def_1arg[] = {
{"path_reverse", BLOCK(gen_op_simple(PATH_REVERSE_BEGIN),
gen_call("arg", gen_noop()),
gen_op_simple(PATH_END))},
};
for (unsigned i=0; i<sizeof(builtin_def_1arg)/sizeof(builtin_def_1arg[0]); i++) {
builtins = BLOCK(builtins, gen_function(builtin_def_1arg[i].name,
gen_param("arg"),
builtin_def_1arg[i].code));
}
}
{
// Note that we can now define `range` as a jq-coded function
block rangevar = gen_op_var_fresh(STOREV, "rangevar");
Expand Down
2 changes: 1 addition & 1 deletion src/builtin.jq
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def add: reduce .[] as $x (null; . + $x);
def del(f): delpaths([path(f)]);
def _assign(paths; $value): reduce path(paths) as $p (.; setpath($p; $value));
def _modify(paths; update):
reduce path(paths) as $p (.;
reduce path_reverse(paths) as $p (.;
. as $dot
| null
| label $out
Expand Down
81 changes: 72 additions & 9 deletions src/execute.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ struct jq_state {

jv path;
jv value_at_path;
int path_reverse;
int subexp_nest;
int debug_trace_enabled;
int initial_execution;
Expand Down Expand Up @@ -335,6 +336,7 @@ static void set_error(jq_state *jq, jv value) {
}

#define ON_BACKTRACK(op) ((op)+NUM_OPCODES)
#define IS_BACKTRACK(op) ((op)>=NUM_OPCODES)

jv jq_next(jq_state *jq) {
jv cfunc_input[MAX_CFUNCTION_ARGS];
Expand Down Expand Up @@ -616,6 +618,7 @@ jv jq_next(jq_state *jq) {
break;
}

case PATH_REVERSE_BEGIN:
case PATH_BEGIN: {
jv v = stack_pop(jq);
stack_push(jq, jq->path);
Expand All @@ -624,11 +627,13 @@ jv jq_next(jq_state *jq) {

stack_push(jq, jv_number(jq->subexp_nest));
stack_push(jq, jq->value_at_path);
stack_push(jq, jq->path_reverse ? jv_true() : jv_false());
stack_push(jq, jv_copy(v));

jq->path = jv_array();
jq->value_at_path = v; // next INDEX operation must index into v
jq->subexp_nest = 0;
jq->path_reverse = !!(opcode == PATH_REVERSE_BEGIN);
break;
}

Expand All @@ -645,6 +650,10 @@ jv jq_next(jq_state *jq) {
}
jv_free(v); // discard value, only keep path

v = stack_pop(jq);
jq->path_reverse == !!(jv_get_kind(v) == JV_KIND_TRUE);
jv_free(v);

jv old_value_at_path = stack_pop(jq);
int old_subexp_nest = (int)jv_number_value(stack_pop(jq));

Expand All @@ -662,6 +671,7 @@ jv jq_next(jq_state *jq) {
break;
}

case ON_BACKTRACK(PATH_REVERSE_BEGIN):
case ON_BACKTRACK(PATH_BEGIN):
case ON_BACKTRACK(PATH_END): {
jv_free(jq->path);
Expand Down Expand Up @@ -718,7 +728,11 @@ jv jq_next(jq_state *jq) {
}

case EACH:
case EACH_OPT: {
case EACH_FORWARD:
case EACH_BACKWARD:
case EACH_OPT:
case EACH_FORWARD_OPT:
case EACH_BACKWARD_OPT: {
jv container = stack_pop(jq);
// detect invalid path expression like path(reverse | .[])
if (!path_intact(jq, jv_copy(container))) {
Expand All @@ -734,32 +748,81 @@ jv jq_next(jq_state *jq) {
// fallthrough
}
case ON_BACKTRACK(EACH):
case ON_BACKTRACK(EACH_OPT): {
case ON_BACKTRACK(EACH_FORWARD):
case ON_BACKTRACK(EACH_BACKWARD):
case ON_BACKTRACK(EACH_OPT):
case ON_BACKTRACK(EACH_FORWARD_OPT):
case ON_BACKTRACK(EACH_BACKWARD_OPT): {
int idx = jv_number_value(stack_pop(jq));
jv container = stack_pop(jq);
int forward = 1;

int keep_going, is_last = 0;
jv key, value;
if (jv_get_kind(container) == JV_KIND_ARRAY) {
if (opcode == EACH || opcode == EACH_OPT) idx = 0;
else idx = idx + 1;
int len = jv_array_length(jv_copy(container));
keep_going = idx < len;
is_last = idx == len - 1;

switch (opcode) {
case EACH: case EACH_OPT:
case EACH_FORWARD: case EACH_FORWARD_OPT:
if (jq->path_reverse) {
idx = jv_array_length(jv_copy(container)) - 1;
forward = 0;
} else {
idx = 0;
}
break;
case EACH_BACKWARD: case EACH_BACKWARD_OPT:
idx = jv_array_length(jv_copy(container)) - 1;
forward = 0;
break;
case ON_BACKTRACK(EACH): case ON_BACKTRACK(EACH_OPT):
if (jq->path_reverse) {
forward = 0;
idx--;
} else {
idx++;
}
break;
case ON_BACKTRACK(EACH_FORWARD): case ON_BACKTRACK(EACH_FORWARD_OPT):
idx++;
break;
case ON_BACKTRACK(EACH_BACKWARD): case ON_BACKTRACK(EACH_BACKWARD_OPT):
forward = 0;
idx--;
break;
}
if (forward) {
keep_going = idx < len;
is_last = idx == len - 1;
} else {
keep_going = idx > -1;
is_last = idx == 0;
}
if (keep_going) {
key = jv_number(idx);
value = jv_array_get(jv_copy(container), idx);
}
} else if (jv_get_kind(container) == JV_KIND_OBJECT) {
if (opcode == EACH || opcode == EACH_OPT) idx = jv_object_iter(container);
else idx = jv_object_iter_next(container, idx);
switch (opcode) {
case EACH: case EACH_OPT:
case EACH_FORWARD: case EACH_FORWARD_OPT:
case EACH_BACKWARD: case EACH_BACKWARD_OPT:
idx = jv_object_iter(container);
break;
case ON_BACKTRACK(EACH): case ON_BACKTRACK(EACH_OPT):
case ON_BACKTRACK(EACH_FORWARD): case ON_BACKTRACK(EACH_FORWARD_OPT):
case ON_BACKTRACK(EACH_BACKWARD): case ON_BACKTRACK(EACH_BACKWARD_OPT):
idx = jv_object_iter_next(container, idx);
break;
}
keep_going = jv_object_iter_valid(container, idx);
if (keep_going) {
key = jv_object_iter_key(container, idx);
value = jv_object_iter_value(container, idx);
}
} else {
assert(opcode == EACH || opcode == EACH_OPT);
assert(!IS_BACKTRACK(opcode));
if (opcode == EACH) {
char errbuf[15];
set_error(jq,
Expand Down
5 changes: 5 additions & 0 deletions src/opcode_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ OP(INDEX, NONE, 2, 1)
OP(INDEX_OPT, NONE, 2, 1)
OP(EACH, NONE, 1, 1)
OP(EACH_OPT, NONE, 1, 1)
OP(EACH_FORWARD, NONE, 1, 1)
OP(EACH_FORWARD_OPT, NONE, 1, 1)
OP(EACH_BACKWARD, NONE, 1, 1)
OP(EACH_BACKWARD_OPT, NONE, 1, 1)
OP(FORK, BRANCH, 0, 0)
OP(FORK_OPT, BRANCH, 0, 0)
OP(JUMP, BRANCH, 0, 0)
Expand All @@ -25,6 +29,7 @@ OP(SUBEXP_BEGIN, NONE, 1, 2)
OP(SUBEXP_END, NONE, 2, 2)

OP(PATH_BEGIN, NONE, 1, 2)
OP(PATH_REVERSE_BEGIN, NONE, 1, 2)
OP(PATH_END, NONE, 2, 1)

OP(CALL_BUILTIN, CFUNC, -1, 1)
Expand Down
Loading