internal/trace: refactor Stack.Frames to return iter.Seq

The Frames function is almost an iter.Seq, except for its bool return
value.

Since none of the callers in the Go tree rely on the bool, we can remove
it. However, doing so might still obscure the intended usage as an iterator.

This refactor changes the API to return iter.Seq, making the intended
usage explicit. Refactoring the existing callers to take advantage of
the new interface will be done in a follow-up CL.

Change-Id: I03e4d6d762910e418cc37d59a6c519eb7f39b3b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/608855
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>

diff --git a/src/cmd/trace/gstate.go b/src/cmd/trace/gstate.go
index 638d492670a6e74cfe3945db9b77eb002bd49caa..76c58073b33e853952d84eb46f66e9eb43e4a190 100644 (file)
--- a/src/cmd/trace/gstate.go
+++ b/src/cmd/trace/gstate.go
@@ -364,7 +364,7 @@ func (gs *gState[R]) rangeEnd(ts trace.Time, name string, stack trace.Stack, ctx
 
 func lastFunc(s trace.Stack) string {
        var last trace.StackFrame
-       s.Frames(func(f trace.StackFrame) bool {
+       s.Frames()(func(f trace.StackFrame) bool {
                last = f
                return true
        })

diff --git a/src/cmd/trace/pprof.go b/src/cmd/trace/pprof.go
index c3e5a3a0450f0302b5b93395d26e768f59f67d87..856b97b75fed9d6d847ed99046d26f8e733084e2 100644 (file)
--- a/src/cmd/trace/pprof.go
+++ b/src/cmd/trace/pprof.go
@@ -307,7 +307,7 @@ func (m *stackMap) profile() []traceviewer.ProfileRecord {
        for stack, record := range m.stacks {
                rec := *record
                i := 0
-               stack.Frames(func(frame trace.StackFrame) bool {
+               stack.Frames()(func(frame trace.StackFrame) bool {
                        rec.Stack = append(rec.Stack, &trace.Frame{
                                PC:   frame.PC,
                                Fn:   frame.Func,
@@ -327,7 +327,7 @@ func (m *stackMap) profile() []traceviewer.ProfileRecord {
 // pcsForStack extracts the first pprofMaxStack PCs from stack into pcs.
 func pcsForStack(stack trace.Stack, pcs *[pprofMaxStack]uint64) {
        i := 0
-       stack.Frames(func(frame trace.StackFrame) bool {
+       stack.Frames()(func(frame trace.StackFrame) bool {
                pcs[i] = frame.PC
                i++
                return i < len(pcs)

diff --git a/src/cmd/trace/regions.go b/src/cmd/trace/regions.go
index cb04190fd8d9e053192fbd74f577dad4a8a0499d..cae38355a58019af943d7a74b2cf501e80d9d1e5 100644 (file)
--- a/src/cmd/trace/regions.go
+++ b/src/cmd/trace/regions.go
@@ -72,7 +72,7 @@ func fingerprintRegion(r *trace.UserRegionSummary) regionFingerprint {
 func regionTopStackFrame(r *trace.UserRegionSummary) trace.StackFrame {
        var frame trace.StackFrame
        if r.Start != nil && r.Start.Stack() != trace.NoStack {
-               r.Start.Stack().Frames(func(f trace.StackFrame) bool {
+               r.Start.Stack().Frames()(func(f trace.StackFrame) bool {
                        frame = f
                        return false
                })

diff --git a/src/cmd/trace/viewer.go b/src/cmd/trace/viewer.go
index 79c9583b0d3bfed7ad53e71b57669eb370117aa1..c367c7d6366b5fe1614f1debb978f30e5d9ce8f5 100644 (file)
--- a/src/cmd/trace/viewer.go
+++ b/src/cmd/trace/viewer.go
@@ -15,7 +15,7 @@ import (
 // used to store the frames to reduce allocations.
 func viewerFrames(stk trace.Stack) []*trace.Frame {
        var frames []*trace.Frame
-       stk.Frames(func(f trace.StackFrame) bool {
+       stk.Frames()(func(f trace.StackFrame) bool {
                frames = append(frames, &trace.Frame{
                        PC:   f.PC,
                        Fn:   f.Func,

diff --git a/src/internal/trace/event.go b/src/internal/trace/event.go
index 7d869e885fc2c91e1c9254b0f91a750bb051f588..90effce65336aff667583eeed4770fccba083e83 100644 (file)
--- a/src/internal/trace/event.go
+++ b/src/internal/trace/event.go
@@ -6,6 +6,7 @@ package trace
 
 import (
        "fmt"
+       "iter"
        "math"
        "strings"
        "time"
@@ -265,24 +266,25 @@ type Stack struct {
 }
 
 // Frames is an iterator over the frames in a Stack.
-func (s Stack) Frames(yield func(f StackFrame) bool) bool {
-       if s.id == 0 {
-               return true
-       }
-       stk := s.table.stacks.mustGet(s.id)
-       for _, pc := range stk.pcs {
-               f := s.table.pcs[pc]
-               sf := StackFrame{
-                       PC:   f.pc,
-                       Func: s.table.strings.mustGet(f.funcID),
-                       File: s.table.strings.mustGet(f.fileID),
-                       Line: f.line,
+func (s Stack) Frames() iter.Seq[StackFrame] {
+       return func(yield func(StackFrame) bool) {
+               if s.id == 0 {
+                       return
                }
-               if !yield(sf) {
-                       return false
+               stk := s.table.stacks.mustGet(s.id)
+               for _, pc := range stk.pcs {
+                       f := s.table.pcs[pc]
+                       sf := StackFrame{
+                               PC:   f.pc,
+                               Func: s.table.strings.mustGet(f.funcID),
+                               File: s.table.strings.mustGet(f.fileID),
+                               Line: f.line,
+                       }
+                       if !yield(sf) {
+                               return
+                       }
                }
        }
-       return true
 }
 
 // NoStack is a sentinel value that can be compared against any Stack value, indicating
@@ -796,7 +798,7 @@ func (e Event) String() string {
                if s.Stack != NoStack {
                        fmt.Fprintln(&sb)
                        fmt.Fprintln(&sb, "TransitionStack=")
-                       s.Stack.Frames(func(f StackFrame) bool {
+                       s.Stack.Frames()(func(f StackFrame) bool {
                                fmt.Fprintf(&sb, "\t%s @ 0x%x\n", f.Func, f.PC)
                                fmt.Fprintf(&sb, "\t\t%s:%d\n", f.File, f.Line)
                                return true
@@ -809,7 +811,7 @@ func (e Event) String() string {
        if stk := e.Stack(); stk != NoStack {
                fmt.Fprintln(&sb)
                fmt.Fprintln(&sb, "Stack=")
-               stk.Frames(func(f StackFrame) bool {
+               stk.Frames()(func(f StackFrame) bool {
                        fmt.Fprintf(&sb, "\t%s @ 0x%x\n", f.Func, f.PC)
                        fmt.Fprintf(&sb, "\t\t%s:%d\n", f.File, f.Line)
                        return true

diff --git a/src/internal/trace/summary.go b/src/internal/trace/summary.go
index fa3e3359c7c59d6fe2413c453c5a830635313364..1cd506ac5a7cf6b6697014e378132144f076350e 100644 (file)
--- a/src/internal/trace/summary.go
+++ b/src/internal/trace/summary.go
@@ -394,7 +394,7 @@ func (s *Summarizer) Event(ev *Event) {
                                if stk != NoStack {
                                        var frame StackFrame
                                        var ok bool
-                                       stk.Frames(func(f StackFrame) bool {
+                                       stk.Frames()(func(f StackFrame) bool {
                                                frame = f
                                                ok = true
                                                return true

diff --git a/src/internal/trace/testtrace/validation.go b/src/internal/trace/testtrace/validation.go
index ec492110e2a29d4129e3aaf0e8a355954e921dde..42a561f1f033d09101021eec0dcca578786162d5 100644 (file)
--- a/src/internal/trace/testtrace/validation.go
+++ b/src/internal/trace/testtrace/validation.go
@@ -351,7 +351,7 @@ func (v *Validator) getOrCreateThread(e *errAccumulator, ev trace.Event, m trace
 func checkStack(e *errAccumulator, stk trace.Stack) {
        // Check for non-empty values, but we also check for crashes due to incorrect validation.
        i := 0
-       stk.Frames(func(f trace.StackFrame) bool {
+       stk.Frames()(func(f trace.StackFrame) bool {
                if i == 0 {
                        // Allow for one fully zero stack.
                        //

diff --git a/src/internal/trace/trace_test.go b/src/internal/trace/trace_test.go
index 1929069cc5da97edbaeb69c8ba60a085b9c44974..dcf9d05fb4e2b886a472708034a6979774bb40d0 100644 (file)
--- a/src/internal/trace/trace_test.go
+++ b/src/internal/trace/trace_test.go
@@ -148,7 +148,7 @@ func TestTraceCPUProfile(t *testing.T) {
                                if hogRegion != nil && ev.Goroutine() == hogRegion.Goroutine() {
                                        traceSamples++
                                        var fns []string
-                                       ev.Stack().Frames(func(frame trace.StackFrame) bool {
+                                       ev.Stack().Frames()(func(frame trace.StackFrame) bool {
                                                if frame.Func != "runtime.goexit" {
                                                        fns = append(fns, fmt.Sprintf("%s:%d", frame.Func, frame.Line))
                                                }
@@ -438,7 +438,7 @@ func TestTraceStacks(t *testing.T) {
                stackMatches := func(stk trace.Stack, frames []frame) bool {
                        i := 0
                        match := true
-                       stk.Frames(func(f trace.StackFrame) bool {
+                       stk.Frames()(func(f trace.StackFrame) bool {
                                if f.Func != frames[i].fn {
                                        match = false
                                        return false

diff --git a/src/runtime/trace_cgo_test.go b/src/runtime/trace_cgo_test.go
index f0db3b7ffb7624f8e9cce9102a9fc817be12f2f3..28b6f7833ee3eacab2271e4a6c70b0dbde0f57a1 100644 (file)
--- a/src/runtime/trace_cgo_test.go
+++ b/src/runtime/trace_cgo_test.go
@@ -94,7 +94,7 @@ func mustFindLogV2(t *testing.T, trc io.Reader, category string) trace.Event {
 // dumpStack returns e.Stack() as a string.
 func dumpStackV2(e *trace.Event) string {
        var buf bytes.Buffer
-       e.Stack().Frames(func(f trace.StackFrame) bool {
+       e.Stack().Frames()(func(f trace.StackFrame) bool {
                file := strings.TrimPrefix(f.File, runtime.GOROOT())
                fmt.Fprintf(&buf, "%s\n\t%s:%d\n", f.Func, file, f.Line)
                return true