Fix accidentally quadratic behavior in _modify
We need to be careful to not grab an extra reference when mutating data structures because that means we make extra copies. Doing that every time in `_modify` is really painful, as that function implements `|=` and all modify-assign operators. `jv_setpath()` also grabs additional references, so this is not the only fix needed for the modify-assign operators to not be accidentally quadratic. We have to use `LOADVN` in order to make this fix possible, so we should really byte-code `_modify` rather than jq-code it. However, as a stop-gap to make this fix easier, I'm adding syntax for referring to a `$binding` such that you get `LOADVN` instead of `LOADV`. This syntax is not meant to be used outside jq's internals! Therefore it is not documented. I promise to break it later, so don't use it! TBD, but later: - Optimize `_modify` for the case where `update` outputs more than one value. This is trivial: add a `setpath($p; null)` in the middle of `_modify` before calling `update`. But unfortunately the VM retains a reference to `value_at_path` for path expression checking, and fixing that will require more work.
This commit is contained in:
1 parent
0c3455d329
commit
a9f97e9e61
4 files changed
+1163
-1143
No files matched your search
+19
-1
@@ -11,7 +11,25 @@ def min_by(f): _min_by_impl(map([f]));
|
||||
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 (.; label $out | (setpath($p; getpath($p) | update) | ., break $out), delpaths([$p]));
|
||||
def _modify(paths; update):
|
||||
reduce path(paths) as $p (.;
|
||||
. as $dot
|
||||
| null
|
||||
| label $out
|
||||
| ($dot | getpath($p)) as $v
|
||||
| (
|
||||
( $$$$v
|
||||
| update
|
||||
| (., break $out) as $v
|
||||
| $$$$dot
|
||||
| setpath($p; $v)
|
||||
),
|
||||
(
|
||||
$$$$dot
|
||||
| delpaths([$p])
|
||||
)
|
||||
)
|
||||
);
|
||||
def map_values(f): .[] |= f;
|
||||
|
||||
# recurse
|
||||
|
||||
+1119
-1132
File diff suppressed because it is too large.
Load diff
+6
-10
@@ -1,9 +1,8 @@
|
||||
/* A Bison parser, made by GNU Bison 3.3.2. */
|
||||
/* A Bison parser, made by GNU Bison 3.0.4. */
|
||||
|
||||
/* Bison interface for Yacc-like parsers in C
|
||||
|
||||
Copyright (C) 1984, 1989-1990, 2000-2015, 2018-2019 Free Software Foundation,
|
||||
Inc.
|
||||
Copyright (C) 1984, 1989-1990, 2000-2015 Free Software Foundation, Inc.
|
||||
|
||||
This program is free software: you can redistribute it and/or modify
|
||||
it under the terms of the GNU General Public License as published by
|
||||
@@ -31,9 +30,6 @@
|
||||
This special exception was added by the Free Software Foundation in
|
||||
version 2.2 of Bison. */
|
||||
|
||||
/* Undocumented macros, especially those whose name start with YY_,
|
||||
are private implementation details. Do not rely on them. */
|
||||
|
||||
#ifndef YY_YY_SRC_PARSER_H_INCLUDED
|
||||
# define YY_YY_SRC_PARSER_H_INCLUDED
|
||||
/* Debug traces. */
|
||||
@@ -44,7 +40,7 @@
|
||||
extern int yydebug;
|
||||
#endif
|
||||
/* "%code requires" blocks. */
|
||||
#line 11 "src/parser.y" /* yacc.c:1927 */
|
||||
#line 11 "src/parser.y" /* yacc.c:1909 */
|
||||
|
||||
#include "locfile.h"
|
||||
struct lexer_param;
|
||||
@@ -61,7 +57,7 @@ struct lexer_param;
|
||||
} \
|
||||
} while (0)
|
||||
|
||||
#line 65 "src/parser.h" /* yacc.c:1927 */
|
||||
#line 61 "src/parser.h" /* yacc.c:1909 */
|
||||
|
||||
/* Token type. */
|
||||
#ifndef YYTOKENTYPE
|
||||
@@ -167,12 +163,12 @@ struct lexer_param;
|
||||
|
||||
union YYSTYPE
|
||||
{
|
||||
#line 31 "src/parser.y" /* yacc.c:1927 */
|
||||
#line 31 "src/parser.y" /* yacc.c:1909 */
|
||||
|
||||
jv literal;
|
||||
block blk;
|
||||
|
||||
#line 176 "src/parser.h" /* yacc.c:1927 */
|
||||
#line 172 "src/parser.h" /* yacc.c:1909 */
|
||||
};
|
||||
|
||||
typedef union YYSTYPE YYSTYPE;
|
||||
|
||||
@@ -749,6 +749,25 @@ FORMAT {
|
||||
else
|
||||
$$ = BLOCK(gen_subexp(gen_const(jv_object())), $2, gen_op_simple(POP));
|
||||
} |
|
||||
/*
|
||||
* This `$$$$varname` hack is strictly private to jq builtins. DO NOT USE!!
|
||||
*
|
||||
* This is used in `_modify`, in src/builtin.jq, to avoid holding on to a
|
||||
* reference to `.`.
|
||||
*
|
||||
* We could just have the compiler emit bytecode for `_modify` so it can use
|
||||
* LOADVN w/o needing jq syntax for LOADVN.
|
||||
*
|
||||
* This syntax, `$$$$varname`, violates referential transparency: it has
|
||||
* side-effects that are surprising.
|
||||
*
|
||||
* DO NOT USE!! I will break your jq code if you do use this outside
|
||||
* src/builtin.jq.
|
||||
*/
|
||||
'$' '$' '$' '$' IDENT {
|
||||
$$ = gen_location(@$, locations, gen_op_unbound(LOADVN, jv_string_value($5)));
|
||||
jv_free($5);
|
||||
} |
|
||||
'$' IDENT {
|
||||
$$ = gen_location(@$, locations, gen_op_unbound(LOADV, jv_string_value($2)));
|
||||
jv_free($2);
|
||||
|
||||
Reference in new issue
Block a user