From 269da15ba9900a10bb9ab121ef7f90d9acaf4e20 Mon Sep 17 00:00:00 2001
From: jonathan gold <goldfish@uber.com>
Date: Tue, 5 Nov 2019 21:28:59 -0800
Subject: [PATCH 1/5] WIP: Sketch of an implementation for
 https://github.com/uber-go/zap/issues/753

---
 field.go                     | 30 ++++++++++++++++++++++++++++++
 field_test.go                | 12 ++++++++++++
 zapcore/json_encoder_test.go |  2 ++
 3 files changed, 44 insertions(+)

diff --git a/field.go b/field.go
index 5130e1347..478bc76a3 100644
--- a/field.go
+++ b/field.go
@@ -38,6 +38,25 @@ func Skip() Field {
 	return Field{Type: zapcore.SkipType}
 }
 
+// nilField returns a field which will marshal explicitly as null. See original
+// motivation in https://github.com/uber-go/zap/issues/753 .
+//
+// TODO: We might also consider a way to widen the overall APIs in this package,
+// ultimately exporting this function, by:
+//
+//	- Adding zapcore.NilType
+//
+//	- Adding zapcore.ObjectEncoder.AddNil()
+//
+// Doing so will provide encoder implementations a more efficient way to encode
+// nil (primary benefit), and would also help with debugging or maintaining the
+// implementation of zap itself (secondary benefit).
+//
+// But we avoid doing this until we are ready for a major version change, since
+// it would potentially break users of this package who switch on
+// zapcore.FieldType or which have implemented zapcore.ObjectEncoder.
+func nilField(key string) Field { return Reflect(key, nil) }
+
 // Binary constructs a field that carries an opaque binary blob.
 //
 // Binary data is serialized in an encoding-appropriate format. For example,
@@ -56,6 +75,15 @@ func Bool(key string, val bool) Field {
 	return Field{Key: key, Type: zapcore.BoolType, Integer: ival}
 }
 
+// Boolp constructs a field that carries a *bool. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Boolp(key string, val *bool) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Bool(key, *val)
+}
+
 // ByteString constructs a field that carries UTF-8 encoded text as a []byte.
 // To log opaque binary blobs (which aren't necessarily valid UTF-8), use
 // Binary.
