internal/core/dep: adapt Recurse to mimic dep.Visit

https://cuelang.org/cl/556460 swapped two indirect calls to func Visit
with calls to the method visitor.visit, allowing better memory reuse.

However, it missed that the two call sites weren't equal semantically:

* The call site in internal/core/export/self.go called markDeps,
  which then called dep.Visit with the Config.Descend option set.

* The call site in tools/flow/tasks called markTaskDependencies,
  which then called dep.Visit with the Config.Dynamic option set.

The former caused Visit to call visitor.visit, like our new code,
but the latter caused Visit to call visitor.dynamic,
which our new code certainly does not do.

To fix this unintentional change in behavior in the refactor above,
factor out the logic based on Dynamic so that Recurse can reuse it,
and remember the value of Dynamic in visitor so Recurse can apply it.

In the added test output, we can now see that root.run depends on
root.prepare again, like v0.5.0 did, and unlike the previous commit.

Fixes #2517.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I299000385ffdfa79218ddbe8b8faa901b5006a9d
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/557477
Reviewed-by: Marcel van Lohuizen <mpvl@gmail.com>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>

internal/core/dep/dep.go

@@ -132,7 +132,7 @@ func (d *Dependency) Recurse() {
 	d.visitor.all = d.visitor.recurse
 	d.visitor.top = true
 
-	d.visitor.visit(d.Node, false)
+	d.visitor.visitReusingVisitor(d.Node, false)
 
 	d.visitor.all = savedAll
 	d.visitor.top = savedTop
@@ -186,24 +186,29 @@ func Visit(cfg *Config, c *adt.OpContext, n *adt.Vertex, f VisitFunc) error {
 		panic("nil context")
 	}
 	v := visitor{
-		ctxt:    c,
-		fn:      f,
-		pkg:     cfg.Pkg,
-		recurse: cfg.Descend,
-		all:     cfg.Descend,
-		top:     true,
+		ctxt:       c,
+		fn:         f,
+		pkg:        cfg.Pkg,
+		recurse:    cfg.Descend,
+		all:        cfg.Descend,
+		top:        true,
+		cfgDynamic: cfg.Dynamic,
 	}
+	return v.visitReusingVisitor(n, true)
+}
 
-	if cfg.Dynamic {
-		v.marked = marked{}
-
+// visitReusingVisitor is factored out of Visit so that we may reuse visitor.
+func (v *visitor) visitReusingVisitor(n *adt.Vertex, top bool) error {
+	if v.cfgDynamic {
+		if v.marked == nil {
+			v.marked = marked{}
+		}
 		v.marked.markExpr(n)
 
-		v.dynamic(n, true)
+		v.dynamic(n, top)
 	} else {
-		v.visit(n, true)
+		v.visit(n, top)
 	}
-
 	return v.err
 }
 
@@ -255,6 +260,9 @@ type visitor struct {
 	pathStack []refEntry
 	numRefs   int // count of reported dependencies
 
+	// cfgDynamic is kept from the original config.
+	cfgDynamic bool
+
 	marked marked
 }
 

tools/flow/testdata/issue2517.txtar

@@ -0,0 +1,79 @@
+#IgnoreConcrete: true
+#InferTasks: true
+-- in.cue --
+package x
+
+root: {
+	prepare: {
+		$id: "prepare"
+	}
+	env2: {
+		input: prepare.stdout
+	}
+	run: {
+		$id: "run"
+		env: env2
+	}
+}
+-- out/run/errors --
+-- out/run/t0 --
+graph TD
+  t0("root.prepare [Ready]")
+  t1("root.run [Waiting]")
+  t1-->t0
+
+-- out/run/t1 --
+graph TD
+  t0("root.prepare [Terminated]")
+  t1("root.run [Ready]")
+  t1-->t0
+
+-- out/run/t1/value --
+{
+	$id:    "prepare"
+	stdout: "foo"
+}
+-- out/run/t1/stats --
+Leaks:  0
+Freed:  11
+Reused: 6
+Allocs: 5
+Retain: 0
+
+Unifications: 11
+Conjuncts:    17
+Disjuncts:    11
+-- out/run/t2 --
+graph TD
+  t0("root.prepare [Terminated]")
+  t1("root.run [Terminated]")
+  t1-->t0
+
+-- out/run/t2/value --
+{
+	$id:    "run"
+	stdout: "foo"
+	env: {
+		input: "foo"
+	}
+}
+-- out/run/t2/stats --
+Leaks:  0
+Freed:  12
+Reused: 12
+Allocs: 0
+Retain: 0
+
+Unifications: 12
+Conjuncts:    21
+Disjuncts:    12
+-- out/run/stats/totals --
+Leaks:  0
+Freed:  23
+Reused: 18
+Allocs: 5
+Retain: 0
+
+Unifications: 23
+Conjuncts:    38
+Disjuncts:    23