internal/trace: make Value follow reflect conventions

A previous change renamed Value.Uint64 to Value.ToUint64 to accomodate
string values. The method for a string value is then Value.ToString,
while the method for a debug string (for example, for fmt) is just
called String, as per fmt.Stringer.

This change follows a request from Dominik Honnef, maintainer of
gotraceui, to make Value follow the conventions of the reflect package.
The Value type there has a method String which fulfills both purposes:
getting the string for a String Value, and as fmt.Stringer. It's
not exactly pretty, but it does make sense to just stick to convention.

Change-Id: I55b364be88088d2121527bffc833ef03dbdb9764
Reviewed-on: https://go-review.googlesource.com/c/go/+/680978
Reviewed-by: Florian Lehner <lehner.florian86@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>

diff --git a/src/cmd/trace/gen.go b/src/cmd/trace/gen.go
index 4455f830461d4809ec33eaaae4f599bc3be782ad..9cc22df1f68f413c9975037995158e2bf519eb70 100644 (file)
--- a/src/cmd/trace/gen.go
+++ b/src/cmd/trace/gen.go
@@ -283,11 +283,11 @@ func (g *globalMetricGenerator) GlobalMetric(ctx *traceContext, ev *trace.Event)
        m := ev.Metric()
        switch m.Name {
        case "/memory/classes/heap/objects:bytes":
-               ctx.HeapAlloc(ctx.elapsed(ev.Time()), m.Value.ToUint64())
+               ctx.HeapAlloc(ctx.elapsed(ev.Time()), m.Value.Uint64())
        case "/gc/heap/goal:bytes":
-               ctx.HeapGoal(ctx.elapsed(ev.Time()), m.Value.ToUint64())
+               ctx.HeapGoal(ctx.elapsed(ev.Time()), m.Value.Uint64())
        case "/sched/gomaxprocs:threads":
-               ctx.Gomaxprocs(m.Value.ToUint64())
+               ctx.Gomaxprocs(m.Value.Uint64())
        }
 }
 

diff --git a/src/internal/trace/event.go b/src/internal/trace/event.go
index 21f1569f43fde511bf712482cd8f4d9230a81ef0..f31412e35d889afef2c1292b7ca59658d36781c6 100644 (file)
--- a/src/internal/trace/event.go
+++ b/src/internal/trace/event.go
@@ -8,6 +8,7 @@ import (
        "fmt"
        "iter"
        "math"
+       "strconv"
        "strings"
        "time"
 
@@ -812,6 +813,10 @@ func (e Event) String() string {
        switch kind := e.Kind(); kind {
        case EventMetric:
                m := e.Metric()
+               v := m.Value.String()
+               if m.Value.Kind() == ValueString {
+                       v = strconv.Quote(v)
+               }
                fmt.Fprintf(&sb, " Name=%q Value=%s", m.Name, m.Value)
        case EventLabel:
                l := e.Label()

diff --git a/src/internal/trace/gc.go b/src/internal/trace/gc.go
index f5e8fe79f293be13c2640872d01a270325917b53..46890e784df24cf84e7584926c26c031ae0a5e53 100644 (file)
--- a/src/internal/trace/gc.go
+++ b/src/internal/trace/gc.go
@@ -103,7 +103,7 @@ func MutatorUtilizationV2(events []Event, flags UtilFlags) [][]MutatorUtil {
                        if m.Name != "/sched/gomaxprocs:threads" {
                                break
                        }
-                       gomaxprocs := int(m.Value.ToUint64())
+                       gomaxprocs := int(m.Value.Uint64())
                        if len(ps) > gomaxprocs {
                                if flags&UtilPerProc != 0 {
                                        // End each P's series.

diff --git a/src/internal/trace/testtrace/validation.go b/src/internal/trace/testtrace/validation.go
index 3de1e1d4bdf5ed4402c2438a52388bdce5600581..5edcf3a5b2dc0a4e18393a6bb85fb66e7e55e172 100644 (file)
--- a/src/internal/trace/testtrace/validation.go
+++ b/src/internal/trace/testtrace/validation.go
@@ -135,7 +135,7 @@ func (v *Validator) Event(ev trace.Event) error {
                switch m.Value.Kind() {
                case trace.ValueUint64:
                        // Just make sure it doesn't panic.
-                       _ = m.Value.ToUint64()
+                       _ = m.Value.Uint64()
                }
        case trace.EventLabel:
                l := ev.Label()

diff --git a/src/internal/trace/value.go b/src/internal/trace/value.go
index bf396b6a9ee3943ac05e67678546ad5843cebc36..fc2808e59753a82da1a55081b2b45959ecd9d3ff 100644 (file)
--- a/src/internal/trace/value.go
+++ b/src/internal/trace/value.go
@@ -35,24 +35,27 @@ func (v Value) Kind() ValueKind {
        return v.kind
 }
 
-// ToUint64 returns the uint64 value for a ValueUint64.
+// Uint64 returns the uint64 value for a ValueUint64.
 //
 // Panics if this Value's Kind is not ValueUint64.
-func (v Value) ToUint64() uint64 {
+func (v Value) Uint64() uint64 {
        if v.kind != ValueUint64 {
-               panic("ToUint64 called on Value of a different Kind")
+               panic("Uint64 called on Value of a different Kind")
        }
        return v.scalar
 }
 
-// ToString returns the uint64 value for a ValueString.
-//
-// Panics if this Value's Kind is not ValueString.
-func (v Value) ToString() string {
-       if v.kind != ValueString {
-               panic("ToString called on Value of a different Kind")
+// String returns the string value for a ValueString, and otherwise
+// a string representation of the value for other kinds of values.
+func (v Value) String() string {
+       if v.kind == ValueString {
+               return unsafe.String((*byte)(v.pointer), int(v.scalar))
+       }
+       switch v.kind {
+       case ValueUint64:
+               return fmt.Sprintf("Value{Uint64(%d)}", v.Uint64())
        }
-       return unsafe.String((*byte)(v.pointer), int(v.scalar))
+       return "Value{Bad}"
 }
 
 func uint64Value(x uint64) Value {
@@ -62,14 +65,3 @@ func uint64Value(x uint64) Value {
 func stringValue(s string) Value {
        return Value{kind: ValueString, scalar: uint64(len(s)), pointer: unsafe.Pointer(unsafe.StringData(s))}
 }
-
-// String returns the string representation of the value.
-func (v Value) String() string {
-       switch v.Kind() {
-       case ValueUint64:
-               return fmt.Sprintf("Value{Uint64(%d)}", v.ToUint64())
-       case ValueString:
-               return fmt.Sprintf("Value{String(%s)}", v.ToString())
-       }
-       return "Value{Bad}"
-}