@@ -224,6 +252,8 @@ func Any(key string, value interface{}) Field {
 		return Array(key, val)
 	case bool:
 		return Bool(key, val)
+	case *bool:
+		return Boolp(key, val)
 	case []bool:
 		return Bools(key, val)
 	case complex128:
diff --git a/field_test.go b/field_test.go
index 069bfe372..fbe61fd17 100644
--- a/field_test.go
+++ b/field_test.go
@@ -63,6 +63,13 @@ func TestFieldConstructors(t *testing.T) {
 	name := username("phil")
 	ints := []int{5, 6}
 
+	// Helpful values for use in constructing pointers to primitives below.
+	var (
+		boolTrue = true
+		// TODO(goldfish): round out the rest of the primitive types for
+		// https://github.com/uber-go/zap/issues/753
+	)
+
 	tests := []struct {
 		name   string
 		field  Field
@@ -91,6 +98,7 @@ func TestFieldConstructors(t *testing.T) {
 		{"Uint8", Field{Key: "k", Type: zapcore.Uint8Type, Integer: 1}, Uint8("k", 1)},
 		{"Uintptr", Field{Key: "k", Type: zapcore.UintptrType, Integer: 10}, Uintptr("k", 0xa)},
 		{"Reflect", Field{Key: "k", Type: zapcore.ReflectType, Interface: ints}, Reflect("k", ints)},
+		{"Reflect", Field{Key: "k", Type: zapcore.ReflectType}, Reflect("k", nil)},
 		{"Stringer", Field{Key: "k", Type: zapcore.StringerType, Interface: addr}, Stringer("k", addr)},
 		{"Object", Field{Key: "k", Type: zapcore.ObjectMarshalerType, Interface: name}, Object("k", name)},
 		{"Any:ObjectMarshaler", Any("k", name), Object("k", name)},
@@ -139,6 +147,10 @@ func TestFieldConstructors(t *testing.T) {
 		{"Any:Duration", Any("k", time.Second), Duration("k", time.Second)},
 		{"Any:Durations", Any("k", []time.Duration{time.Second}), Durations("k", []time.Duration{time.Second})},
 		{"Any:Fallback", Any("k", struct{}{}), Reflect("k", struct{}{})},
+		{"Ptr:Bool", Boolp("k", nil), Reflect("k", nil)},
+		{"Ptr:Bool", Boolp("k", &boolTrue), Bool("k", true)},
+		{"Any:PtrBool", Any("k", (*bool)(nil)), Reflect("k", nil)},
+		{"Any:PtrBool", Any("k", &boolTrue), Bool("k", true)},
 		{"Namespace", Namespace("k"), Field{Key: "k", Type: zapcore.NamespaceType}},
 	}
 
diff --git a/zapcore/json_encoder_test.go b/zapcore/json_encoder_test.go
index 31e0852df..dfe2925f0 100644
--- a/zapcore/json_encoder_test.go
+++ b/zapcore/json_encoder_test.go
@@ -64,6 +64,7 @@ func TestJSONEncodeEntry(t *testing.T) {
 				"so": "passes",
 				"answer": 42,
 				"common_pie": 3.14,
+				"null_value": null,
 				"such": {
 					"aee": "lol",
 					"bee": 123,
@@ -84,6 +85,7 @@ func TestJSONEncodeEntry(t *testing.T) {
 				zap.String("so", "passes"),
 				zap.Int("answer", 42),
 				zap.Float64("common_pie", 3.14),
+				zap.Reflect("null_value", nil),
 				zap.Reflect("such", foo{
 					A: "lol",
 					B: 123,

From 9b708454bb9fbe3ca0c24d96d68552375994937e Mon Sep 17 00:00:00 2001
From: jonathan gold <goldfish@uber.com>
Date: Tue, 5 Nov 2019 21:34:48 -0800
Subject: [PATCH 2/5] small cosmetic changes

---
 field.go      | 4 ++--
 field_test.go | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/field.go b/field.go
index 478bc76a3..e2dfc98ec 100644
--- a/field.go
+++ b/field.go
@@ -38,8 +38,8 @@ func Skip() Field {
 	return Field{Type: zapcore.SkipType}
 }
 
-// nilField returns a field which will marshal explicitly as null. See original
-// motivation in https://github.com/uber-go/zap/issues/753 .
+// nilField returns a field which will marshal explicitly as nil. See motivation
+// in https://github.com/uber-go/zap/issues/753 .
 //
 // TODO: We might also consider a way to widen the overall APIs in this package,
 // ultimately exporting this function, by:
diff --git a/field_test.go b/field_test.go
index fbe61fd17..25c77bc04 100644
--- a/field_test.go
+++ b/field_test.go
@@ -147,10 +147,11 @@ func TestFieldConstructors(t *testing.T) {
 		{"Any:Duration", Any("k", time.Second), Duration("k", time.Second)},
 		{"Any:Durations", Any("k", []time.Duration{time.Second}), Durations("k", []time.Duration{time.Second})},
 		{"Any:Fallback", Any("k", struct{}{}), Reflect("k", struct{}{})},
-		{"Ptr:Bool", Boolp("k", nil), Reflect("k", nil)},
+		{"Ptr:Bool", Boolp("k", nil), nilField("k")},
 		{"Ptr:Bool", Boolp("k", &boolTrue), Bool("k", true)},
-		{"Any:PtrBool", Any("k", (*bool)(nil)), Reflect("k", nil)},
+		{"Any:PtrBool", Any("k", (*bool)(nil)), nilField("k")},
 		{"Any:PtrBool", Any("k", &boolTrue), Bool("k", true)},
+		// TODO(goldfish): rest of Ptr* for scalar types...
 		{"Namespace", Namespace("k"), Field{Key: "k", Type: zapcore.NamespaceType}},
 	}
 

From 606e9d7eba3764f3c69792e23caaee9ba1337572 Mon Sep 17 00:00:00 2001
From: jonathan gold <goldfish@uber.com>
Date: Wed, 6 Nov 2019 16:54:00 -0800
Subject: [PATCH 3/5] review updates

---
 buffer/buffer.go             |   7 +-
 field.go                     | 217 ++++++++++++++++++++++++++++++++---
 field_test.go                |  99 +++++++++++++++-
 zapcore/encoder.go           |   4 +-
 zapcore/json_encoder.go      |  26 +++--
 zapcore/json_encoder_test.go |   1 +
 6 files changed, 322 insertions(+), 32 deletions(-)

diff --git a/buffer/buffer.go b/buffer/buffer.go
index 7592e8c63..2b292c762 100644
--- a/buffer/buffer.go
+++ b/buffer/buffer.go
@@ -44,6 +44,11 @@ func (b *Buffer) AppendString(s string) {
 	b.bs = append(b.bs, s...)
 }
 
+// AppendBytes appends a byte slice to the Buffer.
+func (b *Buffer) AppendBytes(bs []byte) {
+	b.bs = append(b.bs, bs...)
+}
+
 // AppendInt appends an integer to the underlying buffer (assuming base 10).
 func (b *Buffer) AppendInt(i int64) {
 	b.bs = strconv.AppendInt(b.bs, i, 10)
@@ -94,7 +99,7 @@ func (b *Buffer) Reset() {
 
 // Write implements io.Writer.
 func (b *Buffer) Write(bs []byte) (int, error) {
-	b.bs = append(b.bs, bs...)
+	b.AppendBytes(bs)
 	return len(bs), nil
 }
 
diff --git a/field.go b/field.go
index e2dfc98ec..83c1ea245 100644
--- a/field.go
+++ b/field.go
@@ -39,22 +39,9 @@ func Skip() Field {
 }
 
 // nilField returns a field which will marshal explicitly as nil. See motivation
-// in https://github.com/uber-go/zap/issues/753 .
-//
-// TODO: We might also consider a way to widen the overall APIs in this package,
-// ultimately exporting this function, by:
-//
-//	- Adding zapcore.NilType
-//
-//	- Adding zapcore.ObjectEncoder.AddNil()
-//
-// Doing so will provide encoder implementations a more efficient way to encode
-// nil (primary benefit), and would also help with debugging or maintaining the
-// implementation of zap itself (secondary benefit).
-//
-// But we avoid doing this until we are ready for a major version change, since
-// it would potentially break users of this package who switch on
-// zapcore.FieldType or which have implemented zapcore.ObjectEncoder.
+// in https://github.com/uber-go/zap/issues/753 . If we ever make breaking
+// changes and add zapcore.NilType and zapcore.ObjectEncoder.AddNil, the
+// implementation here should be changed to reflect that.
 func nilField(key string) Field { return Reflect(key, nil) }
 
 // Binary constructs a field that carries an opaque binary blob.
@@ -98,6 +85,15 @@ func Complex128(key string, val complex128) Field {
 	return Field{Key: key, Type: zapcore.Complex128Type, Interface: val}
 }
 
+// Complex128p constructs a field that carries a *complex128. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Complex128p(key string, val *complex128) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Complex128(key, *val)
+}
+
 // Complex64 constructs a field that carries a complex number. Unlike most
 // numeric fields, this costs an allocation (to convert the complex64 to
 // interface{}).
@@ -105,6 +101,15 @@ func Complex64(key string, val complex64) Field {
 	return Field{Key: key, Type: zapcore.Complex64Type, Interface: val}
 }
 
+// Complex64p constructs a field that carries a *complex64. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Complex64p(key string, val *complex64) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Complex64(key, *val)
+}
+
 // Float64 constructs a field that carries a float64. The way the
 // floating-point value is represented is encoder-dependent, so marshaling is
 // necessarily lazy.
@@ -112,6 +117,15 @@ func Float64(key string, val float64) Field {
 	return Field{Key: key, Type: zapcore.Float64Type, Integer: int64(math.Float64bits(val))}
 }
 
+// Float64p constructs a field that carries a *float64. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Float64p(key string, val *float64) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Float64(key, *val)
+}
+
 // Float32 constructs a field that carries a float32. The way the
 // floating-point value is represented is encoder-dependent, so marshaling is
 // necessarily lazy.
@@ -119,66 +133,183 @@ func Float32(key string, val float32) Field {
 	return Field{Key: key, Type: zapcore.Float32Type, Integer: int64(math.Float32bits(val))}
 }
 
+// Float32p constructs a field that carries a *float32. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Float32p(key string, val *float32) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Float32(key, *val)
+}
+
 // Int constructs a field with the given key and value.
 func Int(key string, val int) Field {
 	return Int64(key, int64(val))
 }
 
+// Intp constructs a field that carries a *int. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Intp(key string, val *int) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Int(key, *val)
+}
+
 // Int64 constructs a field with the given key and value.
 func Int64(key string, val int64) Field {
 	return Field{Key: key, Type: zapcore.Int64Type, Integer: val}
 }
 
+// Int64p constructs a field that carries a *int64. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Int64p(key string, val *int64) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Int64(key, *val)
+}
+
 // Int32 constructs a field with the given key and value.
 func Int32(key string, val int32) Field {
 	return Field{Key: key, Type: zapcore.Int32Type, Integer: int64(val)}
 }
 
+// Int32p constructs a field that carries a *int32. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Int32p(key string, val *int32) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Int32(key, *val)
+}
+
 // Int16 constructs a field with the given key and value.
 func Int16(key string, val int16) Field {
 	return Field{Key: key, Type: zapcore.Int16Type, Integer: int64(val)}
 }
 
+// Int16p constructs a field that carries a *int16. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Int16p(key string, val *int16) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Int16(key, *val)
+}
+
 // Int8 constructs a field with the given key and value.
 func Int8(key string, val int8) Field {
 	return Field{Key: key, Type: zapcore.Int8Type, Integer: int64(val)}
 }
 
+// Int8p constructs a field that carries a *int8. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Int8p(key string, val *int8) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Int8(key, *val)
+}
+
 // String constructs a field with the given key and value.
 func String(key string, val string) Field {
 	return Field{Key: key, Type: zapcore.StringType, String: val}
 }
 
+// Stringp constructs a field that carries a *string. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Stringp(key string, val *string) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return String(key, *val)
+}
+
 // Uint constructs a field with the given key and value.
 func Uint(key string, val uint) Field {
 	return Uint64(key, uint64(val))
 }
 
+// Uintp constructs a field that carries a *uint. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Uintp(key string, val *uint) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Uint(key, *val)
+}
+
 // Uint64 constructs a field with the given key and value.
 func Uint64(key string, val uint64) Field {
 	return Field{Key: key, Type: zapcore.Uint64Type, Integer: int64(val)}
 }
 
