diff --git a/cli/test.yaml b/cli/test.yaml index a2ef24f..4818048 100644 --- a/cli/test.yaml +++ b/cli/test.yaml @@ -5502,22 +5502,6 @@ expected: | [1,3,6,5,5] -- name: update-assignment operator against object with overlapping paths - args: - - -c - - '(.a.b,.a,.a.p.z) |= {p: ., q: .}' - input: '{"a":{"b":1}}' - expected: | - {"a":{"p":{"b":{"p":1,"q":1},"z":{"p":null,"q":null}},"q":{"b":{"p":1,"q":1}}}} - -- name: update-assignment operator against nested object with overlapping paths - args: - - -c - - '(.a.b.c,.a,.a.p.b.z) |= {p: ., q: .}' - input: '{"a":{"b":{"c":1}}}' - expected: | - {"a":{"p":{"b":{"c":{"p":1,"q":1},"z":{"p":null,"q":null}}},"q":{"b":{"c":{"p":1,"q":1}}}}} - - name: update-assignment operator with optional operator args: - -c diff --git a/compiler.go b/compiler.go index 978d2fa..6b51e0e 100644 --- a/compiler.go +++ b/compiler.go @@ -132,18 +132,13 @@ func (c *compiler) compile(q *Query) error { } func (c *compiler) compileImport(i *Import) error { - var path, alias string - var err error - if i.ImportPath != "" { - path, alias = i.ImportPath, i.ImportAlias - } else { - path = i.IncludePath - } + path, alias := cmp.Or(i.ImportPath, i.IncludePath), i.ImportAlias if c.moduleLoader == nil { return fmt.Errorf("cannot load module: %q", path) } if strings.HasPrefix(alias, "$") { var vals any + var err error if moduleLoader, ok := c.moduleLoader.(interface { LoadJSONWithMeta(string, map[string]any) (any, error) }); ok { @@ -166,6 +161,7 @@ func (c *compiler) compileImport(i *Import) error { return nil } var q *Query + var err error if moduleLoader, ok := c.moduleLoader.(interface { LoadModuleWithMeta(string, map[string]any) (*Query, error) }); ok { @@ -397,7 +393,7 @@ func (c *compiler) compileQuery(e *Query) error { return c.compileAlt(e.Left, e.Right) case OpAssign, OpModify, OpUpdateAdd, OpUpdateSub, OpUpdateMul, OpUpdateDiv, OpUpdateMod, OpUpdateAlt: - return c.compileQueryUpdate(e.Left, e.Right, e.Op, nil) + return c.compileQueryUpdate(e.Left, e.Right, e.Op) case OpOr: return c.compileIf( &If{ @@ -471,11 +467,7 @@ func (c *compiler) compileAlt(l, r *Query) error { return c.compileQuery(r) } -// compileQueryUpdate appends the compiled code for an assignment operator. The -// alloc query is nil unless the assignment may update the input value in -// place, in which case it yields the allocator of the enclosing reduce -// expression; see compiler#compileReduce. -func (c *compiler) compileQueryUpdate(l, r *Query, op Operator, alloc *Query) error { +func (c *compiler) compileQueryUpdate(l, r *Query, op Operator) error { switch op { case OpAssign: // optimize assignment operator with constant indexing and slicing @@ -495,7 +487,12 @@ func (c *compiler) compileQueryUpdate(l, r *Query, op Operator, alloc *Query) er } fallthrough case OpModify: - return c.compileUpdateFunc(op.getFunc(), []*Query{l, r}, alloc) + return c.compileFunc( + &Func{ + Name: op.getFunc(), + Args: []*Query{l, r}, + }, + ) default: name := "$%0" c.append(&code{op: opdup}) @@ -503,59 +500,27 @@ func (c *compiler) compileQueryUpdate(l, r *Query, op Operator, alloc *Query) er return err } c.append(&code{op: opstore, v: c.pushVariable(name)}) - return c.compileUpdateFunc( - "_modify", - []*Query{ - l, - {Term: &Term{ - Type: TermTypeFunc, - Func: &Func{ - Name: op.getFunc(), - Args: []*Query{ - {Term: &Term{Type: TermTypeIdentity}}, - {Term: &Term{Type: TermTypeFunc, Func: &Func{Name: name}}}, + return c.compileFunc( + &Func{ + Name: "_modify", + Args: []*Query{ + l, + {Term: &Term{ + Type: TermTypeFunc, + Func: &Func{ + Name: op.getFunc(), + Args: []*Query{ + {Term: &Term{Type: TermTypeIdentity}}, + {Term: &Term{Type: TermTypeFunc, Func: &Func{Name: name}}}, + }, }, - }, - }}, + }}, + }, }, - alloc, ) } } -const ( - // Appended to the names of the _assign and _modify variants taking an - // allocator as an additional argument. The resulting name is not a valid - // function name, so that a query cannot call such a variant directly with - // an arbitrary value in place of the allocator. - allocatorArgSuffix = "/3" - - // The name of the variable holding the allocator of a reduce expression. - // It is not a valid variable name, just like the variable of the operand - // of an update-assignment operator. - allocatorName = "$%allocator" -) - -// compileUpdateFunc appends a call to _assign or _modify, or to the variant -// taking the allocator to update the input value with as its third argument. -func (c *compiler) compileUpdateFunc(name string, args []*Query, alloc *Query) error { - if alloc == nil { - return c.compileFunc(&Func{Name: name, Args: args}) - } - args = append(args, alloc) - fn := c.lookupBuiltin(name+allocatorArgSuffix, len(args)) - if fn == nil { - switch name { - case "_assign": - c.compileAssign(true) - case "_modify": - c.compileModify(true) - } - fn = c.lookupBuiltin(name+allocatorArgSuffix, len(args)) - } - return c.compileCallPc(fn, args) -} - func (c *compiler) compileBind(l, r *Query, patterns []*Pattern) error { defer c.newScopeDepth()() c.append(&code{op: opdup}) @@ -751,25 +716,6 @@ func (c *compiler) compileReduce(e *Reduce) error { } f() c.append(&code{op: opstore, v: v}) - // The accumulated value is passed to the update query and replaced by its - // result, so an assignment forming the whole update query is the only - // thing that observes it. Such an assignment may modify the accumulated - // value in place, instead of copying it on every iteration, as long as it - // keeps track of the values it allocated; see Query#inPlaceUpdate. The - // allocator is created once, outside the loop, and lives until the - // reduction finishes. - l, r, op, inPlace := e.Update.inPlaceUpdate() - var alloc *Query - if inPlace { - c.appends( - &code{op: oppush, v: nil}, - &code{op: opcall, v: [3]any{funcAllocator, 0, "_allocator"}}, - &code{op: opstore, v: c.pushVariable(allocatorName)}, - ) - alloc = &Query{Term: &Term{ - Type: TermTypeFunc, Func: &Func{Name: allocatorName}, - }} - } setfork := c.lazy(func() *code { return &code{op: opfork, v: len(c.codes)} }) @@ -781,13 +727,7 @@ func (c *compiler) compileReduce(e *Reduce) error { } c.append(&code{op: opload, v: v}) f = c.newScopeDepth() - var err error - if inPlace { - err = c.compileQueryUpdate(l, r, op, alloc) - } else { - err = c.compileQuery(e.Update) - } - if err != nil { + if err := c.compileQuery(e.Update); err != nil { return err } f() @@ -992,9 +932,9 @@ func (c *compiler) compileFunc(e *Func) error { if !compiled { switch e.Name { case "_assign": - c.compileAssign(false) + c.compileAssign() case "_modify": - c.compileModify(false) + c.compileModify() case "_last": c.compileLast() } @@ -1090,56 +1030,29 @@ func (c *compiler) compileFunc(e *Func) error { // // To overcome the difficulty of reducing allocations on `setpath`, we use the // `allocator` type and track the allocated addresses during the reduction. -// -// The shared variant takes the allocator of the enclosing reduce expression as -// its third argument instead of creating a new one, so that the value being -// assigned to is modified in place on every iteration but the first; see -// compiler#compileReduce. -func (c *compiler) compileAssign(shared bool) { - name, argcnt := "_assign", 2 - if shared { - name, argcnt = name+allocatorArgSuffix, 3 - } - defer c.appendBuiltin(name, argcnt)() +func (c *compiler) compileAssign() { + defer c.appendBuiltin("_assign", 2)() scope := c.newScope() v, p := [2]int{scope.id, 0}, [2]int{scope.id, 1} x, a := [2]int{scope.id, 2}, [2]int{scope.id, 3} // Cannot reuse v, p due to backtracking in x. w, q := [2]int{scope.id, 4}, [2]int{scope.id, 5} c.appends( - &code{op: opscope, v: [3]int{scope.id, 6, argcnt}}, + &code{op: opscope, v: [3]int{scope.id, 6, 2}}, &code{op: opstore, v: v}, // def _assign(p; $x): &code{op: opstore, v: p}, &code{op: opstore, v: x}, - ) - if shared { - c.append(&code{op: opstore, v: a}) // the allocator argument - } - c.appends( &code{op: opload, v: v}, &code{op: opexpbegin}, &code{op: opload, v: x}, &code{op: opcallpc}, &code{op: opstore, v: x}, &code{op: opexpend}, - ) - if shared { - c.appends( - &code{op: opload, v: v}, - &code{op: opload, v: a}, - &code{op: opcallpc}, - &code{op: opstore, v: a}, - ) - } else { - c.appends( - &code{op: oppush, v: nil}, - &code{op: opcall, v: [3]any{funcAllocator, 0, "_allocator"}}, - &code{op: opstore, v: a}, - ) - } - c.appends( + &code{op: oppush, v: nil}, + &code{op: opcall, v: [3]any{funcAllocator, 0, "_allocator"}}, + &code{op: opstore, v: a}, &code{op: opload, v: v}, - &code{op: opfork, v: len(c.codes) + 17}, // reduce [L1] + &code{op: opfork, v: len(c.codes) + 30}, // reduce [L1] &code{op: opdup}, &code{op: opstore, v: w}, &code{op: oppathbegin}, // path(p) @@ -1163,50 +1076,24 @@ func (c *compiler) compileAssign(shared bool) { // Appends the compiled code for the update-assignment operator (`|=`) to // maximize performance. We use the `allocator` type, just like `_assign/2`. -// -// The shared variant takes the allocator of the enclosing reduce expression as -// its third argument instead of creating a new one; see -// compiler#compileReduce and compiler#compileAssign. -func (c *compiler) compileModify(shared bool) { - name, argcnt := "_modify", 2 - if shared { - name, argcnt = name+allocatorArgSuffix, 3 - } - defer c.appendBuiltin(name, argcnt)() +func (c *compiler) compileModify() { + defer c.appendBuiltin("_modify", 2)() scope := c.newScope() v, p := [2]int{scope.id, 0}, [2]int{scope.id, 1} f, d := [2]int{scope.id, 2}, [2]int{scope.id, 3} a, l := [2]int{scope.id, 4}, [2]int{scope.id, 5} c.appends( - &code{op: opscope, v: [3]int{scope.id, 6, argcnt}}, + &code{op: opscope, v: [3]int{scope.id, 6, 2}}, &code{op: opstore, v: v}, // def _modify(p; f): &code{op: opstore, v: p}, &code{op: opstore, v: f}, - ) - if shared { - c.append(&code{op: opstore, v: a}) // the allocator argument - } - c.appends( &code{op: oppush, v: []any{}}, &code{op: opstore, v: d}, - ) - if shared { - c.appends( - &code{op: opload, v: v}, - &code{op: opload, v: a}, - &code{op: opcallpc}, - &code{op: opstore, v: a}, - ) - } else { - c.appends( - &code{op: oppush, v: nil}, - &code{op: opcall, v: [3]any{funcAllocator, 0, "_allocator"}}, - &code{op: opstore, v: a}, - ) - } - c.appends( + &code{op: oppush, v: nil}, + &code{op: opcall, v: [3]any{funcAllocator, 0, "_allocator"}}, + &code{op: opstore, v: a}, &code{op: opload, v: v}, - &code{op: opfork, v: len(c.codes) + 31}, // reduce [L1] + &code{op: opfork, v: len(c.codes) + 39}, // reduce [L1] &code{op: oppathbegin}, // path(p) &code{op: opload, v: p}, &code{op: opcallpc}, @@ -1215,13 +1102,12 @@ func (c *compiler) compileModify(shared bool) { &code{op: opstore, v: p}, // as $p (.; &code{op: opforklabel, v: l}, // label $l | &code{op: opload, v: v}, // - &code{op: opfork, v: len(c.codes) + 28}, // [L2] + &code{op: opfork, v: len(c.codes) + 36}, // [L2] &code{op: oppop}, // (getpath($p) | &code{op: opload, v: a}, - &code{op: opload, v: a}, &code{op: opload, v: p}, &code{op: opload, v: v}, - &code{op: opcall, v: [3]any{funcGetpathWithAllocator, 2, "getpath"}}, + &code{op: opcall, v: [3]any{internalFuncs["getpath"].callback, 1, "getpath"}}, &code{op: opload, v: f}, // f) &code{op: opcallpc}, &code{op: opload, v: p}, // setpath($p; ...) @@ -1229,8 +1115,8 @@ func (c *compiler) compileModify(shared bool) { &code{op: opcall, v: [3]any{funcSetpathWithAllocator, 3, "_setpath"}}, &code{op: opstore, v: v}, &code{op: opload, v: v}, // ., break $l - &code{op: opfork, v: len(c.codes) + 26}, // [L4] - &code{op: opjump, v: len(c.codes) + 30}, // [L3] + &code{op: opfork, v: len(c.codes) + 34}, // [L4] + &code{op: opjump, v: len(c.codes) + 38}, // [L3] &code{op: opload, v: l}, // [L4] &code{op: opcall, v: [3]any{funcBreak(""), 0, "_break"}}, &code{op: opload, v: p}, // append $p to $d [L2] @@ -1383,11 +1269,7 @@ func listModuleDeps(q *Query) []any { if v == nil { v = make(map[string]any) } - relpath := i.ImportPath - if relpath == "" { - relpath = i.IncludePath - } - v["relpath"] = relpath + v["relpath"] = cmp.Or(i.ImportPath, i.IncludePath) if i.ImportAlias != "" { v["as"] = strings.TrimPrefix(i.ImportAlias, "$") } diff --git a/func.go b/func.go index d352716..81dbe31 100644 --- a/func.go +++ b/func.go @@ -1488,17 +1488,6 @@ func (a allocator) allocated(v any) bool { return ok } -// release forgets every allocated address when the value is one of them, so -// that a value handed to a query is copied on update again. -func (a allocator) release(v any) { - switch v.(type) { - case map[string]any, []any: - if a.allocated(v) { - clear(a) - } - } -} - func (a allocator) makeObject(l int) map[string]any { v := make(map[string]any, l) if a != nil { @@ -1525,15 +1514,6 @@ func funcSetpathWithAllocator(v any, args []any) any { return setpath(v, args[0], args[1], args[2].(allocator)) } -// Used in compiler#compileModify: the update function may store the value -// anywhere, so it must not be modified in place afterwards. Registered as -// getpath to keep tracking paths through the call. -func funcGetpathWithAllocator(v any, args []any) any { - w := funcGetpath(v, args[0]) - args[1].(allocator).release(w) - return w -} - func setpath(v, p, n any, a allocator) any { path, ok := p.([]any) if !ok {