cmd/digraph: fix bug in allpaths

The previous algorithm forgot to follow all paths through
the target node. Use a simpler algorithm.

Thanks to Gemini for one of the test cases.

Fixes golang/go#74842

Change-Id: I7bb9290c3e78c7094e4a840b894b0906c25cb42c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/692675
Commit-Queue: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Alan Donovan <adonovan@google.com>

Author: adonovan

cmd/digraph/digraph.go

@@ -67,6 +67,8 @@ func (l nodelist) println(sep string) {
 
 type nodeset map[string]bool
 
+func singleton(x string) nodeset { return nodeset{x: true} }
+
 func (s nodeset) sort() nodelist {
 	nodes := make(nodelist, len(s))
 	var i int
@@ -191,26 +193,17 @@ func (g graph) sccs() []nodeset {
 }
 
 func (g graph) allpaths(from, to string) error {
-	// Mark all nodes to "to".
-	seen := make(nodeset) // value of seen[x] indicates whether x is on some path to "to"
-	var visit func(node string) bool
-	visit = func(node string) bool {
-		reachesTo, ok := seen[node]
-		if !ok {
-			reachesTo = node == to
-			seen[node] = reachesTo
-			for e := range g[node] {
-				if visit(e) {
-					reachesTo = true
-				}
-			}
-			if reachesTo && node != to {
-				seen[node] = true
-			}
+	// We intersect the forward closure of 'from' with
+	// the reverse closure of 'to'. This is not the most
+	// efficient implementation, but it's the clearest,
+	// and the previous one had bugs.
+	seen := g.reachableFrom(singleton(from))
+	rev := g.transpose().reachableFrom(singleton(to))
+	for n := range seen {
+		if !rev[n] {
+			delete(seen, n)
 		}
-		return reachesTo
 	}
-	visit(from)
 
 	// For each marked node, collect its marked successors.
 	var edges []string
@@ -241,7 +234,7 @@ func (g graph) somepath(from, to string) error {
 		tail *path
 	}
 
-	seen := nodeset{from: true}
+	seen := singleton(from)
 
 	var queue []*path
 	queue = append(queue, &path{node: from, tail: nil})
@@ -469,14 +462,14 @@ func digraph(cmd string, args []string) error {
 		}
 
 		edges := make(map[string]struct{})
-		for from := range g.reachableFrom(nodeset{node: true}) {
+		for from := range g.reachableFrom(singleton(node)) {
 			for to := range g[from] {
 				edges[fmt.Sprintf("%s %s", from, to)] = struct{}{}
 			}
 		}
 
 		gtrans := g.transpose()
-		for from := range gtrans.reachableFrom(nodeset{node: true}) {
+		for from := range gtrans.reachableFrom(singleton(node)) {
 			for to := range gtrans[from] {
 				edges[fmt.Sprintf("%s %s", to, from)] = struct{}{}
 			}

cmd/digraph/digraph_test.go

@@ -156,6 +156,33 @@ func TestAllpaths(t *testing.T) {
 			to:   "H",
 			want: "A B\nA C\nB D\nC D\nD E\nE F\nE G\nF H\nG H\n",
 		},
+		{
+			// C <--> B --> A --> D <--> E
+			//        ⋃
+			name: "non-regression test for #74842",
+			in:   "A D\nB A\nB B\nB C\nC B\nD E\nE D",
+			to:   "D",
+			want: "A D\nD E\nE D\n",
+		},
+		{
+			// A --> B --> D
+			//       ^
+			//       v
+			//       C[123]
+			name: "regression test for #74842",
+			in:   "A B\nB C1\nB C2\nB C3\nB D\nC1 B\nC2 B\nC3 B\n",
+			to:   "D",
+			want: "A B\nB C1\nB C2\nB C3\nB D\nC1 B\nC2 B\nC3 B\n",
+		},
+		{
+			// A -------> B --> D
+			//  \--> C ---^     |
+			//       ^----------+
+			name: "another regression test for #74842",
+			in:   "A B\nA C\nB D\nC B\nD C\n",
+			to:   "D",
+			want: "A B\nA C\nB D\nC B\nD C\n",
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			stdin = strings.NewReader(test.in)

cmd/digraph/doc.go

@@ -28,9 +28,9 @@ The supported commands are:
 	reverse <node> ...
 		the set of nodes that transitively reach the specified nodes
 	somepath <node> <node>
-		the list of nodes on some arbitrary path from the first node to the second
+		the list of edges on some arbitrary path from the first node to the second
 	allpaths <node> <node>
-		the set of nodes on all paths from the first node to the second
+		the set of edges on all paths from the first node to the second
 	sccs
 		all strongly connected components (one per line)
 	scc <node>