+// Uint64p constructs a field that carries a *uint64. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Uint64p(key string, val *uint64) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Uint64(key, *val)
+}
+
 // Uint32 constructs a field with the given key and value.
 func Uint32(key string, val uint32) Field {
 	return Field{Key: key, Type: zapcore.Uint32Type, Integer: int64(val)}
 }
 
+// Uint32p constructs a field that carries a *uint32. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Uint32p(key string, val *uint32) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Uint32(key, *val)
+}
+
 // Uint16 constructs a field with the given key and value.
 func Uint16(key string, val uint16) Field {
 	return Field{Key: key, Type: zapcore.Uint16Type, Integer: int64(val)}
 }
 
+// Uint16p constructs a field that carries a *uint16. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Uint16p(key string, val *uint16) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Uint16(key, *val)
+}
+
 // Uint8 constructs a field with the given key and value.
 func Uint8(key string, val uint8) Field {
 	return Field{Key: key, Type: zapcore.Uint8Type, Integer: int64(val)}
 }
 
+// Uint8p constructs a field that carries a *uint8. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Uint8p(key string, val *uint8) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Uint8(key, *val)
+}
+
 // Uintptr constructs a field with the given key and value.
 func Uintptr(key string, val uintptr) Field {
 	return Field{Key: key, Type: zapcore.UintptrType, Integer: int64(val)}
 }
 
