internal/impl: use XXX_unrecognized exclusively for unknown fields
The protobuf data model makes no distinction between unknown fields
that are within the extension field ranges or not. Now that we eagerly
unmarshal extensions, there is even less need for storing unknown
fields in the extension map. Instead, use the XXX_unrecognized field
exclusively for this purpose.
Change-Id: I673a7d6259fe9fdbdc295bcfa8252ef4da415343
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/175579
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/internal/impl/legacy_extension.go b/internal/impl/legacy_extension.go
index e187127..9ea5b0a 100644
--- a/internal/impl/legacy_extension.go
+++ b/internal/impl/legacy_extension.go
@@ -183,12 +183,7 @@
if x.Value != nil {
panic("value for extension descriptor still populated")
}
- x.Desc = nil
- if len(x.Raw) == 0 {
- p.x.Clear(t.Number())
- } else {
- p.x.Set(t.Number(), x)
- }
+ p.x.Clear(t.Number())
}
func (p legacyExtensionTypes) ByNumber(n pref.FieldNumber) pref.ExtensionType {
@@ -269,9 +264,6 @@
// The Value may only be populated if Desc is also populated.
Value interface{} // TODO: switch to protoreflect.Value
- // Raw is the raw encoded bytes for the extension field.
- // It is possible for Raw to be populated irrespective of whether the
- // other fields are populated.
Raw []byte // TODO: remove; let this be handled by XXX_unrecognized
}
diff --git a/internal/impl/legacy_test.go b/internal/impl/legacy_test.go
index 927d715..afe28d2 100644
--- a/internal/impl/legacy_test.go
+++ b/internal/impl/legacy_test.go
@@ -43,6 +43,7 @@
rawOf := func(toks ...pack.Token) pref.RawFields {
return pref.RawFields(pack.Message(toks).Marshal())
}
+ raw1 := rawOf(pack.Tag{1, pack.BytesType}, pack.Bytes("1")) // 0a0131
raw1a := rawOf(pack.Tag{1, pack.VarintType}, pack.Svarint(-4321)) // 08c143
raw1b := rawOf(pack.Tag{1, pack.Fixed32Type}, pack.Uint32(0xdeadbeef)) // 0defbeadde
raw1c := rawOf(pack.Tag{1, pack.Fixed64Type}, pack.Float64(math.Pi)) // 09182d4454fb210940
@@ -51,17 +52,6 @@
raw3a := rawOf(pack.Tag{3, pack.StartGroupType}, pack.Tag{3, pack.EndGroupType}) // 1b1c
raw3b := rawOf(pack.Tag{3, pack.BytesType}, pack.Bytes("\xde\xad\xbe\xef")) // 1a04deadbeef
- raw1 := rawOf(pack.Tag{1, pack.BytesType}, pack.Bytes("1")) // 0a0131
- raw3 := rawOf(pack.Tag{3, pack.BytesType}, pack.Bytes("3")) // 1a0133
- raw10 := rawOf(pack.Tag{10, pack.BytesType}, pack.Bytes("10")) // 52023130 - extension
- raw15 := rawOf(pack.Tag{15, pack.BytesType}, pack.Bytes("15")) // 7a023135 - extension
- raw26 := rawOf(pack.Tag{26, pack.BytesType}, pack.Bytes("26")) // d201023236
- raw32 := rawOf(pack.Tag{32, pack.BytesType}, pack.Bytes("32")) // 8202023332
- raw45 := rawOf(pack.Tag{45, pack.BytesType}, pack.Bytes("45")) // ea02023435 - extension
- raw46 := rawOf(pack.Tag{45, pack.BytesType}, pack.Bytes("46")) // ea02023436 - extension
- raw47 := rawOf(pack.Tag{45, pack.BytesType}, pack.Bytes("47")) // ea02023437 - extension
- raw99 := rawOf(pack.Tag{99, pack.BytesType}, pack.Bytes("99")) // 9a06023939
-
joinRaw := func(bs ...pref.RawFields) (out []byte) {
for _, b := range bs {
out = append(out, b...)
@@ -177,93 +167,6 @@
if got, want := m.XXX_unrecognized, joinRaw(raw1); !bytes.Equal(got, want) {
t.Errorf("data mismatch:\ngot: %x\nwant: %x", got, want)
}
-
- fs.Set(45, raw45)
- fs.Set(10, raw10) // extension
- fs.Set(32, raw32)
- fs.Set(1, nil) // deletion
- fs.Set(26, raw26)
- fs.Set(47, raw47) // extension
- fs.Set(46, raw46) // extension
- if got, want := fs.Len(), 6; got != want {
- t.Errorf("Len() = %d, want %d", got, want)
- }
- if got, want := m.XXX_unrecognized, joinRaw(raw32, raw26); !bytes.Equal(got, want) {
- t.Errorf("data mismatch:\ngot: %x\nwant: %x", got, want)
- }
-
- // Verify iteration order.
- i = 0
- want = []struct {
- num pref.FieldNumber
- raw pref.RawFields
- }{
- {32, raw32},
- {26, raw26},
- {10, raw10}, // extension
- {45, raw45}, // extension
- {46, raw46}, // extension
- {47, raw47}, // extension
- }
- fs.Range(func(num pref.FieldNumber, raw pref.RawFields) bool {
- if i < len(want) {
- if num != want[i].num || !bytes.Equal(raw, want[i].raw) {
- t.Errorf("Range(%d) = (%d, %x), want (%d, %x)", i, num, raw, want[i].num, want[i].raw)
- }
- } else {
- t.Errorf("unexpected Range iteration: %d", i)
- }
- i++
- return true
- })
-
- // Perform partial deletion while iterating.
- i = 0
- fs.Range(func(num pref.FieldNumber, raw pref.RawFields) bool {
- if i%2 == 0 {
- fs.Set(num, nil)
- }
- i++
- return true
- })
-
- if got, want := fs.Len(), 3; got != want {
- t.Errorf("Len() = %d, want %d", got, want)
- }
- if got, want := m.XXX_unrecognized, joinRaw(raw26); !bytes.Equal(got, want) {
- t.Errorf("data mismatch:\ngot: %x\nwant: %x", got, want)
- }
-
- fs.Set(15, raw15) // extension
- fs.Set(3, raw3)
- fs.Set(99, raw99)
- if got, want := fs.Len(), 6; got != want {
- t.Errorf("Len() = %d, want %d", got, want)
- }
- if got, want := m.XXX_unrecognized, joinRaw(raw26, raw3, raw99); !bytes.Equal(got, want) {
- t.Errorf("data mismatch:\ngot: %x\nwant: %x", got, want)
- }
-
- // Perform partial iteration.
- i = 0
- want = []struct {
- num pref.FieldNumber
- raw pref.RawFields
- }{
- {26, raw26},
- {3, raw3},
- }
- fs.Range(func(num pref.FieldNumber, raw pref.RawFields) bool {
- if i < len(want) {
- if num != want[i].num || !bytes.Equal(raw, want[i].raw) {
- t.Errorf("Range(%d) = (%d, %x), want (%d, %x)", i, num, raw, want[i].num, want[i].raw)
- }
- } else {
- t.Errorf("unexpected Range iteration: %d", i)
- }
- i++
- return i < 2
- })
}
func mustMakeExtensionType(x *ptype.StandaloneExtension, v interface{}) pref.ExtensionType {
diff --git a/internal/impl/legacy_unknown.go b/internal/impl/legacy_unknown.go
index 97eb3be..919d02d 100644
--- a/internal/impl/legacy_unknown.go
+++ b/internal/impl/legacy_unknown.go
@@ -7,7 +7,6 @@
import (
"container/list"
"reflect"
- "sort"
"github.com/golang/protobuf/v2/internal/encoding/wire"
pref "github.com/golang/protobuf/v2/reflect/protoreflect"
@@ -21,98 +20,13 @@
return nil
}
fieldOffset := offsetOf(fu)
- unkFunc := func(p *messageDataType) pref.UnknownFields {
+ return func(p *messageDataType) pref.UnknownFields {
if p.p.IsNil() {
return emptyUnknownFields{}
}
rv := p.p.Apply(fieldOffset).AsValueOf(bytesType)
return (*legacyUnknownBytes)(rv.Interface().(*[]byte))
}
- extFunc := makeLegacyExtensionMapFunc(t)
- if extFunc != nil {
- return func(p *messageDataType) pref.UnknownFields {
- if p.p.IsNil() {
- return emptyUnknownFields{}
- }
- return &legacyUnknownBytesAndExtensionMap{
- unkFunc(p), extFunc(p), p.mi.PBType.ExtensionRanges(),
- }
- }
- }
- return unkFunc
-}
-
-// legacyUnknownBytesAndExtensionMap is a wrapper around both XXX_unrecognized
-// and also the extension field map.
-type legacyUnknownBytesAndExtensionMap struct {
- u pref.UnknownFields
- x *legacyExtensionMap
- r pref.FieldRanges
-}
-
-func (fs *legacyUnknownBytesAndExtensionMap) Len() int {
- n := fs.u.Len()
- fs.x.Range(func(_ pref.FieldNumber, x ExtensionFieldV1) bool {
- if len(x.Raw) > 0 {
- n++
- }
- return true
- })
- return n
-}
-
-func (fs *legacyUnknownBytesAndExtensionMap) Get(num pref.FieldNumber) (raw pref.RawFields) {
- if fs.r.Has(num) {
- return fs.x.Get(num).Raw
- }
- return fs.u.Get(num)
-}
-
-func (fs *legacyUnknownBytesAndExtensionMap) Set(num pref.FieldNumber, raw pref.RawFields) {
- if fs.r.Has(num) {
- x := fs.x.Get(num)
- x.Raw = raw
- fs.x.Set(num, x)
- return
- }
- fs.u.Set(num, raw)
-}
-
-func (fs *legacyUnknownBytesAndExtensionMap) Range(f func(pref.FieldNumber, pref.RawFields) bool) {
- // Range over unknown fields not in the extension range.
- // Create a closure around f to capture whether iteration terminated early.
- var stop bool
- fs.u.Range(func(n pref.FieldNumber, b pref.RawFields) bool {
- stop = stop || !f(n, b)
- return !stop
- })
- if stop {
- return
- }
-
- // Range over unknown fields in the extension range in ascending order
- // to ensure protoreflect.UnknownFields.Range remains deterministic.
- type entry struct {
- num pref.FieldNumber
- raw pref.RawFields
- }
- var xs []entry
- fs.x.Range(func(n pref.FieldNumber, x ExtensionFieldV1) bool {
- if len(x.Raw) > 0 {
- xs = append(xs, entry{n, x.Raw})
- }
- return true
- })
- sort.Slice(xs, func(i, j int) bool { return xs[i].num < xs[j].num })
- for _, x := range xs {
- if !f(x.num, x.raw) {
- return
- }
- }
-}
-
-func (fs *legacyUnknownBytesAndExtensionMap) IsSupported() bool {
- return true
}
// legacyUnknownBytes is a wrapper around XXX_unrecognized that implements
