Prevent duplicate visit calls in WalkDeps

WalkDeps was following every possible path through the dependency
tree, which can be enormous.  Modify it to only call visit for
any particular (child, parent) pair once for each direct dependency
by not recursing into child if visitDown returns true but child
has already been visited.

Test: TestWalkDeps, TestWalkDepsDuplicates
Change-Id: Ieef28399bd10e744417cdeb661dfa04fbeb4ec60

context.go

@@ -2397,15 +2397,15 @@ func (c *Context) walkDeps(topModule *moduleInfo, allowDuplicates bool,
 	walk = func(module *moduleInfo) {
 		for _, dep := range module.directDeps {
 			if allowDuplicates || !visited[dep.module] {
-				visited[dep.module] = true
 				visiting = dep.module
 				recurse := true
 				if visitDown != nil {
 					recurse = visitDown(dep, module)
 				}
-				if recurse {
+				if recurse && !visited[dep.module] {
 					walk(dep.module)
 				}
+				visited[dep.module] = true
 				if visitUp != nil {
 					visitUp(dep, module)
 				}

context_test.go

@@ -122,9 +122,9 @@ func TestContextParse(t *testing.T) {
 	}
 }
 
-// |---B===D       - represents a non-walkable edge
+// |===B---D       - represents a non-walkable edge
 // A               = represents a walkable edge
-// |===C---E===G
+// |===C===E---G
 //     |       |   A should not be visited because it's the root node.
 //     |===F===|   B, D and E should not be walked.
 func TestWalkDeps(t *testing.T) {
@@ -191,31 +191,29 @@ func TestWalkDeps(t *testing.T) {
 	topModule := ctx.modulesFromName("A", nil)[0]
 	ctx.walkDeps(topModule, false,
 		func(dep depInfo, parent *moduleInfo) bool {
+			outputDown += ctx.ModuleName(dep.module.logicModule)
 			if dep.module.logicModule.(Walker).Walk() {
-				outputDown += ctx.ModuleName(dep.module.logicModule)
 				return true
 			}
 			return false
 		},
 		func(dep depInfo, parent *moduleInfo) {
-			if dep.module.logicModule.(Walker).Walk() {
-				outputUp += ctx.ModuleName(dep.module.logicModule)
-			}
+			outputUp += ctx.ModuleName(dep.module.logicModule)
 		})
-	if outputDown != "CFG" {
-		t.Errorf("unexpected walkDeps behaviour: %s\ndown should be: CFG", outputDown)
+	if outputDown != "BCEFG" {
+		t.Errorf("unexpected walkDeps behaviour: %s\ndown should be: BCEFG", outputDown)
 	}
-	if outputUp != "GFC" {
-		t.Errorf("unexpected walkDeps behaviour: %s\nup should be: GFC", outputUp)
+	if outputUp != "BEGFC" {
+		t.Errorf("unexpected walkDeps behaviour: %s\nup should be: BEGFC", outputUp)
 	}
 }
 
-// |---B===D       - represents a non-walkable edge
-// A               = represents a walkable edge
-// |===C===E===\
-//     |       |   A should not be visited because it's the root node.
-//     |===F===G   B, D should not be walked.
-//         \===/   G should be visited multiple times
+// |===B---D           - represents a non-walkable edge
+// A                   = represents a walkable edge
+// |===C===E===\       A should not be visited because it's the root node.
+//     |       |       B, D should not be walked.
+//     |===F===G===H   G should be visited multiple times
+//         \===/       H should only be visited once
 func TestWalkDepsDuplicates(t *testing.T) {
 	ctx := NewContext()
 	ctx.MockFileSystem(map[string][]byte{
@@ -251,6 +249,11 @@ func TestWalkDepsDuplicates(t *testing.T) {
 
 			foo_module {
 			    name: "G",
+				deps: ["H"],
+			}
+
+			foo_module {
+			    name: "H",
 			}
 		`),
 	})
@@ -280,22 +283,20 @@ func TestWalkDepsDuplicates(t *testing.T) {
 	topModule := ctx.modulesFromName("A", nil)[0]
 	ctx.walkDeps(topModule, true,
 		func(dep depInfo, parent *moduleInfo) bool {
+			outputDown += ctx.ModuleName(dep.module.logicModule)
 			if dep.module.logicModule.(Walker).Walk() {
-				outputDown += ctx.ModuleName(dep.module.logicModule)
 				return true
 			}
 			return false
 		},
 		func(dep depInfo, parent *moduleInfo) {
-			if dep.module.logicModule.(Walker).Walk() {
-				outputUp += ctx.ModuleName(dep.module.logicModule)
-			}
+			outputUp += ctx.ModuleName(dep.module.logicModule)
 		})
-	if outputDown != "CEGFGG" {
-		t.Errorf("unexpected walkDeps behaviour: %s\ndown should be: CEGFGG", outputDown)
+	if outputDown != "BCEGHFGG" {
+		t.Errorf("unexpected walkDeps behaviour: %s\ndown should be: BCEGHFGG", outputDown)
 	}
-	if outputUp != "GEGGFC" {
-		t.Errorf("unexpected walkDeps behaviour: %s\nup should be: GEGGFC", outputUp)
+	if outputUp != "BHGEGGFC" {
+		t.Errorf("unexpected walkDeps behaviour: %s\nup should be: BHGEGGFC", outputUp)
 	}
 }
 

module_ctx.go

@@ -164,7 +164,7 @@ type ModuleContext interface {
 	VisitDirectDepsIf(pred func(Module) bool, visit func(Module))
 	VisitDepsDepthFirst(visit func(Module))
 	VisitDepsDepthFirstIf(pred func(Module) bool, visit func(Module))
-	WalkDeps(visit func(Module, Module) bool)
+	WalkDeps(visit func(child, parent Module) bool)
 
 	ModuleSubDir() string
 
@@ -446,10 +446,10 @@ func (m *baseModuleContext) VisitDepsDepthFirstIf(pred func(Module) bool,
 }
 
 // WalkDeps calls visit for each transitive dependency, traversing the dependency tree in top down order.  visit may be
-// called multiple times for the same module if there are multiple paths through the dependency tree to the module or
-// multiple direct dependencies with different tags.  OtherModuleDependencyTag will return the tag for the currently
-// visited path to the module.  If visit returns false WalkDeps will not continue recursing down the current path.
-func (m *baseModuleContext) WalkDeps(visit func(Module, Module) bool) {
+// called multiple times for the same (child, parent) pair if there are multiple direct dependencies between the
+// child and parent with different tags.  OtherModuleDependencyTag will return the tag for the currently visited
+// (child, parent) pair.  If visit returns false WalkDeps will not continue recursing down to child.
+func (m *baseModuleContext) WalkDeps(visit func(child, parent Module) bool) {
 	m.context.walkDeps(m.module, true, func(dep depInfo, parent *moduleInfo) bool {
 		m.visitingParent = parent
 		m.visitingDep = dep