Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/types: confusing behaviour of Selection.Indirect() for Kind()==MethodVal #8353

Open
adonovan opened this issue Jul 10, 2014 · 5 comments

Comments

Projects
None yet
4 participants
@adonovan
Copy link

commented Jul 10, 2014

In go/types/selection.go, the intended significance of the Indirect flag (for MethodVal)
is hard to tell from the example.  I was assuming that is means whether there are any
pointer indirections between the type of the receiver and the declared type of the
method, but that doesn't seem to explain it:

I instrumented recordSelection:

% cat sel.go 

package main

type T struct {}

func (T)f() {}
func (*T)g() {}

func main() {
        var x T
        x.f()
        x.g()

        var p *T
        p.f()
        p.g()
}

% ./ssadump sel.go
sel.go:11:2: SEL method (main.T) f() indirect=false
sel.go:12:2: SEL method (main.T) g() indirect=false
sel.go:15:2: SEL method (*main.T) f() indirect=true
sel.go:16:2: SEL method (*main.T) g() indirect=true

In the last selection, there is no actual indirection between the receiver type *T and
the method type *T, yet the indirect flag is reported as true.  (The indirect flag seems
to record only the pointerness of the receiver, which is redundant information.)

I think, by analogy with field selections, the indirect bit should be set iff there was
an implicit pointer indirection between the actual receiver type and the formal receiver
type, e.g a (T) method was called with an expression of type (*T), or in this example:

   type A struct {}
   func (*A) f() {}
   type B struct {*A}

   ... B{}.f()   // indirect, since method (*A).f 

An (A) method is called with an expression of type B.
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2014

Comment 1:

Labels changed: added repo-tools, release-go1.4.

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2014

Comment 2:

Status changed to Accepted.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2014

Comment 3:

go/types is not a released package. Unless this is affecting a released binary (namely
godoc), this is not for any release.

Labels changed: added release-none, removed release-go1.4.

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2014

Comment 4:

It does not affect godoc. It's fine to delay this.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015

@rsc rsc removed release-none labels Apr 10, 2015

@rsc rsc changed the title go/types: confusing behaviour of Selection.Indirect() for Kind()==MethodVal x/tools/go/types: confusing behaviour of Selection.Indirect() for Kind()==MethodVal Apr 14, 2015

@rsc rsc modified the milestones: Unreleased, Unplanned Apr 14, 2015

@rsc rsc removed the repo-tools label Apr 14, 2015

@griesemer griesemer changed the title x/tools/go/types: confusing behaviour of Selection.Indirect() for Kind()==MethodVal go/types: confusing behaviour of Selection.Indirect() for Kind()==MethodVal Jul 31, 2015

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2017

I'm not sure we can change this at this point. @adonovan please comment on this issue, or close otherwise. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.