+// Uintptrp constructs a field that carries a *uintptr. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Uintptrp(key string, val *uintptr) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Uintptr(key, *val)
+}
+
 // Reflect constructs a field with the given key and an arbitrary object. It uses
 // an encoding-appropriate, reflection-based function to lazily serialize nearly
 // any object into the logging context, but it's relatively slow and
@@ -211,6 +342,15 @@ func Time(key string, val time.Time) Field {
 	return Field{Key: key, Type: zapcore.TimeType, Integer: val.UnixNano(), Interface: val.Location()}
 }
 
+// Timep constructs a field that carries a *time.Time. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Timep(key string, val *time.Time) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Time(key, *val)
+}
+
 // Stack constructs a field that stores a stacktrace of the current goroutine
 // under provided key. Keep in mind that taking a stacktrace is eager and
 // expensive (relatively speaking); this function both makes an allocation and
@@ -229,6 +369,15 @@ func Duration(key string, val time.Duration) Field {
 	return Field{Key: key, Type: zapcore.DurationType, Integer: int64(val)}
 }
 
+// Durationp constructs a field that carries a *time.Duration. The returned Field will safely
+// and explicitly represent `nil` when appropriate.
+func Durationp(key string, val *time.Duration) Field {
+	if val == nil {
+		return nilField(key)
+	}
+	return Duration(key, *val)
+}
+
 // Object constructs a field with the given key and ObjectMarshaler. It
 // provides a flexible, but still type-safe and efficient, way to add map- or
 // struct-like user-defined types to the logging context. The struct's
