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

cmd/compile: corrupt error message for wrongly implemented generic interface #49579

Closed
rogpeppe opened this issue Nov 14, 2021 · 8 comments
Closed
Labels
generics

Comments

@rogpeppe
Copy link
Contributor

@rogpeppe rogpeppe commented Nov 14, 2021

go version devel go1.18-c2397905e0 Sat Nov 13 03:33:55 2021 +0000 linux/amd64

I ran this code: https://gotipplay.golang.org/p/D0f2z4hHt4W

package main

func main() {
}

type I[F any] interface {
	Q(*F)
}

func G[F any]() I[any] {
	return g[F]{}
}

type g[F any] struct{}

func (*g[F]) Q(*any) {}

It correctly diagnoses that g[F] doesn't implement the required interface (because Q is implemented on the pointer type, not the value type), but the error message that I see looks wrong:

./prog.go:11:9: cannot use g[F]{} (value of type g[F]) as type I[interface{}] in return statement:
	g[F] does not implement I[interface{}] (wrong type for Q method)
		have Q^^%!s(<nil>)
		want Q(*interface{})
@rogpeppe rogpeppe added the generics label Nov 14, 2021
@rogpeppe
Copy link
Contributor Author

@rogpeppe rogpeppe commented Nov 14, 2021

@danscales

@rogpeppe
Copy link
Contributor Author

@rogpeppe rogpeppe commented Nov 14, 2021

Looks like wrongType has a name but not a type in this code:

r = fmt.Sprintf("(wrong type for %s)\n\t\thave %s^^%s\n\t\twant %s^^%s",

leading to this hack failing to remove the ^^ substring:

r = strings.Replace(r, "^^func", "", -1)

@danscales danscales self-assigned this Nov 14, 2021
@danscales
Copy link
Contributor

@danscales danscales commented Nov 14, 2021

OK, thanks, I will take a look and see how to fix it.

@danscales
Copy link
Contributor

@danscales danscales commented Nov 15, 2021

@griesemer I looked at this, and to me it seems like this might be happening because of a types2 bug. As far as I can tell, the types2.Func that represents Q at line 16 (last line) has a object.typ of nil at the point where we're checking the 'return g[F]{}' statement, whereas it seems like it should have a Signature type which is the same as the type for the Q in the interface I. This manifests in going down the wrong conditional in missingMethodReason() and wrongType.typ being nil when we go to print it out.

If you have time, maybe you can check this out (or happy to discuss with you directly), since it may be hiding a more serious bug. Or at least I need to understand better when a Func has a object.typ field that is nil.

@hanchaoqun
Copy link
Contributor

@hanchaoqun hanchaoqun commented Nov 17, 2021

When calling "resolve", always enter the check.later branch (delayed processing), resulting in the "completeMethods" function is not actually called until "processDelayed".
But the processing order of that is relatively late, so the Q method of g[F] finding through “lookupMethodFold” is uncompleted (typ==nil).

func (t *Named) resolve(ctxt *Context) *Named {

check.later(completeMethods)

if i, m := lookupMethodFold(named.methods, pkg, name, checkFold); m != nil {

== initFiles ==

== collectObjects ==

== packageObjects ==
...
crash.go:16:14: -- checking func Q (white, objPath = )
crash.go:16:8:  .  type g
crash.go:16:8:  .  => g[F₃ interface{}] (under = struct{}) // *Named
crash.go:16:7:  .  type *g[F]
crash.go:16:9:  .  .  type g[F]
crash.go:16:8:  .  .  .  -- instantiating g with [%!s(*syntax.Name=&{F {{{0xc000562e70 16 10}}}})]
crash.go:16:8:  .  .  .  .  type g
crash.go:16:8:  .  .  .  .  => g[F₃ interface{}] (under = struct{}) // *Named
crash.go:16:10: .  .  .  .  type F
crash.go:16:10: .  .  .  .  => F₄ (under = interface{}) // *TypeParam
crash.go:16:8:  .  .  .  => g[F₄]
crash.go:16:9:  .  .  => g[F₄] (under = ) // *Named
crash.go:16:7:  .  => *g[F₄] // *Pointer
crash.go:16:16: .  type *any
crash.go:16:17: .  .  type any
crash.go:16:17: .  .  => interface{} // *Interface
crash.go:16:16: .  => *interface{} // *Pointer
crash.go:16:14: => func (*g[F]).Q(*interface{}) (black)  --- A  : push back to check.later's processing list

== processDelayed ==
crash.go:3:13:  --- main: func()
crash.go:4:1:   --- 
crash.go:6:15:  type set for interface{Q(*F₁)}
crash.go:6:15:  => {func (I[F interface{}]).Q(*F)}
crash.go:10:17: type set for interface{Q(*interface{})}
crash.go:10:17: => {func (I[F interface{}]).Q(*interface{})}
crash.go:10:24: --- G: func[F₂ interface{}]() I[interface{}]
crash.go:11:13: expr g[F]{}
crash.go:11:10: .  type g[F]
crash.go:11:9:  .  .  -- instantiating g with [%!s(*syntax.Name=&{F {{{0xc000562e70 11 11}}}})]
crash.go:11:9:  .  .  .  type g
crash.go:11:9:  .  .  .  => g[F₃ interface{}] (under = struct{}) // *Named
crash.go:11:11: .  .  .  type F
crash.go:11:11: .  .  .  => F₂ (under = interface{}) // *TypeParam
crash.go:11:9:  .  .  => g[F₂]
crash.go:11:10: .  => g[F₂] (under = ) // *Named -----  B : A is not processed yet.       
...
...       ----- C :  processing A here.      

If make modification on the following line, when the input parameter ctxt of "expandNamed" is nil (check != nil && !isCtxtNil), don't entering the check.later branch, it can work right

crash.go:11:9:  ERROR: cannot use g[F]{} (value of type g[F]) as type I[interface{}] in return statement:
        g[F] does not implement I[interface{}] (Q method has pointer receiver)

@findleyr
Copy link
Contributor

@findleyr findleyr commented Nov 17, 2021

If make modification on the following line, when the input parameter ctxt of "expandNamed" is nil (check != nil && !isCtxtNil), don't entering the check.later branch, it can work right

That's probably "luck" -- if check is non-nil (i.e. we are in a type-checking pass), it should always be OK to defer actions using check.later. Most likely we're not eagerly expanding in some place where we need to. The clue that ctxt == nil in that case is helpful though. I will take a look at this today.

@findleyr findleyr assigned findleyr and unassigned danscales Nov 17, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Nov 17, 2021

Change https://golang.org/cl/364714 mentions this issue: go/types: complete methods on pointer receivers in missingMethod

@findleyr
Copy link
Contributor

@findleyr findleyr commented Nov 17, 2021

The fix ended up (thankfully) being straightforward: during the type-checking pass methods found in missingMethod go through objDecl, because it's possible that they're not fully set up yet. But the check for methods with a pointer receiver bypassed objDecl. This may not have mattered before, but matters now that we lazily complete methods on instantiated types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generics
Projects
None yet
Development

No branches or pull requests

5 participants