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>

5 files changed, 23 insertions, 26 deletions

diff --git a/src/cmd/trace/gen.go b/src/cmd/trace/gen.go
index 4455f83046..9cc22df1f6 100644
--- 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 21f1569f43..f31412e35d 100644
--- 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 f5e8fe79f2..46890e784d 100644
--- 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 3de1e1d4bd..5edcf3a5b2 100644
--- 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 bf396b6a9e..fc2808e597 100644
--- 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}"
-}