[go] reflect: allow Value.Bytes and Value.SetBytes on byte arrays

Joe Tsai (Gerrit)

unread,

Oct 20, 2021, 1:13:05 PM10/20/21

to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Joe Tsai has uploaded this change for review.

reflect: allow Value.Bytes and Value.SetBytes on byte arrays

Modify Value.Bytes and Value.SetBytes to be callable
on non-nil pointer to byte arrays or addressable byte arrays.

DO NOT SUBMIT: Proposal not yet accepted

Fixes #47066

Change-Id: Ic3ba4432353b8da5f33b3188e20034a33b2f6ee8
---
M src/reflect/value.go
1 file changed, 69 insertions(+), 11 deletions(-)

diff --git a/src/reflect/value.go b/src/reflect/value.go
index a8274cc..ff68af8 100644
--- a/src/reflect/value.go
+++ b/src/reflect/value.go
@@ -285,14 +285,32 @@
 }
 
 // Bytes returns v's underlying value.
-// It panics if v's underlying value is not a slice of bytes.
+// It panics if v's underlying value is not a slice of bytes,
+// a non-nil pointer to an array of bytes, or an addressable array of bytes.
 func (v Value) Bytes() []byte {
-	v.mustBe(Slice)
-	if v.typ.Elem().Kind() != Uint8 {
-		panic("reflect.Value.Bytes of non-byte slice")
+	k := v.kind()
+	switch k {
+	case Slice:
+		if v.typ.Elem().Kind() != Uint8 {
+			panic("reflect.Value.Bytes of non-byte slice")
+		}
+		// Slice is always bigger than a word; assume flagIndir.
+		return *(*[]byte)(v.ptr)
+	case Pointer:
+		v = v.Elem() // may panic for nil pointers
+		fallthrough
+	case Array:
+		if v.typ.Elem().Kind() != Uint8 {
+			panic("reflect.Value.Bytes of non-byte array")
+		}
+		if !v.CanAddr() {
+			panic("reflect.Value.Bytes of unaddressable byte array")
+		}
+		p := (*byte)(v.ptr)
+		n := int((*arrayType)(unsafe.Pointer(v.typ)).len)
+		return unsafe.Slice(p, n)
 	}
-	// Slice is always bigger than a word; assume flagIndir.
-	return *(*[]byte)(v.ptr)
+	panic(&ValueError{"reflect.Value.Bytes", v.kind()})
 }
 
 // runes returns v's underlying value.
@@ -1973,14 +1991,38 @@
 }
 
 // SetBytes sets v's underlying value.
