Go: proposal: reflect: add Type.IsEmptyInterface

Created on 10 Oct 2020  路  9Comments  路  Source: golang/go

Coming from #41882, @ianlancetaylor suggests adding:

func IsEmptyInterface(t reflect.Type) bool

to reflect package to detect empty interface.

Currently, users (including standard libraries, Google internal code, are relying on reflect.Type.NumMethod() == 0 to check empty interface, which is wrong. After #22075 is fixed, code which uses wrong behavior should be changed to:

reflect.TypeOf((*interface{})(nil)).Elem().Implements(T)

Adding reflect.IsEmptyInterface will help user's migration easier.

Proposal release-blocker

Most helpful comment

A few thoughts:

  1. Adding a new function to package reflect makes sense for long-term stable state, but users during transition who want to support both Go 1.15 and Go 1.16 will need a solution outside of the standard library. The solution is simple enough to just copy/paste into their own code if necessary. But it might be nice to save them this hassle.

  2. Do we declare it as a standalone function or as a method? (I don't immediately see any significant technical arguments either way, except that it can only be a method if declared in package reflect.)

  3. I recommend IsEmptyInterface(T) return false if T is not an interface type. The function has to check this anyway, and it seemed like a handful of users were checking t.Kind() == reflect.Interface && t.NumMethod() == 0 together. So it's slightly nicer to them (and marginally more efficient) if they can just write IsEmptyInterface(t).

All 9 comments

See discussion in #41882.

A few thoughts:

  1. Adding a new function to package reflect makes sense for long-term stable state, but users during transition who want to support both Go 1.15 and Go 1.16 will need a solution outside of the standard library. The solution is simple enough to just copy/paste into their own code if necessary. But it might be nice to save them this hassle.

  2. Do we declare it as a standalone function or as a method? (I don't immediately see any significant technical arguments either way, except that it can only be a method if declared in package reflect.)

  3. I recommend IsEmptyInterface(T) return false if T is not an interface type. The function has to check this anyway, and it seemed like a handful of users were checking t.Kind() == reflect.Interface && t.NumMethod() == 0 together. So it's slightly nicer to them (and marginally more efficient) if they can just write IsEmptyInterface(t).

Change https://golang.org/cl/263078 mentions this issue: internal/reflectutil: add helper function to detect empty interface

See also #42099.

We are probably going to roll back that change - see #42123 - so it looks like we don't need this.

We are probably going to roll back that change - see #42123 - so it looks like we don't need this.

I still think we should add this method, to provide a better/correct way for user to detect empty interface.

It's certainly easy to go overboard on helpers.
We've come a long way without this one, and NumMethod() == 0 is both clear and correct.

Since we rolled back that change, this seems like a likely decline.

No change in consensus, so declined.

Was this page helpful?
0 / 5 - 0 ratings