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

Jump to bottom

spec: document nil checks for value methods #32035

Open
dotaheor opened this issue May 14, 2019 · 10 comments
Open

spec: document nil checks for value methods #32035

dotaheor opened this issue May 14, 2019 · 10 comments
Assignees
@griesemer
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@dotaheor
Copy link

dotaheor commented May 14, 2019 •
edited

What version of Go are you using (go version)?

go version go1.12 linux/amd64

What did you do?

package main

import (
	"fmt"
)


type S struct {}

func (S) M(n int) {
	fmt.Println(n)
}

type T struct {
	*S
}

type I interface {
	M(n int)
}


func foo() {
	var v = &T{}
	var i I = v
	f := i.M // ok
	v.S = &S{}
	f(111) // 111
}

func bar() {
	var v = &T{}
	f := v.M // panic
	v.S = &S{}
	f(111)
}

func main() {
	foo()
	bar()
}

What did you expect to see?

Same behavior

What did you see instead?

Different behaviors.
@dotaheor
Copy link
Author

This is another bug I found in playing with this one #32021, which is closed. But I think there is also an inconsistency between reflection and direct code.

@dotaheor

This comment has been minimized.

Sign in to view
@dotaheor

This comment has been minimized.

Sign in to view
@FiloSottile FiloSottile added this to the Unplanned milestone May 14, 2019
@FiloSottile FiloSottile added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 14, 2019
@FiloSottile
Copy link
Contributor

/cc @ianlancetaylor

@ianlancetaylor
Copy link
Contributor

In the case f := v.M we check for a nil pointer at the point where we create the method value. We do this because checking for the nil pointer when the method value is invoked leads to a confusing backtrace. In the case f := i.M we check that i is not nil, but we do not check that the value itself contains a nil pointer.

So this is working as expected, in order to avoid a nil dereference in a generated wrapper function, but, you're right, it's not completely consistent.

@ianlancetaylor ianlancetaylor changed the title runtime: inconsistency between evaluating interface method and promoted method cmd/compile: inconsistency between evaluating interface method and promoted method May 14, 2019
@beoran
Copy link

beoran commented May 15, 2019

I would say this inconsistency needs to be documented better since it is surprising people who use Go. Please make this a documentation issue.

@dotaheor
Copy link
Author

In my opinion, the inconsistency in #32021 is more serious than the one is the current issue. I think, if it is possible, reflection code implementation and direct code implementation should be consistent with each other.

Personally, I prefer the current reflection implementation way.

@ianlancetaylor
Copy link
Contributor

If we document something, it would be a change to the language spec saying that you can not create a method value of a value method of a nil pointer, and you can not create a method value of a promoted method of an embedded pointer if the pointer is nil. To put it another way, all nil checks are done when the method value is created, not when it is invoked.

For interface types, you can not create a value method if the interface value is nil. However, we do not check anything about the value stored in the interface value, and I'm not sure that we can.

@ianlancetaylor ianlancetaylor changed the title cmd/compile: inconsistency between evaluating interface method and promoted method spec: document nil checks for value methods May 15, 2019
@ianlancetaylor
Copy link
Contributor

CC @griesemer

@griesemer griesemer self-assigned this May 15, 2019
@griesemer griesemer modified the milestones: Unplanned, Go1.13 May 15, 2019
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@dotaheor dotaheor mentioned this issue Jun 16, 2020
Closed
@go101 go101 mentioned this issue Aug 24, 2021
Closed
@cuonglm
Copy link
Member

cuonglm commented Aug 24, 2021 •
edited

I think this is clear specify in the spec for this behavior:

As with selectors, a reference to a non-interface method with a value receiver using a pointer will automatically dereference that pointer: pt.Mv is equivalent to (*pt).Mv.

As with method calls, a reference to a non-interface method with a pointer receiver using an addressable value will automatically take the address of that value: t.Mp is equivalent to (&t).Mp.

Also #47863 added example for how receiver is evaluated, can we close this issue now? cc @griesemer @mdempsky @ianlancetaylor

@go101 go101 mentioned this issue Mar 30, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@griesemer griesemer

Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
9 participants
@beoran @rsc @andybons @FiloSottile @ianlancetaylor @cuonglm @griesemer @gopherbot @dotaheor