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: typeparams: should //go:nointerface methods satisfy type parameter constraints? #47045

Open
mdempsky opened this issue Jul 3, 2021 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@mdempsky
Copy link
Contributor

mdempsky commented Jul 3, 2021

The "fieldtrack" experiment includes a //go:nointerface directive, which says a method shouldn't be considered for interface satisfiability. For example, this package fails to compile with GOEXPERIMENT=fieldtrack:

package p

type T int

//go:nointerface
func (*T) M() {}

type I interface { M() }

var i I = new(T) // ERROR: *T does not implement I, because (*T).M is marked //go:nointerface

What effect should //go:nointerface have on type parameter constraint satisfaction? Should *T satisfy constraint I as a type parameter constraint? How should a compiler handle:

package p

type T int

//go:nointerface
func (*T) M() {}

type I interface { M() }

func F[X I]() {
  var _ I = new(X)
}

var _ = F[T]

Fieldtracking is formally just an experiment, so I don't think it's urgent to support either way (i.e., either to accept and handle correctly if it's allowed, or to consistently reject if it's not allowed). I also think we reserve the right to change our mind on the right behavior here. But it seems worth making sure we're more-or-less on the same page about long-term direction here.

I'm currently inclined to think *T does not satisfy constraint interface { M() } if (*T).M is //go:nointerface. (This is a bit bothersome to implement in cmd/compile, because currently types2 is responsible for constraint satisfaction, but it doesn't know about directives like //go:nointerface; but I don't think it should be that hard to handle.)

/cc @griesemer @ianlancetaylor @findleyr

@mdempsky mdempsky added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jul 3, 2021
@mdempsky mdempsky added this to the Go1.18 milestone Jul 3, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/332611 mentions this issue: [dev.typeparams] cmd/compile: fix unified IR support for //go:nointerface

gopherbot pushed a commit that referenced this issue Jul 26, 2021
…face

This CL changes fixedbugs/issue30862.go into a "runindir" test so that
it can use '-goexperiment fieldtrack' and test that //go:nointerface
works with cmd/compile. In particular, this revealed that -G=3 and
unified IR did not handle it correctly.

This CL also fixes unified IR's support for //go:nointerface and adds
a test that checks that //go:nointerface, promoted methods, and
generics all interact as expected.

Updates #47045.

Change-Id: Ib8acff8ae18bf124520d00c98e8915699cba2abd
Reviewed-on: https://go-review.googlesource.com/c/go/+/332611
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
@ianlancetaylor
Copy link
Contributor

I don't think we know what go:nointerface should do for constraints. Moving to 1.19 milestone.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.18, Go1.19 Jan 28, 2022
@cherrymui
Copy link
Member

Since type parameter constraints are just interfaces, I'd think nointerface methods don't satisfy them, just like they don't satisfy regular interfaces.

@ianlancetaylor
Copy link
Contributor

Moving to Backlog.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.19, Backlog Jun 24, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
Status: Triage Backlog
Status: No status
Development

No branches or pull requests

4 participants