From 2d1bfcb75a025c90db26aa61f6e65d39b8976f50 Mon Sep 17 00:00:00 2001 From: Jonathan Amsterdam Date: Tue, 21 Mar 2023 09:12:30 -0400 Subject: [PATCH] log/slog: rename and remove files - Remove the norace_test.go files, moving their contents elsewhere. - Rename the internal/testutil package to internal/slogtest. - Remove value_unsafe.go, moving its contents to value.go. Updates golang/go#56345. Change-Id: I2a24ace5aea47f7a3067cd671f606c4fb279d744 Reviewed-on: https://go-review.googlesource.com/c/go/+/478197 Run-TryBot: Jonathan Amsterdam TryBot-Result: Gopher Robot Reviewed-by: Ian Lance Taylor --- src/go/build/deps_test.go | 2 +- src/log/slog/example_level_handler_test.go | 4 +- src/log/slog/example_logvaluer_secret_test.go | 4 +- src/log/slog/example_test.go | 4 +- src/log/slog/internal/buffer/buffer_test.go | 21 ++++- src/log/slog/internal/buffer/norace_test.go | 26 ------ .../testutil.go => slogtest/slogtest.go} | 4 +- src/log/slog/logger_test.go | 14 +++ src/log/slog/norace_test.go | 23 ----- src/log/slog/value.go | 81 +++++++++++++++-- src/log/slog/value_unsafe.go | 90 ------------------- 11 files changed, 118 insertions(+), 155 deletions(-) delete mode 100644 src/log/slog/internal/buffer/norace_test.go rename src/log/slog/internal/{testutil/testutil.go => slogtest/slogtest.go} (84%) delete mode 100644 src/log/slog/norace_test.go delete mode 100644 src/log/slog/value_unsafe.go diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index c9a66c7441a42..e28f837fa1c0f 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -386,7 +386,7 @@ var depsRules = ` log/slog/internal, log/slog/internal/buffer, slices < log/slog - < log/slog/internal/testutil; + < log/slog/internal/slogtest; NET, log < net/mail; diff --git a/src/log/slog/example_level_handler_test.go b/src/log/slog/example_level_handler_test.go index 9e0eb764bf182..9ddeab336967d 100644 --- a/src/log/slog/example_level_handler_test.go +++ b/src/log/slog/example_level_handler_test.go @@ -7,7 +7,7 @@ package slog_test import ( "context" "log/slog" - "log/slog/internal/testutil" + "log/slog/internal/slogtest" "os" ) @@ -63,7 +63,7 @@ func (h *LevelHandler) Handler() slog.Handler { // Another typical use would be to decrease the log level (to LevelDebug, say) // during a part of the program that was suspected of containing a bug. func ExampleHandler_levelHandler() { - th := slog.HandlerOptions{ReplaceAttr: testutil.RemoveTime}.NewTextHandler(os.Stdout) + th := slog.HandlerOptions{ReplaceAttr: slogtest.RemoveTime}.NewTextHandler(os.Stdout) logger := slog.New(NewLevelHandler(slog.LevelWarn, th)) logger.Info("not printed") logger.Warn("printed") diff --git a/src/log/slog/example_logvaluer_secret_test.go b/src/log/slog/example_logvaluer_secret_test.go index 05f32a54a20ac..efc22a20e3f59 100644 --- a/src/log/slog/example_logvaluer_secret_test.go +++ b/src/log/slog/example_logvaluer_secret_test.go @@ -6,7 +6,7 @@ package slog_test import ( "log/slog" - "log/slog/internal/testutil" + "log/slog/internal/slogtest" "os" ) @@ -23,7 +23,7 @@ func (Token) LogValue() slog.Value { // with an alternative representation to avoid revealing secrets. func ExampleLogValuer_secret() { t := Token("shhhh!") - logger := slog.New(slog.HandlerOptions{ReplaceAttr: testutil.RemoveTime}. + logger := slog.New(slog.HandlerOptions{ReplaceAttr: slogtest.RemoveTime}. NewTextHandler(os.Stdout)) logger.Info("permission granted", "user", "Perry", "token", t) diff --git a/src/log/slog/example_test.go b/src/log/slog/example_test.go index 709d7a922cc20..06a275064861c 100644 --- a/src/log/slog/example_test.go +++ b/src/log/slog/example_test.go @@ -6,7 +6,7 @@ package slog_test import ( "log/slog" - "log/slog/internal/testutil" + "log/slog/internal/slogtest" "net/http" "os" "time" @@ -16,7 +16,7 @@ func ExampleGroup() { r, _ := http.NewRequest("GET", "localhost", nil) // ... - logger := slog.New(slog.HandlerOptions{ReplaceAttr: testutil.RemoveTime}.NewTextHandler(os.Stdout)) + logger := slog.New(slog.HandlerOptions{ReplaceAttr: slogtest.RemoveTime}.NewTextHandler(os.Stdout)) slog.SetDefault(logger) slog.Info("finished", diff --git a/src/log/slog/internal/buffer/buffer_test.go b/src/log/slog/internal/buffer/buffer_test.go index 323d4112f0abd..40b1d1fda8aa9 100644 --- a/src/log/slog/internal/buffer/buffer_test.go +++ b/src/log/slog/internal/buffer/buffer_test.go @@ -4,7 +4,11 @@ package buffer -import "testing" +import ( + "internal/race" + "internal/testenv" + "testing" +) func Test(t *testing.T) { b := New() @@ -20,3 +24,18 @@ func Test(t *testing.T) { t.Errorf("got %q, want %q", got, want) } } + +func TestAlloc(t *testing.T) { + if race.Enabled { + t.Skip("skipping test in race mode") + } + testenv.SkipIfOptimizationOff(t) + got := int(testing.AllocsPerRun(5, func() { + b := New() + defer b.Free() + b.WriteString("not 1K worth of bytes") + })) + if got != 0 { + t.Errorf("got %d allocs, want 0", got) + } +} diff --git a/src/log/slog/internal/buffer/norace_test.go b/src/log/slog/internal/buffer/norace_test.go deleted file mode 100644 index 226965b5b86fa..0000000000000 --- a/src/log/slog/internal/buffer/norace_test.go +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2022 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package buffer - -import ( - "internal/race" - "internal/testenv" - "testing" -) - -func TestAlloc(t *testing.T) { - if race.Enabled { - t.Skip("skipping test in race mode") - } - testenv.SkipIfOptimizationOff(t) - got := int(testing.AllocsPerRun(5, func() { - b := New() - defer b.Free() - b.WriteString("not 1K worth of bytes") - })) - if got != 0 { - t.Errorf("got %d allocs, want 0", got) - } -} diff --git a/src/log/slog/internal/testutil/testutil.go b/src/log/slog/internal/slogtest/slogtest.go similarity index 84% rename from src/log/slog/internal/testutil/testutil.go rename to src/log/slog/internal/slogtest/slogtest.go index 6b9a36dead484..f44e6b5f893ab 100644 --- a/src/log/slog/internal/testutil/testutil.go +++ b/src/log/slog/internal/slogtest/slogtest.go @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// Package testutil contains support functions for testing. -package testutil +// Package slogtest contains support functions for testing slog. +package slogtest import "log/slog" diff --git a/src/log/slog/logger_test.go b/src/log/slog/logger_test.go index 1235caa69a460..e65071424f89f 100644 --- a/src/log/slog/logger_test.go +++ b/src/log/slog/logger_test.go @@ -7,6 +7,8 @@ package slog import ( "bytes" "context" + "internal/race" + "internal/testenv" "io" "log" "path/filepath" @@ -509,3 +511,15 @@ func callerPC(depth int) uintptr { runtime.Callers(depth, pcs[:]) return pcs[0] } + +func wantAllocs(t *testing.T, want int, f func()) { + if race.Enabled { + t.Skip("skipping test in race mode") + } + testenv.SkipIfOptimizationOff(t) + t.Helper() + got := int(testing.AllocsPerRun(5, f)) + if got != want { + t.Errorf("got %d allocs, want %d", got, want) + } +} diff --git a/src/log/slog/norace_test.go b/src/log/slog/norace_test.go deleted file mode 100644 index 3dbee0cb7bd39..0000000000000 --- a/src/log/slog/norace_test.go +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright 2022 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package slog - -import ( - "internal/race" - "internal/testenv" - "testing" -) - -func wantAllocs(t *testing.T, want int, f func()) { - if race.Enabled { - t.Skip("skipping test in race mode") - } - testenv.SkipIfOptimizationOff(t) - t.Helper() - got := int(testing.AllocsPerRun(5, f)) - if got != want { - t.Errorf("got %d allocs, want %d", got, want) - } -} diff --git a/src/log/slog/value.go b/src/log/slog/value.go index de6f22d600571..3a2c41e9e46d9 100644 --- a/src/log/slog/value.go +++ b/src/log/slog/value.go @@ -10,10 +10,32 @@ import ( "slices" "strconv" "time" + "unsafe" ) -// Definitions for Value. -// The Value type itself can be found in value_{safe,unsafe}.go. +// A Value can represent any Go value, but unlike type any, +// it can represent most small values without an allocation. +// The zero Value corresponds to nil. +type Value struct { + // num holds the value for Kinds Int64, Uint64, Float64, Bool and Duration, + // the string length for KindString, and nanoseconds since the epoch for KindTime. + num uint64 + // If any is of type Kind, then the value is in num as described above. + // If any is of type *time.Location, then the Kind is Time and time.Time value + // can be constructed from the Unix nanos in num and the location (monotonic time + // is not preserved). + // If any is of type stringptr, then the Kind is String and the string value + // consists of the length in num and the pointer in any. + // Otherwise, the Kind is Any and any is the value. + // (This implies that Attrs cannot store values of type Kind, *time.Location + // or stringptr.) + any any +} + +type ( + stringptr *byte // used in Value.any when the Value is a string + groupptr *Attr // used in Value.any when the Value is a []Attr +) // Kind is the kind of a Value. type Kind int @@ -58,8 +80,33 @@ func (k Kind) String() string { // (No user-provided value has this type.) type kind Kind +// Kind returns v's Kind. +func (v Value) Kind() Kind { + switch x := v.any.(type) { + case Kind: + return x + case stringptr: + return KindString + case timeLocation: + return KindTime + case groupptr: + return KindGroup + case LogValuer: + return KindLogValuer + case kind: // a kind is just a wrapper for a Kind + return KindAny + default: + return KindAny + } +} + //////////////// Constructors +// StringValue returns a new Value for a string. +func StringValue(value string) Value { + return Value{num: uint64(len(value)), any: stringptr(unsafe.StringData(value))} +} + // IntValue returns a Value for an int. func IntValue(v int) Value { return Int64Value(int64(v)) @@ -114,7 +161,7 @@ func DurationValue(v time.Duration) Value { // GroupValue returns a new Value for a list of Attrs. // The caller must not subsequently mutate the argument slice. func GroupValue(as ...Attr) Value { - return groupValue(as) + return Value{num: uint64(len(as)), any: groupptr(unsafe.SliceData(as))} } // AnyValue returns a Value for the supplied value. @@ -192,7 +239,7 @@ func (v Value) Any() any { case KindLogValuer: return v.any case KindGroup: - return v.uncheckedGroup() + return v.group() case KindInt64: return int64(v.num) case KindUint64: @@ -212,6 +259,21 @@ func (v Value) Any() any { } } +// String returns Value's value as a string, formatted like fmt.Sprint. Unlike +// the methods Int64, Float64, and so on, which panic if v is of the +// wrong kind, String never panics. +func (v Value) String() string { + if sp, ok := v.any.(stringptr); ok { + return unsafe.String(sp, v.num) + } + var buf []byte + return string(v.append(buf)) +} + +func (v Value) str() string { + return unsafe.String(v.any.(stringptr), v.num) +} + // Int64 returns v's value as an int64. It panics // if v is not a signed integer. func (v Value) Int64() int64 { @@ -297,7 +359,14 @@ func (v Value) LogValuer() LogValuer { // Group returns v's value as a []Attr. // It panics if v's Kind is not KindGroup. func (v Value) Group() []Attr { - return v.group() + if sp, ok := v.any.(groupptr); ok { + return unsafe.Slice((*Attr)(sp), v.num) + } + panic("Group: bad kind") +} + +func (v Value) group() []Attr { + return unsafe.Slice((*Attr)(v.any.(groupptr)), v.num) } //////////////// Other @@ -321,7 +390,7 @@ func (v Value) Equal(w Value) bool { case KindAny, KindLogValuer: return v.any == w.any // may panic if non-comparable case KindGroup: - return slices.EqualFunc(v.uncheckedGroup(), w.uncheckedGroup(), Attr.Equal) + return slices.EqualFunc(v.group(), w.group(), Attr.Equal) default: panic(fmt.Sprintf("bad kind: %s", k1)) } diff --git a/src/log/slog/value_unsafe.go b/src/log/slog/value_unsafe.go deleted file mode 100644 index 4008ca51984a8..0000000000000 --- a/src/log/slog/value_unsafe.go +++ /dev/null @@ -1,90 +0,0 @@ -// Copyright 2022 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package slog - -import ( - "unsafe" -) - -// A Value can represent any Go value, but unlike type any, -// it can represent most small values without an allocation. -// The zero Value corresponds to nil. -type Value struct { - // num holds the value for Kinds Int64, Uint64, Float64, Bool and Duration, - // the string length for KindString, and nanoseconds since the epoch for KindTime. - num uint64 - // If any is of type Kind, then the value is in num as described above. - // If any is of type *time.Location, then the Kind is Time and time.Time value - // can be constructed from the Unix nanos in num and the location (monotonic time - // is not preserved). - // If any is of type stringptr, then the Kind is String and the string value - // consists of the length in num and the pointer in any. - // Otherwise, the Kind is Any and any is the value. - // (This implies that Attrs cannot store values of type Kind, *time.Location - // or stringptr.) - any any -} - -type ( - stringptr *byte // used in Value.any when the Value is a string - groupptr *Attr // used in Value.any when the Value is a []Attr -) - -// Kind returns v's Kind. -func (v Value) Kind() Kind { - switch x := v.any.(type) { - case Kind: - return x - case stringptr: - return KindString - case timeLocation: - return KindTime - case groupptr: - return KindGroup - case LogValuer: - return KindLogValuer - case kind: // a kind is just a wrapper for a Kind - return KindAny - default: - return KindAny - } -} - -// StringValue returns a new Value for a string. -func StringValue(value string) Value { - return Value{num: uint64(len(value)), any: stringptr(unsafe.StringData(value))} -} - -func (v Value) str() string { - return unsafe.String(v.any.(stringptr), v.num) -} - -// String returns Value's value as a string, formatted like fmt.Sprint. Unlike -// the methods Int64, Float64, and so on, which panic if v is of the -// wrong kind, String never panics. -func (v Value) String() string { - if sp, ok := v.any.(stringptr); ok { - return unsafe.String(sp, v.num) - } - var buf []byte - return string(v.append(buf)) -} - -func groupValue(as []Attr) Value { - return Value{num: uint64(len(as)), any: groupptr(unsafe.SliceData(as))} -} - -// group returns the Value's value as a []Attr. -// It panics if the Value's Kind is not KindGroup. -func (v Value) group() []Attr { - if sp, ok := v.any.(groupptr); ok { - return unsafe.Slice((*Attr)(sp), v.num) - } - panic("Group: bad kind") -} - -func (v Value) uncheckedGroup() []Attr { - return unsafe.Slice((*Attr)(v.any.(groupptr)), v.num) -}