codec: resolve TODO's

codec/decode.go

@@ -518,7 +518,7 @@ func (d *Decoder) kInterfaceNaked(f *codecFnInfo) (rvn reflect.Value) {
 			}
 		}
 	case valueTypeNil:
-		// rvn = reflect.Zero(f.ti.rt) // TODO
+		// rvn = reflect.Zero(f.ti.rt)
 		// no-op
 	case valueTypeInt:
 		rvn = n.ri()
@@ -1193,7 +1193,7 @@ func (d *Decoder) kMap(f *codecFnInfo, rv reflect.Value) {
 					if vtypeElem == nil {
 						vtypeElem = vtype.Elem()
 					}
-					rvv = reflect.New(vtypeElem) // TODO: use rvzeroaddr?
+					rvv = reflect.New(vtypeElem)
 				}
 			} else if rvv.IsValid() && vtypeKind == reflect.Interface && !rvIsNil(rvv) {
 				rvvn = rvZeroAddrK(vtype, vtypeKind)
@@ -1602,7 +1602,7 @@ func (d *Decoder) MustDecode(v interface{}) {
 // This provides insight to the code location that triggered the error.
 func (d *Decoder) mustDecode(v interface{}) {
 	// xdebug2f(".... mustDecode: v: %#v", v)
-	// TODO: Top-level: ensure that v is a pointer and not nil.
+	// Top-level: v is a pointer and not nil.
 
 	// if d.bi == nil {
 	// 	// if d.d.TryDecodeAsNil() {
@@ -1777,6 +1777,14 @@ func setZero(iv interface{}) {
 }
 
 func setZeroRV(v reflect.Value) {
+	// It not decodeable, we do not touch it.
+	// We considered empty'ing it if not decodeable e.g.
+	//    - if chan, drain it
+	//    - if map, clear it
+	//    - if slice or array, zero all elements up to len
+	//
+	// However, we decided instead that we either will set the
+	// whole value to the zero value, or leave AS IS.
 	if isDecodeable(v) {
 		if v.Kind() == reflect.Ptr {
 			v = v.Elem()
@@ -1784,7 +1792,7 @@ func setZeroRV(v reflect.Value) {
 		if v.CanSet() {
 			v.Set(reflect.Zero(v.Type()))
 		}
-	} // TODO: else drain if chan, clear if map, set all to nil if slice???
+	}
 }
 
 func (d *Decoder) decode(iv interface{}) {
@@ -1854,8 +1862,7 @@ func (d *Decoder) decode(iv interface{}) {
 		} else if !fastpathDecodeTypeSwitch(iv, d) {
 			v := rv4i(iv)
 			d.ensureDecodeable(v)
-			d.decodeValue(v, nil) // TODO: find a way to say: no fast path??? Not necessary???
-			// d.decodeValueFallback(v)
+			d.decodeValue(v, nil)
 		}
 	}
 }
@@ -2144,6 +2151,8 @@ func (d *Decoder) sideDecode(v interface{}, bs []byte) {
 
 // decSliceHelper assists when decoding into a slice, from a map or an array in the stream.
 // A slice can be set from a map or array in stream. This supports the MapBySlice interface.
+//
+// Note: if IsNil, do not call ElemContainerState.
 type decSliceHelper struct {
 	d     *Decoder
 	ct    valueType
@@ -2179,13 +2188,24 @@ func (x decSliceHelper) End() {
 
 func (x decSliceHelper) ElemContainerState(index int) {
 	// Note: if isnil, clen=0, so we never call into ElemContainerState
-	if x.IsNil { //TODO: take out this check
-	} else if x.Array {
+
+	// if x.IsNil {
+	// } else if x.Array {
+	// 	x.d.arrayElem()
+	// } else if index%2 == 0 {
+	// 	x.d.mapElemKey()
+	// } else {
+	// 	x.d.mapElemValue()
+	// }
+
+	if x.Array {
 		x.d.arrayElem()
-	} else if index%2 == 0 {
-		x.d.mapElemKey()
 	} else {
-		x.d.mapElemValue()
+		if index%2 == 0 {
+			x.d.mapElemKey()
+		} else {
+			x.d.mapElemValue()
+		}
 	}
 }
 

codec/encode.go

@@ -304,7 +304,6 @@ func (e *Encoder) kErr(f *codecFnInfo, rv reflect.Value) {
 }
 
 func chanToSlice(rv reflect.Value, rtelem reflect.Type, timeout time.Duration) (rvcs reflect.Value) {
-	// TODO: ensure this doesn't mess up anywhere that rv of kind chan is expected
 	rvcs = reflect.Zero(reflect.SliceOf(rtelem))
 	if timeout < 0 { // consume until close
 		for {

codec/gen.go

@@ -1255,7 +1255,7 @@ func (x *genRunner) encListFallback(varname string, t reflect.Type) {
 
 func (x *genRunner) encMapFallback(varname string, t reflect.Type) {
 	x.linef("if %s == nil { r.EncodeNil(); return }", varname)
-	// TODO: expand this to handle canonical.
+	// NOTE: Canonical Option is not honored
 	i := x.varsfx()
 	x.line("z.EncWriteMapStart(len(" + varname + "))")
 	// x.linef("var %sfirst%s = true", genTempVarPfx, i)

codec/helper.go

@@ -119,6 +119,15 @@ package codec
 // Escape Analysis
 //    - Prefer to return non-pointers if the value is used right away.
 //      Newly allocated values returned as pointers will be heap-allocated as they escape.
+//
+// Prefer functions and methods that
+//    - take no parameters and
+//    - return no results and
+//    - do not allocate.
+// These are optimized by the runtime.
+// For example, in json, we have dedicated functions for ReadMapElemKey, etc
+// which do not delegate to readDelim, as readDelim takes a parameter.
+// The difference in runtime was as much as 5%.
 
 import (
 	"bytes"
@@ -1303,8 +1312,10 @@ func (o *extHandle) SetExt(rt reflect.Type, tag uint64, ext Ext) (err error) {
 	rtid := rt2id(rt)
 	switch rtid {
 	case timeTypId, rawTypId, rawExtTypId:
-		// all natively supported type, so cannot have an extension
-		return // TODO: should we silently ignore, or return an error???
+		// all natively supported type, so cannot have an extension.
+		// However, we do not return an error for these, as we do not document that.
+		// Instead, we silently treat as a no-op, and return.
+		return
 	}
 	// if o == nil {
 	// 	return errors.New("codec.Handle.SetExt: extHandle not initialized")
@@ -3256,7 +3267,7 @@ var _ = isNaN32
 // 	//tzhr, tzmin := tz/60, tz%60 //faster if u convert to int first
 // 	var tzhr, tzmin int16
 // 	if tzint < 0 {
-// 		tzname[3] = '-' // (TODO: verify. this works here)
+// 		tzname[3] = '-'
 // 		tzhr, tzmin = -tzint/60, (-tzint)%60
 // 	} else {
 // 		tzhr, tzmin = tzint/60, tzint%60

codec/helper_unsafe.go

@@ -129,11 +129,8 @@ func rv4i(i interface{}) (rv reflect.Value) {
 }
 
 func rv2i(rv reflect.Value) interface{} {
-	// TODO: consider a more generally-known optimization for reflect.Value ==> Interface
-	//
-	// Currently, we use this fragile method that taps into implememtation details from
+	// We tap into implememtation details from
 	// the source go stdlib reflect/value.go, and trims the implementation.
-
 	urv := (*unsafeReflectValue)(unsafe.Pointer(&rv))
 	return *(*interface{})(unsafe.Pointer(&unsafeIntf{typ: urv.typ, word: rv2ptr(urv)}))
 }
@@ -534,7 +531,14 @@ func rvGetArrayBytesRO(rv reflect.Value, scratch []byte) (bs []byte) {
 
 func rvGetArray4Slice(rv reflect.Value) (v reflect.Value) {
 	urv := (*unsafeReflectValue)(unsafe.Pointer(&rv))
-	t := reflectArrayOf(rvGetSliceLen(rv), rv.Type().Elem()) // TODO: Len or Cap???
+	// It is possible that this slice is based off an array with a larger
+	// len that we want (where array len == slice cap).
+	// However, it is ok to create an array type that is a subset of the full
+	// e.g. full slice is based off a *[16]byte, but we can create a *[4]byte
+	// off of it. That is ok.
+	//
+	// Consequently, we use rvGetSliceLen, not rvGetSliceCap.
+	t := reflectArrayOf(rvGetSliceLen(rv), rv.Type().Elem())
 	uv := (*unsafeReflectValue)(unsafe.Pointer(&v))
 	uv.flag = uintptr(reflect.Array) | unsafeFlagIndir | unsafeFlagAddr
 	uv.typ = ((*unsafeIntf)(unsafe.Pointer(&t))).word

codec/json.go

@@ -661,22 +661,12 @@ func (d *jsonDecDriver) CheckBreak() bool {
 	return d.tok == '}' || d.tok == ']'
 }
 
-// For the ReadXXX methods below, we could just delegate to helper functions
-// readContainerState(c containerState, xc uint8, check bool)
-// - ReadArrayElem would become:
-//   readContainerState(containerArrayElem, ',', d.d.c != containerArrayStart)
-//
-// However, until mid-stack inlining comes in go1.11 which supports inlining of
-// one-liners, we explicitly write them all 5 out to elide the extra func call.
-//
-// TODO: For Go 1.11, if inlined, consider consolidating these.
-
 func (d *jsonDecDriver) ReadArrayElem() {
 	const xc uint8 = ','
-	d.advance()
 	if d.d.c != containerArrayStart {
+		d.advance()
 		if d.tok != xc {
-			d.d.errorf("read array element - expect char '%c' but got char '%c'", xc, d.tok)
+			d.readDelimError(xc)
 		}
 		d.tok = 0
 	}
@@ -686,17 +676,17 @@ func (d *jsonDecDriver) ReadArrayEnd() {
 	const xc uint8 = ']'
 	d.advance()
 	if d.tok != xc {
-		d.d.errorf("read array end - expect char '%c' but got char '%c'", xc, d.tok)
+		d.readDelimError(xc)
 	}
 	d.tok = 0
 }
 
 func (d *jsonDecDriver) ReadMapElemKey() {
 	const xc uint8 = ','
-	d.advance()
 	if d.d.c != containerMapStart {
+		d.advance()
 		if d.tok != xc {
-			d.d.errorf("read map key - expect char '%c' but got char '%c'", xc, d.tok)
+			d.readDelimError(xc)
 		}
 		d.tok = 0
 	}
@@ -706,7 +696,7 @@ func (d *jsonDecDriver) ReadMapElemValue() {
 	const xc uint8 = ':'
 	d.advance()
 	if d.tok != xc {
-		d.d.errorf("read map value - expect char '%c' but got char '%c'", xc, d.tok)
+		d.readDelimError(xc)
 	}
 	d.tok = 0
 }
@@ -715,11 +705,47 @@ func (d *jsonDecDriver) ReadMapEnd() {
 	const xc uint8 = '}'
 	d.advance()
 	if d.tok != xc {
-		d.d.errorf("read map end - expect char '%c' but got char '%c'", xc, d.tok)
+		d.readDelimError(xc)
 	}
 	d.tok = 0
 }
 
+// func (d *jsonDecDriver) readDelim(xc uint8) {
+// 	d.advance()
+// 	if d.tok != xc {
+// 		d.d.errorf("read json delimiter - expect char '%c' but got char '%c'", xc, d.tok)
+// 	}
+// 	d.tok = 0
+// }
+
+// func (d *jsonDecDriver) readDelim(xc uint8) {
+// 	d.advance()
+// 	if d.tok != xc {
+// 		d.d.errorf("read json delimiter - expect char '%c' but got char '%c'", xc, d.tok)
+// 	}
+// 	d.tok = 0
+// }
+
+// func (d *jsonDecDriver) readDelim(xc uint8) {
+// 	d.advance()
+// 	if d.tok != xc {
+// 		d.readDelimError(xc)
+// 	}
+// 	d.tok = 0
+// }
+
+// func (d *jsonDecDriver) readDelim(xc uint8) {
+// 	if d.tok != xc {
+// 		d.d.errorf("read json delimiter - expect char '%c' but got char '%c'", xc, d.tok)
+// 	}
+// 	d.tok = 0
+// }
+
+// //go:noinline
+func (d *jsonDecDriver) readDelimError(xc uint8) {
+	d.d.errorf("read json delimiter - expect char '%c' but got char '%c'", xc, d.tok)
+}
+
 // func (d *jsonDecDriver) readLit(length, fromIdx uint8) {
 // 	// length here is always less than 8 (literals are: null, true, false)
 // 	bs := d.d.decRd.readx(int(length))
@@ -1043,7 +1069,7 @@ func (d *jsonDecDriver) DecodeBytes(bs []byte, zerocopy bool) (bsOut []byte) {
 		// } else {
 		// 	d.bp.get(slen)
 		// 	bsOut = d.buf
-		// 	// bsOut = make([]byte, slen) // TODO: should i check pool? how to return it back?
+		// 	// bsOut = make([]byte, slen)
 		// }
 	} else {
 		bsOut = make([]byte, slen)

codec/xml.go

@@ -421,7 +421,7 @@ func (x *xmlParser) next() (t *xmlToken) {
 // nextTag will parses the next element and fill up toks.
 // It set done flag if/once EOF is reached.
 func (x *xmlParser) nextTag() {
-	// TODO: implement.
+	// ...
 }
 
 // ----------- ENCODER -------------------