From 7efb9f03ad7824216a81a05ec262bd0b9fa7e067 Mon Sep 17 00:00:00 2001 From: liuqiang Date: Thu, 2 Dec 2021 20:07:20 +0800 Subject: [PATCH] fix: 'string' option in struct tag as encoding/json --- decoder/assembler_amd64_go115.go | 13 ++++++ decoder/assembler_amd64_go116.go | 13 ++++++ decoder/assembler_amd64_go117.go | 13 ++++++ decoder/compiler.go | 39 +++++++++-------- issue_test/issue144_test.go | 72 ++++++++++++++++++++++++++++++++ 5 files changed, 132 insertions(+), 18 deletions(-) create mode 100644 issue_test/issue144_test.go diff --git a/decoder/assembler_amd64_go115.go b/decoder/assembler_amd64_go115.go index 318e22448..21cf873b5 100644 --- a/decoder/assembler_amd64_go115.go +++ b/decoder/assembler_amd64_go115.go @@ -241,6 +241,7 @@ _OP_deref : (*_Assembler)._asm_OP_deref, _OP_index : (*_Assembler)._asm_OP_index, _OP_is_null : (*_Assembler)._asm_OP_is_null, + _OP_is_null_quote : (*_Assembler)._asm_OP_is_null_quote, _OP_map_init : (*_Assembler)._asm_OP_map_init, _OP_map_key_i8 : (*_Assembler)._asm_OP_map_key_i8, _OP_map_key_i16 : (*_Assembler)._asm_OP_map_key_i16, @@ -1179,6 +1180,18 @@ self.Link("_not_null_{n}") // _not_null_{n}: } + func (self *_Assembler) _asm_OP_is_null_quote(p *_Instr) { + self.Emit("LEAQ" , jit.Ptr(_IC, 5), _AX) // LEAQ 4(IC), AX + self.Emit("CMPQ" , _AX, _IL) // CMPQ AX, IL + self.Sjmp("JA" , "_not_null_quote_{n}") // JA _not_null_quote_{n} + self.Emit("CMPL" , jit.Sib(_IP, _IC, 1, 0), jit.Imm(_IM_null)) // CMPL (IP)(IC), $"null" + self.Sjmp("JNE" , "_not_null_quote_{n}") // JNE _not_null_quote_{n} + self.Emit("CMPB" , jit.Sib(_IP, _IC, 1, 4), jit.Imm('"')) // CMPB 4(IP)(IC), $'"' + self.Emit("CMOVQEQ", _AX, _IC) // CMOVQEQ AX, IC + self.Xjmp("JE" , p.vi()) // JE {p.vi()} + self.Link("_not_null_quote_{n}") // _not_null_quote_{n}: + } + func (self *_Assembler) _asm_OP_map_init(_ *_Instr) { self.Emit("MOVQ" , jit.Ptr(_VP, 0), _AX) // MOVQ (VP), AX self.Emit("TESTQ", _AX, _AX) // TESTQ AX, AX diff --git a/decoder/assembler_amd64_go116.go b/decoder/assembler_amd64_go116.go index c671255b2..19d34795b 100644 --- a/decoder/assembler_amd64_go116.go +++ b/decoder/assembler_amd64_go116.go @@ -244,6 +244,7 @@ var _OpFuncTab = [256]func(*_Assembler, *_Instr) { _OP_deref : (*_Assembler)._asm_OP_deref, _OP_index : (*_Assembler)._asm_OP_index, _OP_is_null : (*_Assembler)._asm_OP_is_null, + _OP_is_null_quote : (*_Assembler)._asm_OP_is_null_quote, _OP_map_init : (*_Assembler)._asm_OP_map_init, _OP_map_key_i8 : (*_Assembler)._asm_OP_map_key_i8, _OP_map_key_i16 : (*_Assembler)._asm_OP_map_key_i16, @@ -1186,6 +1187,18 @@ func (self *_Assembler) _asm_OP_is_null(p *_Instr) { self.Link("_not_null_{n}") // _not_null_{n}: } +func (self *_Assembler) _asm_OP_is_null_quote(p *_Instr) { + self.Emit("LEAQ" , jit.Ptr(_IC, 5), _AX) // LEAQ 4(IC), AX + self.Emit("CMPQ" , _AX, _IL) // CMPQ AX, IL + self.Sjmp("JA" , "_not_null_quote_{n}") // JA _not_null_quote_{n} + self.Emit("CMPL" , jit.Sib(_IP, _IC, 1, 0), jit.Imm(_IM_null)) // CMPL (IP)(IC), $"null" + self.Sjmp("JNE" , "_not_null_quote_{n}") // JNE _not_null_quote_{n} + self.Emit("CMPB" , jit.Sib(_IP, _IC, 1, 4), jit.Imm('"')) // CMPB 4(IP)(IC), $'"' + self.Emit("CMOVQEQ", _AX, _IC) // CMOVQEQ AX, IC + self.Xjmp("JE" , p.vi()) // JE {p.vi()} + self.Link("_not_null_quote_{n}") // _not_null_quote_{n}: +} + func (self *_Assembler) _asm_OP_map_init(_ *_Instr) { self.Emit("MOVQ" , jit.Ptr(_VP, 0), _AX) // MOVQ (VP), AX self.Emit("TESTQ", _AX, _AX) // TESTQ AX, AX diff --git a/decoder/assembler_amd64_go117.go b/decoder/assembler_amd64_go117.go index 3d9b03c8c..d7b3de9ba 100644 --- a/decoder/assembler_amd64_go117.go +++ b/decoder/assembler_amd64_go117.go @@ -244,6 +244,7 @@ var _OpFuncTab = [256]func(*_Assembler, *_Instr) { _OP_deref : (*_Assembler)._asm_OP_deref, _OP_index : (*_Assembler)._asm_OP_index, _OP_is_null : (*_Assembler)._asm_OP_is_null, + _OP_is_null_quote : (*_Assembler)._asm_OP_is_null_quote, _OP_map_init : (*_Assembler)._asm_OP_map_init, _OP_map_key_i8 : (*_Assembler)._asm_OP_map_key_i8, _OP_map_key_i16 : (*_Assembler)._asm_OP_map_key_i16, @@ -1183,6 +1184,18 @@ func (self *_Assembler) _asm_OP_is_null(p *_Instr) { self.Link("_not_null_{n}") // _not_null_{n}: } +func (self *_Assembler) _asm_OP_is_null_quote(p *_Instr) { + self.Emit("LEAQ" , jit.Ptr(_IC, 5), _AX) // LEAQ 4(IC), AX + self.Emit("CMPQ" , _AX, _IL) // CMPQ AX, IL + self.Sjmp("JA" , "_not_null_quote_{n}") // JA _not_null_quote_{n} + self.Emit("CMPL" , jit.Sib(_IP, _IC, 1, 0), jit.Imm(_IM_null)) // CMPL (IP)(IC), $"null" + self.Sjmp("JNE" , "_not_null_quote_{n}") // JNE _not_null_quote_{n} + self.Emit("CMPB" , jit.Sib(_IP, _IC, 1, 4), jit.Imm('"')) // CMPB 4(IP)(IC), $'"' + self.Emit("CMOVQEQ", _AX, _IC) // CMOVQEQ AX, IC + self.Xjmp("JE" , p.vi()) // JE {p.vi()} + self.Link("_not_null_quote_{n}") // _not_null_quote_{n}: +} + func (self *_Assembler) _asm_OP_map_init(_ *_Instr) { self.Emit("MOVQ" , jit.Ptr(_VP, 0), _AX) // MOVQ (VP), AX self.Emit("TESTQ", _AX, _AX) // TESTQ AX, AX diff --git a/decoder/compiler.go b/decoder/compiler.go index aa94f0bc2..97d5708ad 100644 --- a/decoder/compiler.go +++ b/decoder/compiler.go @@ -57,6 +57,7 @@ const ( _OP_deref _OP_index _OP_is_null + _OP_is_null_quote _OP_map_init _OP_map_key_i8 _OP_map_key_i16 @@ -129,6 +130,7 @@ var _OpNames = [256]string { _OP_deref : "deref", _OP_index : "index", _OP_is_null : "is_null", + _OP_is_null_quote : "is_null_quote", _OP_map_init : "map_init", _OP_map_key_i8 : "map_key_i8", _OP_map_key_i16 : "map_key_i16", @@ -304,11 +306,12 @@ func (self _Instr) vlen() int { func (self _Instr) isBranch() bool { switch self.op() { - case _OP_goto : fallthrough - case _OP_switch : fallthrough - case _OP_is_null : fallthrough - case _OP_check_char : return true - default : return false + case _OP_goto : fallthrough + case _OP_switch : fallthrough + case _OP_is_null : fallthrough + case _OP_is_null_quote : fallthrough + case _OP_check_char : return true + default : return false } } @@ -337,6 +340,7 @@ func (self _Instr) disassemble() string { case _OP_unmarshal_text_p : fallthrough case _OP_recurse : return fmt.Sprintf("%-18s%s", self.op(), self.vt()) case _OP_goto : fallthrough + case _OP_is_null_quote : fallthrough case _OP_is_null : return fmt.Sprintf("%-18sL_%d", self.op(), self.vi()) case _OP_index : fallthrough case _OP_array_clear : fallthrough @@ -932,18 +936,16 @@ func (self *_Compiler) compileStructFieldStr(p *_Program, sp int, vt reflect.Typ p.add(_OP_is_null) p.chr(_OP_match_char, '"') - /* dereference the pointer */ + /* also check for inner "null" */ + n1 = p.pc() + p.add(_OP_is_null_quote) + + /* dereference the pointer only when it is not null */ if vk == reflect.Ptr { vt = vt.Elem() p.rtt(_OP_deref, vt) } - /* also check for inner "null" for a type that is not `string` */ - if vt.Kind() != reflect.String { - n1 = p.pc() - p.add(_OP_is_null) - } - /* string opcode selector */ _OP_string := func() _Op { if ft == jsonNumberType { @@ -973,16 +975,16 @@ func (self *_Compiler) compileStructFieldStr(p *_Program, sp int, vt reflect.Typ default : panic("not reachable") } - /* pin the `is_null` jump location */ - if n1 != -1 { - p.pin(n1) - } - /* the closing quote is not needed when parsing a pure string */ if vt == jsonNumberType || vt.Kind() != reflect.String { p.chr(_OP_match_char, '"') } + /* pin the `is_null_quote` jump location */ + if n1 != -1 && vk != reflect.Ptr { + p.pin(n1) + } + /* "null" but not a pointer, act as if the field is not present */ if vk != reflect.Ptr { p.pin(n0) @@ -992,7 +994,8 @@ func (self *_Compiler) compileStructFieldStr(p *_Program, sp int, vt reflect.Typ /* the "null" case of the pointer */ pc := p.pc() p.add(_OP_goto) - p.pin(n0) + p.pin(n0) // `is_null` jump location + p.pin(n1) // `is_null_quote` jump location p.add(_OP_nil_1) p.pin(pc) } diff --git a/issue_test/issue144_test.go b/issue_test/issue144_test.go new file mode 100644 index 000000000..398ba8e33 --- /dev/null +++ b/issue_test/issue144_test.go @@ -0,0 +1,72 @@ +/* +* Copyright 2021 ByteDance Inc. +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +package issue_test + +import ( + `encoding/json` + `testing` + + `github.com/stretchr/testify/require` + `github.com/davecgh/go-spew/spew` + `github.com/bytedance/sonic` +) + +type Issue144_StringOption struct { + S1 *string `json:"s1,string"` + S2 *string `json:"s2,string"` + S3 string `json:"s3,string"` + J1 json.Number `json:"j1,string"` + J2 *json.Number `json:"j2,string"` + J3 *json.Number `json:"j3,string"` + I1 int `json:"i1,string"` + I2 *int `json:"i2,string"` + I3 *int `json:"i3,string"` +} + +func TestIssue144_StringOption(t *testing.T) { + data := []byte(`{ + "s1":"\"null\"", + "s2":"null", + "s3":"null", + "j1":"null", + "j2":"null", + "j3":"123.456", + "i1":"null", + "i2":"null", + "i3":"-123" + }`) + + var v1, v2 Issue144_StringOption + e1 := json.Unmarshal(data, &v1) + e2 := sonic.Unmarshal(data, &v2) + require.NoError(t, e1) + require.NoError(t, e2) + require.Equal(t, v1, v2) + spew.Dump(v1) + + i, j, s := int(1), json.Number("1"), "null" + v1.I2, v2.I2 = &i, &i + v1.J2, v2.J2 = &j, &j + v1.S2, v2.S2 = &s, &s + + e1 = json.Unmarshal(data, &v1) + e2 = sonic.Unmarshal(data, &v2) + require.NoError(t, e1) + require.NoError(t, e2) + require.Equal(t, v1, v2) + spew.Dump(v1) +} \ No newline at end of file