all: replace x.Type().Kind() to x.Kind()

[gazerro]

If x is a reflect.Value value and it is not the zero Value, x.Kind() can be used instead of x.Type().Kind().

There are many places in the standard library where this replacement can be done

% grep -r ".Type().Kind()" src
src/cmd/fix/cftype.go:          if v.Type().Kind() != reflect.Ptr {
src/cmd/fix/cftype.go:          if v.Type().Kind() != reflect.Struct {
src/net/http/transport.go:  if rv := reflect.ValueOf(altProto["https"]); rv.IsValid() && rv.Type().Kind() == reflect.Struct && rv.Type().NumField() == 1 {
src/reflect/value.go:   if v.Type().Kind() == Float32 && t.Kind() == Float32 {
src/internal/fmtsort/sort.go:   if mapValue.Type().Kind() != reflect.Map {
src/encoding/xml/read.go:   if val.Type().Kind() == reflect.Slice && val.Type().Elem().Kind() != reflect.Uint8 {
src/encoding/binary/binary.go:      switch v.Type().Kind() {
src/encoding/binary/binary.go:      switch v.Type().Kind() {
src/encoding/binary/binary.go:      switch v.Type().Kind() {
src/encoding/binary/binary.go:      switch v.Type().Kind() {
src/encoding/gob/encode.go: if ut.externalEnc == 0 && value.Type().Kind() == reflect.Struct {
src/encoding/gob/decoder.go:    if value.Type().Kind() != reflect.Ptr {
src/time/tzdata_test.go:    switch f1.Type().Kind() {
src/time/tzdata_test.go:        t.Errorf("test internal error: unsupported kind %v", f1.Type().Kind())

Replacing occurrences in the encoding/binary package results in improvements in the WriteStruct benchmark

name                      old time/op    new time/op    delta
ReadStruct-4                 412ns ± 3%     415ns ± 7%     ~     (p=0.548 n=5+5)
WriteStruct-4                404ns ± 1%     325ns ± 0%  -19.43%  (p=0.008 n=5+5)

[davecheney]

@gazerro if you would like to send a change I recommend working package by package to avoid review delays. Also please be aware that the code freeze is rapidly approaching at the end of the month. If you want to address this, please do not delay.

[ianlancetaylor]

Actually the code freeze already happened. This would have to be for 1.18.

[davecheney]

Welp, there you go.

[gazerro]

@davecheney yes, I will send a CL for every single package.

[bcmills]

Replacing occurrences in the encoding/binary package results in improvements in the WriteStruct benchmark

That sounds to me like a missed optimization in the compiler — there doesn't seem to be any semantic reason why the two calls should differ, and even if we update all of the x.Type().Kind() calls in the standard library there are likely plenty of similar calls in third-party packages.

[gazerro]

The two calls differ if x is the zero Value, so the compiler may not always be able to do this optimization because it has to check the value of x at runtime.

[bcmills]

Seems like in most (all?) of these cases in the standard library, the compiler should already know that the value of x is nonzero — either because the value passed to reflect.ValueOf is invariantly non-nil,¹ or because the reflect.Value is already known to be nonzero (due to a previous call or explicit check).

[gazerro]

Yes, I think so. Optimization would be great. But in the standard library it would still be worth using one or the other consistently unless a specific behavior is desired.

For example the in the file src/internal/fmtsort/sort.go, the code

func Sort(mapValue reflect.Value) *SortedMap {
    if mapValue.Type().Kind() != reflect.Map {
        return nil
    }
        ...
}

for me it is not clear if the use of value.Type().Kind() is intensional, to panic if value is Invalid, or not. If x.Kind() were preferred to x.Type().Kind(), I would interpret it as intentional.

In the file src/net/http/transport.go, this code:

if rv := reflect.ValueOf(altProto["https"]); rv.IsValid() && rv.Type().Kind() == reflect.Struct && rv.Type().NumField() == 1 {

can be written as

if rv := reflect.ValueOf(altProto["https"]); rv.Kind() == reflect.Struct && rv.Type().NumField() == 1 {

even if the compiler can optimize it.

[fraenkel]

If you look at the code, all the cost is in computing the rv.Type() but both Kind()s are dirt cheap.

[mknyszek]

Moving this to the backlog, since it's already been pushed back once.

[gopherbot]

Change https://go.dev/cl/632635 mentions this issue: all: replace reflect.Value.Type.Kind with reflect.Value.Kind