go/analysis/passes/modernize: fix the potential deadlock issues in bloop

For golang/go#75599

Change-Id: Ic8009f9a7bcbd8f76b47bf6831132aeb0cbee2f3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/711980
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
TryBot-Bypass: Alan Donovan <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>

Author: cuishuang

go/analysis/passes/modernize/bloop.go

@@ -189,6 +189,7 @@ func enclosingFunc(c inspector.Cursor) (inspector.Cursor, bool) {
 // 2. The only b.N loop in that benchmark function
 //   - b.Loop() can only be called once per benchmark execution
 //   - Multiple calls result in "B.Loop called with timer stopped" error
+//   - Multiple loops may have complex interdependencies that are hard to analyze
 func usesBenchmarkNOnce(c inspector.Cursor, info *types.Info) bool {
 	// Find the enclosing benchmark function
 	curFunc, ok := enclosingFunc(c)
@@ -205,17 +206,14 @@ func usesBenchmarkNOnce(c inspector.Cursor, info *types.Info) bool {
 		return false
 	}
 
-	// Count b.N references in this benchmark function
+	// Count all b.N references in this benchmark function (including nested functions)
 	bnRefCount := 0
-	filter := []ast.Node{(*ast.SelectorExpr)(nil), (*ast.FuncLit)(nil)}
+	filter := []ast.Node{(*ast.SelectorExpr)(nil)}
 	curFunc.Inspect(filter, func(cur inspector.Cursor) bool {
-		switch n := cur.Node().(type) {
-		case *ast.FuncLit:
-			return false // don't descend into nested function literals
-		case *ast.SelectorExpr:
-			if n.Sel.Name == "N" && typesinternal.IsPointerToNamed(info.TypeOf(n.X), "testing", "B") {
-				bnRefCount++
-			}
+		sel := cur.Node().(*ast.SelectorExpr)
+		if sel.Sel.Name == "N" &&
+			typesinternal.IsPointerToNamed(info.TypeOf(sel.X), "testing", "B") {
+			bnRefCount++
 		}
 		return true
 	})

go/analysis/passes/modernize/testdata/src/bloop/bloop_test.go

@@ -113,3 +113,21 @@ func BenchmarkI(b *testing.B) {
 	for i := 0; i < b.N; i++ { // nope: b.N accessed more than once in benchmark
 	}
 }
+
+func BenchmarkJ(b *testing.B) {
+	var wg sync.WaitGroup
+	ch := make(chan int, 10)
+	wg.Add(1)
+	go func() {
+		for i := 0; i < b.N; i++ {
+			<-ch
+		}
+		wg.Done()
+	}()
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ { // nope: multiple b.N loops in same function
+		ch <- i
+	}
+	b.StopTimer()
+	wg.Wait()
+}

go/analysis/passes/modernize/testdata/src/bloop/bloop_test.go.golden

@@ -111,3 +111,21 @@ func BenchmarkI(b *testing.B) {
 	for i := 0; i < b.N; i++ { // nope: b.N accessed more than once in benchmark
 	}
 }
+
+func BenchmarkJ(b *testing.B) {
+	var wg sync.WaitGroup
+	ch := make(chan int, 10)
+	wg.Add(1)
+	go func() {
+		for i := 0; i < b.N; i++ {
+			<-ch
+		}
+		wg.Done()
+	}()
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ { // nope: multiple b.N loops in same function
+		ch <- i
+	}
+	b.StopTimer()
+	wg.Wait()
+}