Browse Source

codecgen: fix bugs with codecgen for arrays

- correct r.ReadArrayCannotExpand to z.DecArrayCannotExpand
- do not derefence a pointer to an array during codecgen, as that just updates a copy of what you want.
- reslice is done only if a slice (not array or chan)

Also, for reflection-based decoding:
- only do a reflect.Value.SetLen if it is a slice; else we get a panic.

Also:
- add a TODO to include a test for arrays, especially for reflection and codecgen.

Fixes #88
Ugorji Nwoke 10 years ago
parent
commit
e2d5eb8190
5 changed files with 37 additions and 22 deletions
  1. 4 2
      codec/decode.go
  2. 11 8
      codec/gen-dec-array.go.tmpl
  3. 11 8
      codec/gen.generated.go
  4. 6 4
      codec/prebuild.sh
  5. 5 0
      codec/values_test.go

+ 4 - 2
codec/decode.go

@@ -748,8 +748,10 @@ func (f decFnInfo) kSlice(rv reflect.Value) {
 					rvlen = containerLenS
 					rvlen = containerLenS
 				}
 				}
 			} else if containerLenS != rvlen {
 			} else if containerLenS != rvlen {
-				rv.SetLen(containerLenS)
-				rvlen = containerLenS
+				if f.seq == seqTypeSlice {
+					rv.SetLen(containerLenS)
+					rvlen = containerLenS
+				}
 			}
 			}
 			j := 0
 			j := 0
 			for ; j < numToRead; j++ {
 			for ; j < numToRead; j++ {

+ 11 - 8
codec/gen-dec-array.go.tmpl

@@ -1,7 +1,9 @@
-{{var "v"}} := *{{ .Varname }}
+{{var "v"}} := {{ if not isArray}}*{{ end }}{{ .Varname }}
 {{var "h"}}, {{var "l"}} := z.DecSliceHelperStart()
 {{var "h"}}, {{var "l"}} := z.DecSliceHelperStart()
 
 
-var {{var "c"}} bool 
+var {{var "c"}} bool
+_ = {{var "c"}}
+
 {{ if not isArray }}if {{var "v"}} == nil {
 {{ if not isArray }}if {{var "v"}} == nil {
 	if {{var "l"}} <= 0 {
 	if {{var "l"}} <= 0 {
         {{var "v"}} = make({{ .CTyp }}, 0)
         {{var "v"}} = make({{ .CTyp }}, 0)
@@ -25,7 +27,7 @@ if {{var "l"}} == 0 { {{ if isSlice }}
 	{{ else }} 
 	{{ else }} 
 	{{var "n"}} := {{var "l"}} 
 	{{var "n"}} := {{var "l"}} 
 	if {{var "l"}} > cap({{var "v"}}) {
 	if {{var "l"}} > cap({{var "v"}}) {
-		{{ if isArray }}r.ReadArrayCannotExpand(len({{var "v"}}), {{var "l"}})
+		{{ if isArray }}z.DecArrayCannotExpand(len({{var "v"}}), {{var "l"}})
 		{{var "n"}} = len({{var "v"}})
 		{{var "n"}} = len({{var "v"}})
 		{{ else }}{{ if .Immutable }}
 		{{ else }}{{ if .Immutable }}
 		{{var "v2"}} := {{var "v"}}
 		{{var "v2"}} := {{var "v"}}
@@ -37,8 +39,8 @@ if {{var "l"}} == 0 { {{ if isSlice }}
 		{{ end }}{{var "c"}} = true 
 		{{ end }}{{var "c"}} = true 
 		{{ end }}
 		{{ end }}
 	} else if {{var "l"}} != len({{var "v"}}) {
 	} else if {{var "l"}} != len({{var "v"}}) {
-		{{var "v"}} = {{var "v"}}[:{{var "l"}}]
-		{{var "c"}} = true 
+		{{ if isSlice }}{{var "v"}} = {{var "v"}}[:{{var "l"}}]
+		{{var "c"}} = true {{ end }}
 	}
 	}
 	{{var "j"}} := 0
 	{{var "j"}} := 0
 	for ; {{var "j"}} < {{var "n"}} ; {{var "j"}}++ {
 	for ; {{var "j"}} < {{var "n"}} ; {{var "j"}}++ {
@@ -51,7 +53,7 @@ if {{var "l"}} == 0 { {{ if isSlice }}
 } else {
 } else {
 	for {{var "j"}} := 0; !r.CheckBreak(); {{var "j"}}++ {
 	for {{var "j"}} := 0; !r.CheckBreak(); {{var "j"}}++ {
 		if {{var "j"}} >= len({{var "v"}}) {
 		if {{var "j"}} >= len({{var "v"}}) {
-			{{ if isArray }}r.ReadArrayCannotExpand(len({{var "v"}}), {{var "j"}}+1)
+			{{ if isArray }}z.DecArrayCannotExpand(len({{var "v"}}), {{var "j"}}+1)
 			{{ else if isSlice}}{{var "v"}} = append({{var "v"}}, {{zero}})// var {{var "z"}} {{ .Typ }}
 			{{ else if isSlice}}{{var "v"}} = append({{var "v"}}, {{zero}})// var {{var "z"}} {{ .Typ }}
 			{{var "c"}} = true {{ end }}
 			{{var "c"}} = true {{ end }}
 		}
 		}
@@ -72,6 +74,7 @@ if {{var "l"}} == 0 { {{ if isSlice }}
 	}
 	}
 	{{var "h"}}.End()
 	{{var "h"}}.End()
 }
 }
-if {{var "c"}} { 
+{{ if not isArray }}if {{var "c"}} { 
 	*{{ .Varname }} = {{var "v"}}
 	*{{ .Varname }} = {{var "v"}}
-}
+}{{ end }}
+

+ 11 - 8
codec/gen.generated.go

@@ -55,10 +55,12 @@ r.ReadMapEnd()
 `
 `
 
 
 const genDecListTmpl = `
 const genDecListTmpl = `
-{{var "v"}} := *{{ .Varname }}
+{{var "v"}} := {{ if not isArray}}*{{ end }}{{ .Varname }}
 {{var "h"}}, {{var "l"}} := z.DecSliceHelperStart()
 {{var "h"}}, {{var "l"}} := z.DecSliceHelperStart()
 
 
-var {{var "c"}} bool 
+var {{var "c"}} bool
+_ = {{var "c"}}
+
 {{ if not isArray }}if {{var "v"}} == nil {
 {{ if not isArray }}if {{var "v"}} == nil {
 	if {{var "l"}} <= 0 {
 	if {{var "l"}} <= 0 {
         {{var "v"}} = make({{ .CTyp }}, 0)
         {{var "v"}} = make({{ .CTyp }}, 0)
@@ -82,7 +84,7 @@ if {{var "l"}} == 0 { {{ if isSlice }}
 	{{ else }} 
 	{{ else }} 
 	{{var "n"}} := {{var "l"}} 
 	{{var "n"}} := {{var "l"}} 
 	if {{var "l"}} > cap({{var "v"}}) {
 	if {{var "l"}} > cap({{var "v"}}) {
-		{{ if isArray }}r.ReadArrayCannotExpand(len({{var "v"}}), {{var "l"}})
+		{{ if isArray }}z.DecArrayCannotExpand(len({{var "v"}}), {{var "l"}})
 		{{var "n"}} = len({{var "v"}})
 		{{var "n"}} = len({{var "v"}})
 		{{ else }}{{ if .Immutable }}
 		{{ else }}{{ if .Immutable }}
 		{{var "v2"}} := {{var "v"}}
 		{{var "v2"}} := {{var "v"}}
@@ -94,8 +96,8 @@ if {{var "l"}} == 0 { {{ if isSlice }}
 		{{ end }}{{var "c"}} = true 
 		{{ end }}{{var "c"}} = true 
 		{{ end }}
 		{{ end }}
 	} else if {{var "l"}} != len({{var "v"}}) {
 	} else if {{var "l"}} != len({{var "v"}}) {
-		{{var "v"}} = {{var "v"}}[:{{var "l"}}]
-		{{var "c"}} = true 
+		{{ if isSlice }}{{var "v"}} = {{var "v"}}[:{{var "l"}}]
+		{{var "c"}} = true {{ end }}
 	}
 	}
 	{{var "j"}} := 0
 	{{var "j"}} := 0
 	for ; {{var "j"}} < {{var "n"}} ; {{var "j"}}++ {
 	for ; {{var "j"}} < {{var "n"}} ; {{var "j"}}++ {
@@ -108,7 +110,7 @@ if {{var "l"}} == 0 { {{ if isSlice }}
 } else {
 } else {
 	for {{var "j"}} := 0; !r.CheckBreak(); {{var "j"}}++ {
 	for {{var "j"}} := 0; !r.CheckBreak(); {{var "j"}}++ {
 		if {{var "j"}} >= len({{var "v"}}) {
 		if {{var "j"}} >= len({{var "v"}}) {
-			{{ if isArray }}r.ReadArrayCannotExpand(len({{var "v"}}), {{var "j"}}+1)
+			{{ if isArray }}z.DecArrayCannotExpand(len({{var "v"}}), {{var "j"}}+1)
 			{{ else if isSlice}}{{var "v"}} = append({{var "v"}}, {{zero}})// var {{var "z"}} {{ .Typ }}
 			{{ else if isSlice}}{{var "v"}} = append({{var "v"}}, {{zero}})// var {{var "z"}} {{ .Typ }}
 			{{var "c"}} = true {{ end }}
 			{{var "c"}} = true {{ end }}
 		}
 		}
@@ -129,8 +131,9 @@ if {{var "l"}} == 0 { {{ if isSlice }}
 	}
 	}
 	{{var "h"}}.End()
 	{{var "h"}}.End()
 }
 }
-if {{var "c"}} { 
+{{ if not isArray }}if {{var "c"}} { 
 	*{{ .Varname }} = {{var "v"}}
 	*{{ .Varname }} = {{var "v"}}
-}
+}{{ end }}
+
 `
 `
 
 

+ 6 - 4
codec/prebuild.sh

@@ -135,16 +135,18 @@ EOF
 
 
 _codegenerators() {
 _codegenerators() {
     if [[ $zforce == "1" || 
     if [[ $zforce == "1" || 
-                "1" == $( _needgen "values_msgp${zsfx}" ) 
-                || "1" == $( _needgen "values_codecgen${zsfx}" ) ]] 
+                "1" == $( _needgen "values_codecgen${zsfx}" ) ||
+                "1" == $( _needgen "values_msgp${zsfx}" ) ||
+                "1" == $( _needgen "values_ffjson${zsfx}" ) ||
+                1 == 0 ]] 
     then
     then
         true && \
         true && \
-            echo "msgp ... " && \
-            msgp -tests=false -pkg=codec -o=values_msgp${zsfx} -file=$zfin && \
             echo "codecgen - !unsafe ... " && \
             echo "codecgen - !unsafe ... " && \
             codecgen -rt codecgen -t 'x,codecgen,!unsafe' -o values_codecgen${zsfx} $zfin && \
             codecgen -rt codecgen -t 'x,codecgen,!unsafe' -o values_codecgen${zsfx} $zfin && \
             echo "codecgen - unsafe ... " && \
             echo "codecgen - unsafe ... " && \
             codecgen -u -rt codecgen -t 'x,codecgen,unsafe' -o values_codecgen_unsafe${zsfx} $zfin && \
             codecgen -u -rt codecgen -t 'x,codecgen,unsafe' -o values_codecgen_unsafe${zsfx} $zfin && \
+            echo "msgp ... " && \
+            msgp -tests=false -pkg=codec -o=values_msgp${zsfx} -file=$zfin && \
             echo "ffjson ... " && \
             echo "ffjson ... " && \
             ffjson -w values_ffjson${zsfx} $zfin && \
             ffjson -w values_ffjson${zsfx} $zfin && \
             # remove (M|Unm)arshalJSON implementations, so they don't conflict with encoding/json bench \
             # remove (M|Unm)arshalJSON implementations, so they don't conflict with encoding/json bench \

+ 5 - 0
codec/values_test.go

@@ -57,6 +57,11 @@ type TestStruc struct {
 
 
 	Iptrslice []*int64
 	Iptrslice []*int64
 
 
+	// TODO: test these separately, specifically for reflection and codecgen.
+	// Unfortunately, ffjson doesn't support these. Its compilation even fails.
+	// Ui64array      [4]uint64
+	// Ui64slicearray [][4]uint64
+
 	AnonInTestStruc
 	AnonInTestStruc
 
 
 	//M map[interface{}]interface{}  `json:"-",bson:"-"`
 	//M map[interface{}]interface{}  `json:"-",bson:"-"`