-// It panics if v's underlying value is not a slice of bytes.
+// It panics if v's underlying value is not a slice of bytes,
+// a non-nil pointer to an array of bytes, or an addressable array of bytes.
+// When storing into an array, the slice length must match the array length.
 func (v Value) SetBytes(x []byte) {
 	v.mustBeAssignable()
-	v.mustBe(Slice)
-	if v.typ.Elem().Kind() != Uint8 {
-		panic("reflect.Value.SetBytes of non-byte slice")
+	k := v.kind()
+	switch k {
+	case Slice:
+		if v.typ.Elem().Kind() != Uint8 {
+			panic("reflect.Value.SetBytes of non-byte slice")
+		}
+		*(*[]byte)(v.ptr) = x
+		return
+	case Pointer:
+		v = v.Elem() // may panic for nil pointers
+		fallthrough
+	case Array:
+		if v.typ.Elem().Kind() != Uint8 {
+			panic("reflect.Value.SetBytes of non-byte array")
+		}
+		if !v.CanAddr() {
+			panic("reflect.Value.SetBytes of unaddressable byte array")
+		}
+		p := (*byte)(v.ptr)
+		n := int((*arrayType)(unsafe.Pointer(v.typ)).len)
+		if n != len(x) {
+			panic("reflect.Value.SetBytes from slice of length " + itoa.Itoa(len(x)) + " to byte array of length " + itoa.Itoa(n))
+		}
+		copy(unsafe.Slice(p, n), x)
+		return
 	}
-	*(*[]byte)(v.ptr) = x
+	panic(&ValueError{"reflect.Value.SetBytes", v.kind()})
 }
 
 // setRunes sets v's underlying value.

To view, visit change 357331. To unsubscribe, or for help writing mail filters, visit settings.

Joseph Tsai (Gerrit)

unread,

Feb 15, 2022, 8:50:54 PM2/15/22

to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Joseph Tsai uploaded patch set #2 to this change.

View Change

reflect: allow Value.Bytes on addressable byte arrays

Modify Value.Bytes to be callable addressable byte arrays.
While related, the behavior of Value.SetBytes was not modified.



Fixes #47066

Change-Id: Ic3ba4432353b8da5f33b3188e20034a33b2f6ee8
---

M src/reflect/all_test.go
M src/reflect/value.go
2 files changed, 49 insertions(+), 6 deletions(-)

To view, visit change 357331. To unsubscribe, or for help writing mail filters, visit settings.

Joseph Tsai (Gerrit)

unread,

Feb 15, 2022, 8:51:50 PM2/15/22

to goph...@pubsubhelper.golang.org, Keith Randall, golang-co...@googlegroups.com

Attention is currently required from: Keith Randall.

Patch set 2:Run-TryBot +1

View Change

1 comment:

Patchset:
- Patch Set #2:
  RELNOTES=yes

To view, visit change 357331. To unsubscribe, or for help writing mail filters, visit settings.

Keith Randall (Gerrit)

unread,

Mar 1, 2022, 1:22:21 PM3/1/22

to Joseph Tsai, goph...@pubsubhelper.golang.org, Keith Randall, Gopher Robot, golang-co...@googlegroups.com

Attention is currently required from: Joseph Tsai.

Patch set 2:Code-Review +2

View Change

To view, visit change 357331. To unsubscribe, or for help writing mail filters, visit settings.

Joseph Tsai (Gerrit)

unread,

Mar 2, 2022, 12:46:23 PM3/2/22

to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Joseph Tsai.

Joseph Tsai uploaded patch set #3 to this change.

View Change

reflect: allow Value.Bytes on addressable byte arrays

Modify Value.Bytes to be callable addressable byte arrays.
While related, the behavior of Value.SetBytes was not modified.

Fixes #47066

Change-Id: Ic3ba4432353b8da5f33b3188e20034a33b2f6ee8
---
M src/reflect/all_test.go
M src/reflect/value.go

2 files changed, 61 insertions(+), 8 deletions(-)

To view, visit change 357331. To unsubscribe, or for help writing mail filters, visit settings.

Joseph Tsai (Gerrit)

unread,

Mar 2, 2022, 12:47:05 PM3/2/22

to goph...@pubsubhelper.golang.org, Keith Randall, Gopher Robot, golang-co...@googlegroups.com

Patch set 3:Run-TryBot +1

View Change

To view, visit change 357331. To unsubscribe, or for help writing mail filters, visit settings.

Joseph Tsai (Gerrit)

unread,

Mar 2, 2022, 1:06:47 PM3/2/22

to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Gopher Robot, Keith Randall, golang-co...@googlegroups.com

Joseph Tsai submitted this change.

View Change



2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: src/reflect/all_test.go
Insertions: 12, Deletions: 2.

The diff is too large to show. Please review the diff.
```

Approvals:
  Keith Randall: Looks good to me, approved
  Joseph Tsai: Trusted; Run TryBots
  Gopher Robot: TryBots succeeded

reflect: allow Value.Bytes on addressable byte arrays

Modify Value.Bytes to be callable addressable byte arrays.
While related, the behavior of Value.SetBytes was not modified.

Fixes #47066

Change-Id: Ic3ba4432353b8da5f33b3188e20034a33b2f6ee8

Reviewed-on: https://go-review.googlesource.com/c/go/+/357331
Trust: Joseph Tsai <joe...@digital-static.net>
Reviewed-by: Keith Randall <k...@golang.org>
Run-TryBot: Joseph Tsai <joe...@digital-static.net>
TryBot-Result: Gopher Robot <go...@golang.org>


---
M src/reflect/all_test.go
M src/reflect/value.go

2 files changed, 66 insertions(+), 8 deletions(-)

diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
index 866f38e..5364166 100644
--- a/src/reflect/all_test.go
+++ b/src/reflect/all_test.go
@@ -3682,8 +3682,11 @@
 }
 
 func TestBytes(t *testing.T) {
-	type B []byte
-	x := B{1, 2, 3, 4}
+	shouldPanic("on int Value", func() { ValueOf(0).Bytes() })
+	shouldPanic("of non-byte slice", func() { ValueOf([]string{}).Bytes() })
+
+	type S []byte
+	x := S{1, 2, 3, 4}
 	y := ValueOf(x).Bytes()
 	if !bytes.Equal(x, y) {
 		t.Fatalf("ValueOf(%v).Bytes() = %v", x, y)
@@ -3691,6 +3694,28 @@
 	if &x[0] != &y[0] {
 		t.Errorf("ValueOf(%p).Bytes() = %p", &x[0], &y[0])
 	}
+
+	type A [4]byte
+	a := A{1, 2, 3, 4}
+	shouldPanic("unaddressable", func() { ValueOf(a).Bytes() })
+	shouldPanic("on ptr Value", func() { ValueOf(&a).Bytes() })
+	b := ValueOf(&a).Elem().Bytes()
+	if !bytes.Equal(a[:], y) {
+		t.Fatalf("ValueOf(%v).Bytes() = %v", a, b)
+	}
+	if &a[0] != &b[0] {
+		t.Errorf("ValueOf(%p).Bytes() = %p", &a[0], &b[0])
+	}
+
+	// Per issue #24746, it was decided that Bytes can be called on byte slices
+	// that normally cannot be converted from per Go language semantics.
+	type B byte
+	type SB []B
+	type AB [4]B
+	ValueOf([]B{1, 2, 3, 4}).Bytes()  // should not panic
+	ValueOf(new([4]B)).Elem().Bytes() // should not panic
+	ValueOf(SB{1, 2, 3, 4}).Bytes()   // should not panic
+	ValueOf(new(AB)).Elem().Bytes()   // should not panic
 }
 
 func TestSetBytes(t *testing.T) {
diff --git a/src/reflect/value.go b/src/reflect/value.go
index dcc359d..89f0253 100644
--- a/src/reflect/value.go
+++ b/src/reflect/value.go
@@ -286,14 +286,28 @@


 }
 
 // Bytes returns v's underlying value.
-// It panics if v's underlying value is not a slice of bytes.

+// It panics if v's underlying value is not a slice of bytes or
+// an addressable array of bytes.


 func (v Value) Bytes() []byte {
-	v.mustBe(Slice)
-	if v.typ.Elem().Kind() != Uint8 {
-		panic("reflect.Value.Bytes of non-byte slice")

+	switch v.kind() {


+	case Slice:
+		if v.typ.Elem().Kind() != Uint8 {
+			panic("reflect.Value.Bytes of non-byte slice")
+		}
+		// Slice is always bigger than a word; assume flagIndir.
+		return *(*[]byte)(v.ptr)

+	case Array:
+		if v.typ.Elem().Kind() != Uint8 {
+			panic("reflect.Value.Bytes of non-byte array")
+		}
+		if !v.CanAddr() {
+			panic("reflect.Value.Bytes of unaddressable byte array")
+		}
+		p := (*byte)(v.ptr)
+		n := int((*arrayType)(unsafe.Pointer(v.typ)).len)
+		return unsafe.Slice(p, n)
 	}
-	// Slice is always bigger than a word; assume flagIndir.
-	return *(*[]byte)(v.ptr)
+	panic(&ValueError{"reflect.Value.Bytes", v.kind()})
 }
 
 // runes returns v's underlying value.

To view, visit change 357331. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward