go/ssa: pop targets stack on range-over-func
Pop Function.targets when building a call to a range-over-func yield function and when building the yield function.
Also adds sanity checks to ensure all function transient fields are cleared.
Fixes golang/go#69298
Change-Id: I38b80ce8939cf2cd6cfd0ce0c119d75356d80ebf
GitHub-Last-Rev: 8c45b9c36e370bc74bdf765b0aa37b743735db8e
GitHub-Pull-Request: golang/tools#516
Reviewed-on: https://go-review.googlesource.com/c/tools/+/611055
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Tim King <taking@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
diff --git a/go/ssa/builder.go b/go/ssa/builder.go
index f1fa43c..cd29fed 100644
--- a/go/ssa/builder.go
+++ b/go/ssa/builder.go
@@ -2566,6 +2566,8 @@
emitJump(fn, done)
fn.currentBlock = done
+ // pop the stack for the range-over-func
+ fn.targets = fn.targets.tail
}
// buildYieldResume emits to fn code for how to resume execution once a call to
@@ -2998,6 +3000,7 @@
}
}
fn.targets = &targets{
+ tail: fn.targets,
_continue: ycont,
// `break` statement targets fn.parent.targets._break.
}
@@ -3075,6 +3078,8 @@
// unreachable.
emitJump(fn, ycont)
}
+ // pop the stack for the yield function
+ fn.targets = fn.targets.tail
// Clean up exits and promote any unresolved exits to fn.parent.
for _, e := range fn.exits {
diff --git a/go/ssa/interp/interp_go122_test.go b/go/ssa/interp/interp_go122_test.go
index aedb588..6e2ab80 100644
--- a/go/ssa/interp/interp_go122_test.go
+++ b/go/ssa/interp/interp_go122_test.go
@@ -39,6 +39,18 @@
run(t, filepath.Join(cwd, "testdata", "rangeoverint.go"), goroot)
}
+func TestIssue69298(t *testing.T) {
+ testenv.NeedsGo1Point(t, 23)
+
+ // TODO: Is cwd actually needed here?
+ goroot := makeGoroot(t)
+ cwd, err := os.Getwd()
+ if err != nil {
+ log.Fatal(err)
+ }
+ run(t, filepath.Join(cwd, "testdata", "fixedbugs/issue69298.go"), goroot)
+}
+
// TestRangeFunc tests range-over-func in a subprocess.
func TestRangeFunc(t *testing.T) {
testenv.NeedsGo1Point(t, 23)
diff --git a/go/ssa/interp/testdata/fixedbugs/issue69298.go b/go/ssa/interp/testdata/fixedbugs/issue69298.go
new file mode 100644
index 0000000..72ea0f5
--- /dev/null
+++ b/go/ssa/interp/testdata/fixedbugs/issue69298.go
@@ -0,0 +1,31 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+ "fmt"
+)
+
+type Seq[V any] func(yield func(V) bool)
+
+func AppendSeq[Slice ~[]E, E any](s Slice, seq Seq[E]) Slice {
+ for v := range seq {
+ s = append(s, v)
+ }
+ return s
+}
+
+func main() {
+ seq := func(yield func(int) bool) {
+ for i := 0; i < 10; i += 2 {
+ if !yield(i) {
+ return
+ }
+ }
+ }
+
+ s := AppendSeq([]int{1, 2}, seq)
+ fmt.Println(s)
+}
diff --git a/go/ssa/sanity.go b/go/ssa/sanity.go
index dfc9576..3d82e93 100644
--- a/go/ssa/sanity.go
+++ b/go/ssa/sanity.go
@@ -407,9 +407,9 @@
}
}
-func (s *sanity) checkFunctionParams(fn *Function) {
- signature := fn.Signature
- params := fn.Params
+func (s *sanity) checkFunctionParams() {
+ signature := s.fn.Signature
+ params := s.fn.Params
// startSigParams is the start of signature.Params() within params.
startSigParams := 0
@@ -438,12 +438,54 @@
}
}
+// checkTransientFields checks whether all transient fields of Function are cleared.
+func (s *sanity) checkTransientFields() {
+ fn := s.fn
+ if fn.build != nil {
+ s.errorf("function transient field 'build' is not nil")
+ }
+ if fn.currentBlock != nil {
+ s.errorf("function transient field 'currentBlock' is not nil")
+ }
+ if fn.vars != nil {
+ s.errorf("function transient field 'vars' is not nil")
+ }
+ if fn.results != nil {
+ s.errorf("function transient field 'results' is not nil")
+ }
+ if fn.returnVars != nil {
+ s.errorf("function transient field 'returnVars' is not nil")
+ }
+ if fn.targets != nil {
+ s.errorf("function transient field 'targets' is not nil")
+ }
+ if fn.lblocks != nil {
+ s.errorf("function transient field 'lblocks' is not nil")
+ }
+ if fn.subst != nil {
+ s.errorf("function transient field 'subst' is not nil")
+ }
+ if fn.jump != nil {
+ s.errorf("function transient field 'jump' is not nil")
+ }
+ if fn.deferstack != nil {
+ s.errorf("function transient field 'deferstack' is not nil")
+ }
+ if fn.source != nil {
+ s.errorf("function transient field 'source' is not nil")
+ }
+ if fn.exits != nil {
+ s.errorf("function transient field 'exits' is not nil")
+ }
+ if fn.uniq != 0 {
+ s.errorf("function transient field 'uniq' is not zero")
+ }
+}
+
func (s *sanity) checkFunction(fn *Function) bool {
s.fn = fn
- // TODO(yuchen): fix the bug caught on fn.targets when checking transient fields
- // see https://go-review.googlesource.com/c/tools/+/610059/comment/4287a123_dc6cbc44/
-
- s.checkFunctionParams(fn)
+ s.checkFunctionParams()
+ s.checkTransientFields()
// TODO(taking): Sanity check origin, typeparams, and typeargs.
if fn.Prog == nil {