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/printer: (a + b * *p) contracts to (a + b**p) #13025

Closed
griesemer opened this issue Oct 22, 2015 · 6 comments
Closed

go/printer: (a + b * *p) contracts to (a + b**p) #13025

griesemer opened this issue Oct 22, 2015 · 6 comments
Milestone

Comments

@griesemer
Copy link
Contributor

http://play.golang.org/p/TL4gjjbxS6

This is unfortunate and probably should be fixed. Go doesn't have a ** operator so there's no parsing issue, but it is a readability issue in my mind.

@griesemer griesemer self-assigned this Oct 22, 2015
@nodirt
Copy link
Contributor

nodirt commented Oct 22, 2015

I will give it a try.

@rsc rsc changed the title gofmt: (a + b * *p) contracts to (a + b**p) go/printer: (a + b * *p) contracts to (a + b**p) Oct 23, 2015
@rsc rsc added this to the Go1.6 milestone Oct 23, 2015
@cznic
Copy link
Contributor

cznic commented Oct 23, 2015

The current outcome seems perfectly logical to me. What result is proposed instead?

@griesemer
Copy link
Contributor Author

@cznic The result is "perfectly logical" but is not great from a readability point of view as mentioned before.

a + b * *p

does seem clearer than

a + b**p

(a**b means a to the power of b in some languages, and we even use that notation in some comments to mean that).

@cznic
Copy link
Contributor

cznic commented Oct 23, 2015

The a + b * *p variant is far less readable for me (and subjectively as ugly as it can get). The distinction between the different precedences of + and * is lost. One more 'visual' rule to learn.

What about a + b*(*p)?

@nodirt
Copy link
Contributor

nodirt commented Oct 23, 2015

AFAIK so far gofmt without -s modified only whitespace, so adding parens would be a change to that.
However, b*(*p) looks readable to me.

@griesemer
Copy link
Contributor Author

I've thought a bit more about this and decided that this is not worth the special handling.

For one, @cznic 's suggestion is a perfectly good Go solution: If the situation arises and readability is a concern, adding ()'s is the work-around.

Secondly, in the std lib (under GOROOT/src) there appears only a single occurrence of this pattern, and in that case spaces are present. src/runtime/map_test.go:382:26:

    nKeys := int(nBuckets * *runtime.HashLoad)

(I didn't find any occurrences under x/arch, x/build, x/net, x/oauth2, x/review, x/sys, and x/tools).

@golang golang locked and limited conversation to collaborators Oct 26, 2016
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

5 participants