The result is different for method foo after adding an exported method Baz

package main

import (
    "fmt"
    "reflect"
)

type Foo struct{}

func (f *Foo) foo(arg struct{}) {}
func (f *Foo) bar()             {}
func (f *Foo) Baz(arg struct{}) {}

func main() {
    f := new(Foo)
    t := reflect.TypeOf(f)
    nMethods := t.NumMethod()
    for i := 0; i < nMethods; i++ {
        method := t.Method(i)
        fmt.Printf("%s %v %v\n", method.Name, method.Type, method.Func)
    }
}

outputs

Baz unc(*main.Foo, struct {}) 0x401000
bar unc(*main.Foo) 0x401000
foo unc(*main.Foo, struct {}) 0x401000

There was a deliberate change here, to remove some information for unexported methods. (Ideally unexported methods would never be seen in the reflect package.) See https://golang.org/cl/21033. However your examples have uncovered at least one bug, maybe two, so I'll dig into this today.

CL https://golang.org/cl/23103 mentions this issue.

@crawshaw, at tip this program now prints:

bar func(*main.Foo) 0x2000
foo <nil> <invalid reflect.Value>

When we talked about dropping this info I was assuming that NumMethod would return a smaller number, so that the unexported methods are completely invisible in the reflect info. Right now they are half-visible, which is kind of the worst of both worlds. Is it possible to make NumMethod and Method(i) only access exported methods while keeping the necessary unexported state around for interface satisfaction and reflect.Type.Implements?

The easy and safe way to do that is to filter in the reflect package, which is now possible. I'm out today, but will send a CL tomorrow.

Thanks.

CL https://golang.org/cl/23253 mentions this issue.

go version devel +1f8d276
The first example program now prints

bar func(*main.Foo) 0x401000

Is it expected? The unexported method bar is not hidden.

By adding the following method

func (f *Foo) Baz(struct{})     {}

it prints all methods

Baz func(*main.Foo, struct {}) 0x401000
bar func(*main.Foo) 0x401000
foo func(*main.Foo, struct {}) 0x401000

That's right, and I think that's as good as we can get for 1.7.

If I were starting from scratch, I don't think I would surface unexported methods via reflect ever. However they are there in Go 1, and have one valid use: determining interface satisfaction when using reflect inside a package that declares an interface with an unexported method.

So I believe we should keep that case working, which means surfacing any method that might meet that requirement. Unfortunately we do not have perfect information about that yet, so we are conservatively removing some methods we know cannot satisfy an interface. The change in https://golang.org/cl/21087, which won't make it for 1.7 gets even closer, and removes your bar example. Hopefully in 1.8 I can modify it to remove even more methods.

reflect.Type.Method output now exposes some unexported methods some of the time. (See #15824)

Either reflect.Type.Method should expose all unexported methods, or it should expose none of them. Anything else is just too confusing. What reflect.Type.Method returns should be a distinct question from reflect.Type.Implements's behavior--the former can elide unexported methods while the latter considers them.

I'm curious how changing the output of reflect.Type.Method is consistent with the compatibility guarantee.

@crawshaw

In #15673, @rsc said the following:

When we talked about dropping this info I was assuming that NumMethod would return a smaller number, so that the unexported methods are completely invisible in the reflect info. Right now they are half-visible, which is kind of the worst of both worlds. Is it possible to make NumMethod and Method(i) only access exported methods while keeping the necessary unexported state around for interface satisfaction and reflect.Type.Implements?

Several questions:
1.) Although I personally agree with removing unexported methods from what the reflect package reports, isn't this a violation of the compatibility agreement? Yes, it wasn't particularly well documented which methods Method would report, but I'm certain some people depend on the ability to get access to unexported methods. Where is the discussion regarding the removal?
2.) From Russ' comment, he mentions that "unexported methods are [to be] completely invisible in the reflect info". Currently, that is not the case since there are some situations where the unexported methods are visible (which means it is not completely invisible). Furthermore, the fact that a line regarding some seemingly unrelated type causes the unexported method to be visible seems to me to be very strange and surprising behavior.
3.) From Russ' comment, he mentions that "keeping the necessary unexported state around for interface satisfaction and reflect.Type.Implements". Doesn't that mean that reflect.Type.Implements should still be working because it has information about the unexported methods, but not that Method or NumMethod should be reporting them?

I don't see this as covered by the compatibility agreement because unexported method reflect data is of questionable value (and it was entirely undocumented and indeed a surprise to me when I found it).

As to your third point and Damien's, I'd be willing to accept that and remove all the method info for unexported methods from the API. I too view the current state as confusing, but it's not completely broken. We could finish the removal in 1.8.

The change to occasionally remove unexported methods from reflect.Type.Method breaks at least one test here. It's probably not a very good test and I'm fine with fixing it, but it does demonstrate that this is a backwards incompatible change that affects some users. I agree that they seem of questionable value.

I do feel quite strongly that if they are to be dropped from reflect.Type.Method, they should be dropped entirely and the behavior documented.

I also feel strongly, that if we alter the behavior, it should be all or none, and that we shouldn't delay that until 1.8. That is, 1.7 should not be in this weird partial state it currently is in.

I agree with Joe, if reflect used to return unexported methods reliably (did it?), I think we should preserve that behavior. People could be depending on it. Returning only exported methods might have to wait until Go 2.

If you ever try to call such a method, it fails with an unexported field error. So I don' think we need the actual implementation of that function to survive in the binary, just enough info to construct a reflect.Method for the unexported function (pkg, name, type, and a func stub that is the canonical "unexported" func).

It did, but I dispute that someone would depend on it in a useful way, beyond rigid unit tests. And keeping them means keeping their type information, which means following those types and keeping everything attached to them.

I'll cut a CL to remove them all.

CL https://golang.org/cl/23410 mentions this issue.

CL https://golang.org/cl/23430 mentions this issue.