[go] reflect: document that Seq and Seq2 panic on unexported values
Cyber Hamza (Gerrit)
Cyber Hamza has uploaded the change for review![]( "Open in Gerrit")
Commit message
reflect: document that Seq and Seq2 panic on unexported values
Fixes #74377
Change diff
diff --git a/src/reflect/iter.go b/src/reflect/iter.go
index 03df87b..9188cbe 100644
--- a/src/reflect/iter.go
+++ b/src/reflect/iter.go
@@ -35,6 +35,7 @@
// Otherwise v's kind must be Int, Int8, Int16, Int32, Int64,
// Uint, Uint8, Uint16, Uint32, Uint64, Uintptr,
// Array, Chan, Map, Slice, or String.
+// It panics if the Value was obtained by accessing unexported struct fields.
func (v Value) Seq() iter.Seq[Value] {
if canRangeFunc(v.abiType()) {
return func(yield func(Value) bool) {
@@ -121,6 +122,7 @@
// that takes a single argument of type func(K, V) bool for some type K, V.
// If v's kind is Pointer, the pointer element type must have kind Array.
// Otherwise v's kind must be Array, Map, Slice, or String.
+// It panics if the Value was obtained by accessing unexported struct fields.
func (v Value) Seq2() iter.Seq2[Value, Value] {
if canRangeFunc2(v.abiType()) {
return func(yield func(Value, Value) bool) {
diff --git a/src/reflect/iter_test.go b/src/reflect/iter_test.go
index 668d665..3a02b0d 100644
--- a/src/reflect/iter_test.go
+++ b/src/reflect/iter_test.go
@@ -410,3 +410,33 @@
// For Type.CanSeq2 test.
func (methodIter2) NonSeq2(yield func(int, int)) {}
+
+func TestSeqPanic(t *testing.T) {
+ // Seq and Seq2 panic if the value is unexported.
+ type s struct {
+ f1 func(int) bool
+ f2 func(int, int) bool
+ }
+ v := reflect.ValueOf(s{
+ f1: func(int) bool { return true },
+ f2: func(int, int) bool { return true },
+ })
+
+ t.Run("Seq", func(t *testing.T) {
+ defer func() {
+ if recover() == nil {
+ t.Error("Seq did not panic on unexported value")
+ }
+ }()
+ v.FieldByName("f1").Seq()
+ })
+
+ t.Run("Seq2", func(t *testing.T) {
+ defer func() {
+ if recover() == nil {
+ t.Error("Seq2 did not panic on unexported value")
+ }
+ }()
+ v.FieldByName("f2").Seq2()
+ })
+}
Change information
- M src/reflect/iter.go
- M src/reflect/iter_test.go
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gopher Robot (Gerrit)
Message from Gopher Robot![]( "Open in Gerrit")
Congratulations on opening your first change. Thank you for your contribution!
Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.
Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.
During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://go.dev/s/release
for more details.
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cherry Mui (Gerrit)
Cherry Mui voted and added 1 comment![]( "Open in Gerrit")
Votes added by Cherry Mui
| Hold | +1 |
1 comment
// It panics if the Value was obtained by accessing unexported struct fields.This is not correct. See the discussion in CL 690616. Thanks.
Related details
- Cyber Hamza
- Ian Lance Taylor
- Russ Cox
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cyber Hamza (Gerrit)
Cyber Hamza uploaded new patchset![]( "Open in Gerrit")
Related details
- Cyber Hamza
- Ian Lance Taylor
- Russ Cox
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cyber Hamza (Gerrit)
Cyber Hamza added 1 comment![]( "Open in Gerrit")
Thank you for the feedback! I've updated the documentation to only mention Func and Chan kinds, per the discussion in CL 690616. I also fixed the test to properly trigger the panic by iterating over the returned function, not just calling Seq()/Seq2(). Please let me know if you'd prefer CL 690616 to handle this instead — happy to defer.
Related details
- Ian Lance Taylor
- Russ Cox
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cherry Mui (Gerrit)
Cherry Mui voted and added 1 comment![]( "Open in Gerrit")
Votes added by Cherry Mui
| Hold | +1 |
1 comment
Thank you for the feedback! I've updated the documentation to only mention Func and Chan kinds, per the discussion in CL 690616. I also fixed the test to properly trigger the panic by iterating over the returned function, not just calling Seq()/Seq2(). Please let me know if you'd prefer CL 690616 to handle this instead — happy to defer.
The added documentation still says it panics in Seq and Seq2, which doesn't match the current behavior.
Also, the test does not distinguish whether it panics in Seq/Seq2, or later when iterating them. If we want to be accurate, we probably want to have a test that can tell them apart.
Still, we probably want to decide the behavior first.
Thanks.
Related details
- Cyber Hamza
- Ian Lance Taylor
- Russ Cox
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |