slog: remove restrictions on types stored in Value

Previously, Go values of type Kind and *time.Location had special
meaning in the Value.any field, so we could not allow Values to
hold Go values of those types.

Define unexported types for those exported ones and wrap values in them.
This allows a Value to hold any Go value.

Change-Id: Iaab01ebad5d16e9f3f7df808b077e28f06539b97
Reviewed-on: https://go-review.googlesource.com/c/exp/+/442360
Reviewed-by: Alan Donovan <[email protected]>
Run-TryBot: Jonathan Amsterdam <[email protected]>

Author: jba

slog/value.go

@@ -19,6 +19,10 @@ import (
 // Kind is the kind of a Value.
 type Kind int
 
+// Unexported version of Kind, just so we can store Kinds in Values.
+// (No user-provided value has this type.)
+type kind Kind
+
 // The following list is sorted alphabetically, but it's also important that
 // AnyKind is 0 so that a zero Value represents nil.
 
@@ -86,10 +90,14 @@ func BoolValue(v bool) Value {
 	return Value{num: u, any: BoolKind}
 }
 
+// Unexported version of *time.Location, just so we can store *time.Locations in
+// Values. (No user-provided value has this type.)
+type timeLocation *time.Location
+
 // TimeValue returns a Value for a time.Time.
 // It discards the monotonic portion.
 func TimeValue(v time.Time) Value {
-	return Value{num: uint64(v.UnixNano()), any: v.Location()}
+	return Value{num: uint64(v.UnixNano()), any: timeLocation(v.Location())}
 }
 
 // DurationValue returns a Value for a time.Duration.
@@ -152,9 +160,7 @@ func AnyValue(v any) Value {
 	case []Attr:
 		return GroupValue(v...)
 	case Kind:
-		panic("cannot store a slog.Kind in a slog.Value")
-	case *time.Location:
-		panic("cannot store a *time.Location in a slog.Value")
+		return Value{any: kind(v)}
 	default:
 		return Value{any: v}
 	}
@@ -166,6 +172,9 @@ func AnyValue(v any) Value {
 func (v Value) Any() any {
 	switch v.Kind() {
 	case AnyKind, GroupKind, LogValuerKind:
+		if k, ok := v.any.(kind); ok {
+			return Kind(k)
+		}
 		return v.any
 	case Int64Kind:
 		return int64(v.num)
@@ -255,7 +264,7 @@ func (v Value) Time() time.Time {
 }
 
 func (v Value) time() time.Time {
-	return time.Unix(0, int64(v.num)).In(v.any.(*time.Location))
+	return time.Unix(0, int64(v.num)).In(v.any.(timeLocation))
 }
 
 // LogValuer returns the Value's value as a LogValuer. It panics

slog/value_safe.go

@@ -6,8 +6,6 @@
 
 package slog
 
-import "time"
-
 // This file defines the most portable representation of Value.
 
 // A Value can represent (almost) any Go value, but unlike type any,
@@ -33,12 +31,14 @@ func (v Value) Kind() Kind {
 	switch k := v.any.(type) {
 	case Kind:
 		return k
-	case *time.Location:
+	case timeLocation:
 		return TimeKind
 	case []Attr:
 		return GroupKind
 	case LogValuer:
 		return LogValuerKind
+	case kind: // a kind is just a wrapper for a Kind
+		return AnyKind
 	default:
 		return AnyKind
 	}

slog/value_test.go

@@ -21,6 +21,7 @@ func TestValueEqual(t *testing.T) {
 		Float64Value(3.7),
 		BoolValue(true),
 		BoolValue(false),
+		TimeValue(testTime),
 		AnyValue(&x),
 		AnyValue(&y),
 		GroupValue(Bool("b", true), Int("i", 3)),
@@ -62,25 +63,27 @@ func TestValueString(t *testing.T) {
 		{Float64Value(.15), "0.15"},
 		{BoolValue(true), "true"},
 		{StringValue("foo"), "foo"},
+		{TimeValue(testTime), "2000-01-02 03:04:05 +0000 UTC"},
 		{AnyValue(time.Duration(3 * time.Second)), "3s"},
 	} {
 		if got := test.v.String(); got != test.want {
-			t.Errorf("%#v: got %q, want %q", test.v, got, test.want)
+			t.Errorf("%#v:\ngot  %q\nwant %q", test.v, got, test.want)
 		}
 	}
 }
 
 func TestValueNoAlloc(t *testing.T) {
 	// Assign values just to make sure the compiler doesn't optimize away the statements.
 	var (
-		i int64
-		u uint64
-		f float64
-		b bool
-		s string
-		x any
-		p = &i
-		d time.Duration
+		i  int64
+		u  uint64
+		f  float64
+		b  bool
+		s  string
+		x  any
+		p  = &i
+		d  time.Duration
+		tm time.Time
 	)
 	a := int(testing.AllocsPerRun(5, func() {
 		i = Int64Value(1).Int64()
@@ -89,6 +92,7 @@ func TestValueNoAlloc(t *testing.T) {
 		b = BoolValue(true).Bool()
 		s = StringValue("foo").String()
 		d = DurationValue(d).Duration()
+		tm = TimeValue(testTime).Time()
 		x = AnyValue(p).Any()
 	}))
 	if a != 0 {
@@ -99,6 +103,7 @@ func TestValueNoAlloc(t *testing.T) {
 	_ = b
 	_ = s
 	_ = x
+	_ = tm
 }
 
 func TestAnyLevelAlloc(t *testing.T) {
@@ -119,6 +124,22 @@ func TestAnyLevel(t *testing.T) {
 	}
 }
 
+func TestSpecialValueTypes(t *testing.T) {
+	t.Run("time.Location", func(t *testing.T) {
+		want := time.UTC
+		got := AnyValue(want).Any()
+		if got != want {
+			t.Errorf("got %v, want %v", got, want)
+		}
+	})
+	t.Run("Kind", func(t *testing.T) {
+		want := BoolKind
+		got := AnyValue(want).Any()
+		if got != want {
+			t.Errorf("got %v, want %v", got, want)
+		}
+	})
+}
 func TestLogValue(t *testing.T) {
 	want := "replaced"
 	r := &replace{StringValue(want)}

slog/value_unsafe.go

@@ -10,7 +10,6 @@ package slog
 
 import (
 	"reflect"
-	"time"
 	"unsafe"
 )
 
@@ -45,12 +44,14 @@ func (v Value) Kind() Kind {
 		return x
 	case stringptr:
 		return StringKind
-	case *time.Location:
+	case timeLocation:
 		return TimeKind
 	case groupptr:
 		return GroupKind
 	case LogValuer:
 		return LogValuerKind
+	case kind: // a kind is just a wrapper for a Kind
+		return AnyKind
 	default:
 		return AnyKind
 	}