Skip to content

Commit

Permalink
src/log/slog: disallow == on Values
Browse files Browse the repository at this point in the history
Comparing two Values with == is sensitive to the internal
representation of Values, and may not correspond to
equality on the Go values they represent. For example,

    StringValue("X") != StringValue(strings.ToUpper("x"))

because Go ends up doing a pointer comparison on the data
stored in the Values.

So make Values non-comparable by adding a non-comparable field.

Updates golang#56345.

Change-Id: Ieedbf454e631cda10bc6fcf470b57d3f1d2182cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/479516
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
  • Loading branch information
jba authored and eric committed Sep 7, 2023
1 parent 0eb9e23 commit 8be4adf
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 2 deletions.
4 changes: 4 additions & 0 deletions src/log/slog/attr.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,7 @@ func (a Attr) Equal(b Attr) bool {
func (a Attr) String() string {
return fmt.Sprintf("%s=%s", a.Key, a.Value)
}

func (a Attr) isEmpty() bool {
return a.Key == "" && a.Value.num == 0 && a.Value.any == nil
}
2 changes: 1 addition & 1 deletion src/log/slog/record.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func (r *Record) AddAttrs(attrs ...Attr) {
// and seeing if the Attr there is non-zero.
if cap(r.back) > len(r.back) {
end := r.back[:len(r.back)+1][len(r.back)]
if end != (Attr{}) {
if !end.isEmpty() {
panic("copies of a slog.Record were both modified")
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/log/slog/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
// it can represent most small values without an allocation.
// The zero Value corresponds to nil.
type Value struct {
_ [0]func() // disallow ==
// 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
Expand Down Expand Up @@ -371,7 +372,7 @@ func (v Value) group() []Attr {

//////////////// Other

// Equal reports whether v and w have equal keys and values.
// Equal reports whether v and w represent the same Go value.
func (v Value) Equal(w Value) bool {
k1 := v.Kind()
k2 := w.Kind()
Expand Down

0 comments on commit 8be4adf

Please sign in to comment.