@@ -258,74 +407,110 @@ func Any(key string, value interface{}) Field {
 		return Bools(key, val)
 	case complex128:
 		return Complex128(key, val)
+	case *complex128:
+		return Complex128p(key, val)
 	case []complex128:
 		return Complex128s(key, val)
 	case complex64:
 		return Complex64(key, val)
+	case *complex64:
+		return Complex64p(key, val)
 	case []complex64:
 		return Complex64s(key, val)
 	case float64:
 		return Float64(key, val)
+	case *float64:
+		return Float64p(key, val)
 	case []float64:
 		return Float64s(key, val)
 	case float32:
 		return Float32(key, val)
+	case *float32:
+		return Float32p(key, val)
 	case []float32:
 		return Float32s(key, val)
 	case int:
 		return Int(key, val)
+	case *int:
+		return Intp(key, val)
 	case []int:
 		return Ints(key, val)
 	case int64:
 		return Int64(key, val)
+	case *int64:
+		return Int64p(key, val)
 	case []int64:
 		return Int64s(key, val)
 	case int32:
 		return Int32(key, val)
+	case *int32:
+		return Int32p(key, val)
 	case []int32:
 		return Int32s(key, val)
 	case int16:
 		return Int16(key, val)
+	case *int16:
+		return Int16p(key, val)
 	case []int16:
 		return Int16s(key, val)
 	case int8:
 		return Int8(key, val)
+	case *int8:
+		return Int8p(key, val)
 	case []int8:
 		return Int8s(key, val)
 	case string:
 		return String(key, val)
+	case *string:
+		return Stringp(key, val)
 	case []string:
 		return Strings(key, val)
 	case uint:
 		return Uint(key, val)
+	case *uint:
+		return Uintp(key, val)
 	case []uint:
 		return Uints(key, val)
 	case uint64:
 		return Uint64(key, val)
+	case *uint64:
+		return Uint64p(key, val)
 	case []uint64:
 		return Uint64s(key, val)
 	case uint32:
 		return Uint32(key, val)
+	case *uint32:
+		return Uint32p(key, val)
 	case []uint32:
 		return Uint32s(key, val)
 	case uint16:
 		return Uint16(key, val)
+	case *uint16:
+		return Uint16p(key, val)
 	case []uint16:
 		return Uint16s(key, val)
 	case uint8:
 		return Uint8(key, val)
+	case *uint8:
+		return Uint8p(key, val)
 	case []byte:
 		return Binary(key, val)
 	case uintptr:
 		return Uintptr(key, val)
+	case *uintptr:
+		return Uintptrp(key, val)
 	case []uintptr:
 		return Uintptrs(key, val)
 	case time.Time:
 		return Time(key, val)
+	case *time.Time:
+		return Timep(key, val)
 	case []time.Time:
 		return Times(key, val)
 	case time.Duration:
 		return Duration(key, val)
+	case *time.Duration:
+		return Durationp(key, val)
 	case []time.Duration:
 		return Durations(key, val)
 	case error:
diff --git a/field_test.go b/field_test.go
index 25c77bc04..d25c964fb 100644
--- a/field_test.go
+++ b/field_test.go
@@ -65,9 +65,25 @@ func TestFieldConstructors(t *testing.T) {
 
 	// Helpful values for use in constructing pointers to primitives below.
 	var (
-		boolTrue = true
-		// TODO(goldfish): round out the rest of the primitive types for
-		// https://github.com/uber-go/zap/issues/753
+		boolVal       bool          = true
+		complex128Val complex128    = complex(0, 0)
+		complex64Val  complex64     = complex(0, 0)
+		durationVal   time.Duration = time.Second
+		float64Val    float64       = 1.0
+		float32Val    float32       = 1.0
+		intVal        int           = 1
+		int64Val      int64         = 1
+		int32Val      int32         = 1
+		int16Val      int16         = 1
+		int8Val       int8          = 1
+		stringVal     string        = "hello"
+		timeVal       time.Time     = time.Unix(100000, 0)
+		uintVal       uint          = 1
+		uint64Val     uint64        = 1
+		uint32Val     uint32        = 1
+		uint16Val     uint16        = 1
+		uint8Val      uint8         = 1
+		uintptrVal    uintptr       = 1
 	)
 
 	tests := []struct {
@@ -148,10 +164,81 @@ func TestFieldConstructors(t *testing.T) {
 		{"Any:Durations", Any("k", []time.Duration{time.Second}), Durations("k", []time.Duration{time.Second})},
 		{"Any:Fallback", Any("k", struct{}{}), Reflect("k", struct{}{})},
 		{"Ptr:Bool", Boolp("k", nil), nilField("k")},
-		{"Ptr:Bool", Boolp("k", &boolTrue), Bool("k", true)},
+		{"Ptr:Bool", Boolp("k", &boolVal), Bool("k", boolVal)},
 		{"Any:PtrBool", Any("k", (*bool)(nil)), nilField("k")},
-		{"Any:PtrBool", Any("k", &boolTrue), Bool("k", true)},
-		// TODO(goldfish): rest of Ptr* for scalar types...
+		{"Any:PtrBool", Any("k", &boolVal), Bool("k", boolVal)},
+		{"Ptr:Complex128", Complex128p("k", nil), nilField("k")},
+		{"Ptr:Complex128", Complex128p("k", &complex128Val), Complex128("k", complex128Val)},
+		{"Any:PtrComplex128", Any("k", (*complex128)(nil)), nilField("k")},
+		{"Any:PtrComplex128", Any("k", &complex128Val), Complex128("k", complex128Val)},
+		{"Ptr:Complex64", Complex64p("k", nil), nilField("k")},
+		{"Ptr:Complex64", Complex64p("k", &complex64Val), Complex64("k", complex64Val)},
+		{"Any:PtrComplex64", Any("k", (*complex64)(nil)), nilField("k")},
+		{"Any:PtrComplex64", Any("k", &complex64Val), Complex64("k", complex64Val)},
+		{"Ptr:Duration", Durationp("k", nil), nilField("k")},
+		{"Ptr:Duration", Durationp("k", &durationVal), Duration("k", durationVal)},
+		{"Any:PtrDuration", Any("k", (*time.Duration)(nil)), nilField("k")},
+		{"Any:PtrDuration", Any("k", &durationVal), Duration("k", durationVal)},
+		{"Ptr:Float64", Float64p("k", nil), nilField("k")},
+		{"Ptr:Float64", Float64p("k", &float64Val), Float64("k", float64Val)},
+		{"Any:PtrFloat64", Any("k", (*float64)(nil)), nilField("k")},
+		{"Any:PtrFloat64", Any("k", &float64Val), Float64("k", float64Val)},
+		{"Ptr:Float32", Float32p("k", nil), nilField("k")},
+		{"Ptr:Float32", Float32p("k", &float32Val), Float32("k", float32Val)},
+		{"Any:PtrFloat32", Any("k", (*float32)(nil)), nilField("k")},
+		{"Any:PtrFloat32", Any("k", &float32Val), Float32("k", float32Val)},
+		{"Ptr:Int", Intp("k", nil), nilField("k")},
+		{"Ptr:Int", Intp("k", &intVal), Int("k", intVal)},
+		{"Any:PtrInt", Any("k", (*int)(nil)), nilField("k")},
+		{"Any:PtrInt", Any("k", &intVal), Int("k", intVal)},
+		{"Ptr:Int64", Int64p("k", nil), nilField("k")},
+		{"Ptr:Int64", Int64p("k", &int64Val), Int64("k", int64Val)},
+		{"Any:PtrInt64", Any("k", (*int64)(nil)), nilField("k")},
+		{"Any:PtrInt64", Any("k", &int64Val), Int64("k", int64Val)},
+		{"Ptr:Int32", Int32p("k", nil), nilField("k")},
+		{"Ptr:Int32", Int32p("k", &int32Val), Int32("k", int32Val)},
+		{"Any:PtrInt32", Any("k", (*int32)(nil)), nilField("k")},
+		{"Any:PtrInt32", Any("k", &int32Val), Int32("k", int32Val)},
+		{"Ptr:Int16", Int16p("k", nil), nilField("k")},
+		{"Ptr:Int16", Int16p("k", &int16Val), Int16("k", int16Val)},
+		{"Any:PtrInt16", Any("k", (*int16)(nil)), nilField("k")},
+		{"Any:PtrInt16", Any("k", &int16Val), Int16("k", int16Val)},
+		{"Ptr:Int8", Int8p("k", nil), nilField("k")},
+		{"Ptr:Int8", Int8p("k", &int8Val), Int8("k", int8Val)},
+		{"Any:PtrInt8", Any("k", (*int8)(nil)), nilField("k")},
+		{"Any:PtrInt8", Any("k", &int8Val), Int8("k", int8Val)},
+		{"Ptr:String", Stringp("k", nil), nilField("k")},
+		{"Ptr:String", Stringp("k", &stringVal), String("k", stringVal)},
+		{"Any:PtrString", Any("k", (*string)(nil)), nilField("k")},
+		{"Any:PtrString", Any("k", &stringVal), String("k", stringVal)},
+		{"Ptr:Time", Timep("k", nil), nilField("k")},
+		{"Ptr:Time", Timep("k", &timeVal), Time("k", timeVal)},
+		{"Any:PtrTime", Any("k", (*time.Time)(nil)), nilField("k")},
+		{"Any:PtrTime", Any("k", &timeVal), Time("k", timeVal)},
+		{"Ptr:Uint", Uintp("k", nil), nilField("k")},
+		{"Ptr:Uint", Uintp("k", &uintVal), Uint("k", uintVal)},
+		{"Any:PtrUint", Any("k", (*uint)(nil)), nilField("k")},
+		{"Any:PtrUint", Any("k", &uintVal), Uint("k", uintVal)},
+		{"Ptr:Uint64", Uint64p("k", nil), nilField("k")},
+		{"Ptr:Uint64", Uint64p("k", &uint64Val), Uint64("k", uint64Val)},
+		{"Any:PtrUint64", Any("k", (*uint64)(nil)), nilField("k")},
+		{"Any:PtrUint64", Any("k", &uint64Val), Uint64("k", uint64Val)},
+		{"Ptr:Uint32", Uint32p("k", nil), nilField("k")},
+		{"Ptr:Uint32", Uint32p("k", &uint32Val), Uint32("k", uint32Val)},
+		{"Any:PtrUint32", Any("k", (*uint32)(nil)), nilField("k")},
+		{"Any:PtrUint32", Any("k", &uint32Val), Uint32("k", uint32Val)},
+		{"Ptr:Uint16", Uint16p("k", nil), nilField("k")},
+		{"Ptr:Uint16", Uint16p("k", &uint16Val), Uint16("k", uint16Val)},
+		{"Any:PtrUint16", Any("k", (*uint16)(nil)), nilField("k")},
+		{"Any:PtrUint16", Any("k", &uint16Val), Uint16("k", uint16Val)},
+		{"Ptr:Uint8", Uint8p("k", nil), nilField("k")},
+		{"Ptr:Uint8", Uint8p("k", &uint8Val), Uint8("k", uint8Val)},
+		{"Any:PtrUint8", Any("k", (*uint8)(nil)), nilField("k")},
+		{"Any:PtrUint8", Any("k", &uint8Val), Uint8("k", uint8Val)},
+		{"Ptr:Uintptr", Uintptrp("k", nil), nilField("k")},
+		{"Ptr:Uintptr", Uintptrp("k", &uintptrVal), Uintptr("k", uintptrVal)},
+		{"Any:PtrUintptr", Any("k", (*uintptr)(nil)), nilField("k")},
+		{"Any:PtrUintptr", Any("k", &uintptrVal), Uintptr("k", uintptrVal)},
 		{"Namespace", Namespace("k"), Field{Key: "k", Type: zapcore.NamespaceType}},
 	}
 
diff --git a/zapcore/encoder.go b/zapcore/encoder.go
index 8d0fc72ce..5e0a69be5 100644
--- a/zapcore/encoder.go
+++ b/zapcore/encoder.go
@@ -294,8 +294,8 @@ type ObjectEncoder interface {
 	AddUint8(key string, value uint8)
 	AddUintptr(key string, value uintptr)
 
-	// AddReflected uses reflection to serialize arbitrary objects, so it's slow
-	// and allocation-heavy.
+	// AddReflected uses reflection to serialize arbitrary objects, so it can be
+	// slow and allocation-heavy.
 	AddReflected(key string, value interface{}) error
 	// OpenNamespace opens an isolated namespace where all subsequent fields will
 	// be added. Applications can use namespaces to prevent key collisions when
diff --git a/zapcore/json_encoder.go b/zapcore/json_encoder.go
index 9aec4eada..cbf002fef 100644
--- a/zapcore/json_encoder.go
+++ b/zapcore/json_encoder.go
@@ -145,16 +145,28 @@ func (enc *jsonEncoder) resetReflectBuf() {
 	}
 }
 
+var nullLiteralBytes = []byte("null")
+
 func (enc *jsonEncoder) AddReflected(key string, obj interface{}) error {
-	enc.resetReflectBuf()
-	err := enc.reflectEnc.Encode(obj)
-	if err != nil {
-		return err
+	// Only invoke the standard JSON encoder if there is actually something to
+	// encode; otherwise write JSON null literal directly. For now, we only
+	// apply this optimization here, but not in AppendReflected. We could apply
+	// it there as well if needed (some discussion in
+	// https://github.com/uber-go/zap/issues/630 indicates that it could be
+	// valuable in some cases) but the primary known benefit at the moment is
+	// from primitive callers (see https://github.com/uber-go/zap/issues/753)
+	valueBytes := nullLiteralBytes
+	if obj != nil {
+		enc.resetReflectBuf()
+		if err := enc.reflectEnc.Encode(obj); err != nil {
+			return err
+		}
+		enc.reflectBuf.TrimNewline()
+		valueBytes = enc.reflectBuf.Bytes()
 	}
-	enc.reflectBuf.TrimNewline()
 	enc.addKey(key)
-	_, err = enc.buf.Write(enc.reflectBuf.Bytes())
-	return err
+	enc.buf.AppendBytes(valueBytes)
+	return nil
 }
 
 func (enc *jsonEncoder) OpenNamespace(key string) {
diff --git a/zapcore/json_encoder_test.go b/zapcore/json_encoder_test.go
index dfe2925f0..f13877ee0 100644
--- a/zapcore/json_encoder_test.go
+++ b/zapcore/json_encoder_test.go
@@ -85,6 +85,7 @@ func TestJSONEncodeEntry(t *testing.T) {
 				zap.String("so", "passes"),
 				zap.Int("answer", 42),
 				zap.Float64("common_pie", 3.14),
+				// Test special-cased handling of nil in AddReflect()
 				zap.Reflect("null_value", nil),
 				zap.Reflect("such", foo{
 					A: "lol",

From fefc76251f58a84a7ed32dc275f6233bcb846014 Mon Sep 17 00:00:00 2001
From: jonathan gold <goldfish@uber.com>
Date: Wed, 6 Nov 2019 20:47:18 -0800
Subject: [PATCH 4/5] Backout `AppendBytes`

---
 buffer/buffer.go        | 7 +------
 zapcore/json_encoder.go | 4 ++--
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/buffer/buffer.go b/buffer/buffer.go
index 2b292c762..7592e8c63 100644
--- a/buffer/buffer.go
+++ b/buffer/buffer.go
@@ -44,11 +44,6 @@ func (b *Buffer) AppendString(s string) {
 	b.bs = append(b.bs, s...)
 }
 
-// AppendBytes appends a byte slice to the Buffer.
-func (b *Buffer) AppendBytes(bs []byte) {
-	b.bs = append(b.bs, bs...)
-}
-
 // AppendInt appends an integer to the underlying buffer (assuming base 10).
 func (b *Buffer) AppendInt(i int64) {
 	b.bs = strconv.AppendInt(b.bs, i, 10)
@@ -99,7 +94,7 @@ func (b *Buffer) Reset() {
 
 // Write implements io.Writer.
 func (b *Buffer) Write(bs []byte) (int, error) {
-	b.AppendBytes(bs)
+	b.bs = append(b.bs, bs...)
 	return len(bs), nil
 }
 
diff --git a/zapcore/json_encoder.go b/zapcore/json_encoder.go
index cbf002fef..d8c4e7e8e 100644
--- a/zapcore/json_encoder.go
+++ b/zapcore/json_encoder.go
@@ -165,8 +165,8 @@ func (enc *jsonEncoder) AddReflected(key string, obj interface{}) error {
 		valueBytes = enc.reflectBuf.Bytes()
 	}
 	enc.addKey(key)
-	enc.buf.AppendBytes(valueBytes)
-	return nil
+	_, err := enc.buf.Write(valueBytes)
+	return err
 }
 
 func (enc *jsonEncoder) OpenNamespace(key string) {

From b4126bfd9016b087da5c800f049bde320b3c89fd Mon Sep 17 00:00:00 2001
From: jonathan gold <goldfish@uber.com>
Date: Wed, 13 Nov 2019 09:58:37 -0800
Subject: [PATCH 5/5] review updates

---
 zapcore/json_encoder.go      | 40 ++++++++++++++++++------------------
 zapcore/json_encoder_test.go |  8 +++++++-
 2 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/zapcore/json_encoder.go b/zapcore/json_encoder.go
index d8c4e7e8e..56256be8d 100644
--- a/zapcore/json_encoder.go
+++ b/zapcore/json_encoder.go
@@ -147,25 +147,27 @@ func (enc *jsonEncoder) resetReflectBuf() {
 
 var nullLiteralBytes = []byte("null")
 
+// Only invoke the standard JSON encoder if there is actually something to
+// encode; otherwise write JSON null literal directly.
+func (enc *jsonEncoder) encodeReflected(obj interface{}) ([]byte, error) {
+	if obj == nil {
+		return nullLiteralBytes, nil
+	}
+	enc.resetReflectBuf()
+	if err := enc.reflectEnc.Encode(obj); err != nil {
+		return nil, err
+	}
+	enc.reflectBuf.TrimNewline()
+	return enc.reflectBuf.Bytes(), nil
+}
+
 func (enc *jsonEncoder) AddReflected(key string, obj interface{}) error {
-	// Only invoke the standard JSON encoder if there is actually something to
-	// encode; otherwise write JSON null literal directly. For now, we only
-	// apply this optimization here, but not in AppendReflected. We could apply
-	// it there as well if needed (some discussion in
-	// https://github.com/uber-go/zap/issues/630 indicates that it could be
-	// valuable in some cases) but the primary known benefit at the moment is
-	// from primitive callers (see https://github.com/uber-go/zap/issues/753)
-	valueBytes := nullLiteralBytes
-	if obj != nil {
-		enc.resetReflectBuf()
-		if err := enc.reflectEnc.Encode(obj); err != nil {
-			return err
-		}
-		enc.reflectBuf.TrimNewline()
-		valueBytes = enc.reflectBuf.Bytes()
+	valueBytes, err := enc.encodeReflected(obj)
+	if err != nil {
+		return err
 	}
 	enc.addKey(key)
-	_, err := enc.buf.Write(valueBytes)
+	_, err = enc.buf.Write(valueBytes)
 	return err
 }
 
@@ -248,14 +250,12 @@ func (enc *jsonEncoder) AppendInt64(val int64) {
 }
 
 func (enc *jsonEncoder) AppendReflected(val interface{}) error {
-	enc.resetReflectBuf()
-	err := enc.reflectEnc.Encode(val)
+	valueBytes, err := enc.encodeReflected(val)
 	if err != nil {
 		return err
 	}
-	enc.reflectBuf.TrimNewline()
 	enc.addElementSeparator()
-	_, err = enc.buf.Write(enc.reflectBuf.Bytes())
+	_, err = enc.buf.Write(valueBytes)
 	return err
 }
 
diff --git a/zapcore/json_encoder_test.go b/zapcore/json_encoder_test.go
index f13877ee0..82496581b 100644
--- a/zapcore/json_encoder_test.go
+++ b/zapcore/json_encoder_test.go
@@ -65,6 +65,7 @@ func TestJSONEncodeEntry(t *testing.T) {
 				"answer": 42,
 				"common_pie": 3.14,
 				"null_value": null,
+				"array_with_null_elements": [{}, null, null, 2],
 				"such": {
 					"aee": "lol",
 					"bee": 123,
@@ -85,8 +86,13 @@ func TestJSONEncodeEntry(t *testing.T) {
 				zap.String("so", "passes"),
 				zap.Int("answer", 42),
 				zap.Float64("common_pie", 3.14),
-				// Test special-cased handling of nil in AddReflect()
+				// Cover special-cased handling of nil in AddReflect() and
+				// AppendReflect(). Note that for the latter, we explicitly test
+				// correct results for both the nil static interface{} value
+				// (`nil`), as well as the non-nil interface value with a
+				// dynamic type and nil value (`(*struct{})(nil)`).
 				zap.Reflect("null_value", nil),
+				zap.Reflect("array_with_null_elements", []interface{}{&struct{}{}, nil, (*struct{})(nil), 2}),
 				zap.Reflect("such", foo{
 					A: "lol",
 					B: 123,