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

spec: remove unnecessary syntax restrictions for method expressions to match compiler #9060

Closed
griesemer opened this issue Nov 4, 2014 · 9 comments

Comments

@griesemer
Copy link
Contributor

griesemer commented Nov 4, 2014

At the moment, method expressions must be of the form T.m or (*T).m . There's no good reason not to generalize the notation to T.m where T is any kind of type that has a method m. The
only options are the ones we already have, as well as unnamed interface types and struct
type embedding fields with methods.

These extra cases are largely academic but there's no real reason for the restriction;
it only adds complexity to the spec and requires extra checks in the compiler.

This would be a backward-compatible language change w/o impact to users. It would
simplify the spec. It simplifies the type-checker. It may simplify the rest of the
implementation.

See also issue #8605.
@griesemer griesemer self-assigned this Nov 4, 2014
@bradfitz bradfitz removed the new label Dec 18, 2014
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@griesemer griesemer modified the milestones: Go1.7Maybe, Unplanned May 18, 2016
@griesemer
Copy link
Contributor Author

griesemer commented May 18, 2016

It appears that both cmd/compile and go/types already accept code like this:

package main

func main() {
    (*struct{ error }).Error(nil)
}

Perhaps we just need to relax the spec.

@griesemer
Copy link
Contributor Author

griesemer commented May 19, 2016

This requires more subtle changes in the spec than anticipated - simply removing the grammar in https://golang.org/ref/spec#Method_expressions and replacing MethodExpression with Type in Operand may do it, but then warrant more simplifications (regarding composite literals). Too subtle to squeeze in before 1.7. Still, would be nice to fix since cmd/compile and go/types seem to accept it effortlessly, and it's a clean generalization.

But esoteric and of no real use for now, so no urgency. Postponing again.

@griesemer griesemer modified the milestones: Unplanned, Go1.7Maybe May 19, 2016
@rsc rsc changed the title spec: remove unnecessary syntax restrictions for method expressions proposal: spec: remove unnecessary syntax restrictions for method expressions Jun 20, 2017
@rsc rsc added the Go2 A change that can only be done in Go 2 label Jun 20, 2017
@mdempsky
Copy link
Member

mdempsky commented Oct 19, 2017

It's worth noting that because of type aliases, the formerly syntactically invalid code:

interface{f()}.f(nil)

can now equivalently be written as the syntactically valid:

type u = interface{f()}
u.f(nil)

gccgo rejects the former, but accepts the latter. cmd/compile and go/types accept both.

Notably though, when interface{f()}.f is replaced with (*struct{error}).Error, gccgo still rejects both forms. (cmd/compile and go/types still accept both.)

@griesemer
Copy link
Contributor Author

griesemer commented Oct 25, 2017

Removing Go2 label. go/types and cmd/compile accept these forms (and have been accepting them for some time), so at this point this is really just about updating the spec to match the compilers. For the same reasons it's also not a language change.

@griesemer griesemer removed Go2 A change that can only be done in Go 2 LanguageChange labels Oct 25, 2017
@griesemer griesemer modified the milestones: Unplanned, Go1.10 Oct 25, 2017
@griesemer griesemer changed the title proposal: spec: remove unnecessary syntax restrictions for method expressions spec: remove unnecessary syntax restrictions for method expressions to match compiler Oct 25, 2017
@gopherbot
Copy link

gopherbot commented Oct 25, 2017

Change https://golang.org/cl/73233 mentions this issue: spec: match syntax for method expressions with implementations

@gopherbot
Copy link

gopherbot commented Oct 25, 2017

Change https://golang.org/cl/73554 mentions this issue: test: add test cases for method expressions with literal receiver types

gopherbot pushed a commit that referenced this issue Oct 26, 2017
For #9060.

Change-Id: Ibd0f047083f3c98cec96c655a3e2024df8e1d2a0
Reviewed-on: https://go-review.googlesource.com/73554
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@cznic
Copy link
Contributor

cznic commented Oct 26, 2017

@griesemer I don't know if/how it's related, but when I try the code from #9060 (comment), having the note

It appears that both cmd/compile and go/types already accept code like this:

in the playground I get

go.(*struct { error }).Error: call to external function
main.main: relocation target go.(*struct { error }).Error not defined
main.main: undefined: "go.(*struct { error }).Error"

Tried also locally with go version go1.9.1 linux/amd64 and the result is the same.

@cznic
Copy link
Contributor

cznic commented Oct 26, 2017

@griesemer Update: Nevermind, I found you've already reported this in #22444.

@gopherbot
Copy link

gopherbot commented Dec 4, 2017

Change https://golang.org/cl/81775 mentions this issue: test: disable broken test for 1.10

gopherbot pushed a commit that referenced this issue Dec 4, 2017
This test was added recently as a regress test for the spec relaxation
in #9060, but doesn't work correctly yet. Disable for now to fix noopt
builders.

Updates #22444.

Change-Id: I45c521ae0da7ffb0c6859d6f7220c59828ac6149
Reviewed-on: https://go-review.googlesource.com/81775
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
mdempsky added a commit to mdempsky/gocode that referenced this issue Apr 27, 2018
In golang/go#9060, the Go spec was relaxed to allow anonymous receiver
types. This CL updates lookdot accordingly.
@golang golang locked and limited conversation to collaborators Dec 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants