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/gofmt: remove plus operator before variable #30299

Open
kevinburke opened this issue Feb 18, 2019 · 4 comments
Open

cmd/gofmt: remove plus operator before variable #30299

kevinburke opened this issue Feb 18, 2019 · 4 comments

Comments

@kevinburke
Copy link
Contributor

@kevinburke kevinburke commented Feb 18, 2019

In the following program:

package main

import "fmt"

func main() {
	a := 3
	b := 7
	a = +b
	a += +b
	fmt.Println("a", a, "b", b)
}

I believe the +b is extraneous and does not modify command operation, in both cases. Could go fmt simplify this by removing the plus operator? Similarly we remove things like braces and semicolons when they are unnecessary and do not affect program operation.

Running go tip.

@ALTree
Copy link
Member

@ALTree ALTree commented Feb 18, 2019

Could go fmt simplify this by removing the plus operator?

gofmt is not a tool that simplifies arithmetic expressions, though. It's a code formatter. Should gofmt also remove the addition in b = a + 0 ? What about a = b + c*0 ? This kind of things belong in a static checker, IMO.

Similarly we remove things like braces and semicolons

Braces and semicolons are syntactic tokens. Eliding an unnecessary token and simplify an arithmetic expression are two very different things.

@mvdan
Copy link
Member

@mvdan mvdan commented Feb 18, 2019

I agree that this is out of place for plain gofmt. Perhaps it could be considered for gofmt -s, but I still think it would be a bit too much.

In fact, gofmt already has some examples to remove unnecessary syntax nodes in arithmetic expressions and slice expressions: https://golang.org/cmd/gofmt/#hdr-Examples

If those aren't part of gofmt -s, why should +expr be?

/cc @griesemer

@mvdan mvdan added the NeedsDecision label Feb 18, 2019
@griesemer
Copy link
Contributor

@griesemer griesemer commented Feb 21, 2019

While we do certain rewrites (and in fact just introduced new ones to canonicalize the formatting of number literals for Go 1.13), we don't want to change what a programmer intended to express in the code. But I admit I don't have a good example except that perhaps one wanted to emphasize a +x next to a -x on separate lines.

You are correct that removing a unary + will not affect the expression's value (explicitly stated by the spec for complex and floating-point numbers); for ints +x is the same as 0+x which is x in all cases; for strings a unary + is not permitted).

In fact, one could remove the unary + from the language - it's never needed and only added for symmetry; which is why we probably don't want to remove it with a rewrite.

As an aside, note that we cannot easily simplify 0 + x without type information: If x is the floating-point value -0.0, than 0 + x results in 0.0 (not -0.0); that is 0 + x cannot be simplified w/o changing the program for floating-point or complex numbers.

I'm leaving this open for now for the wider proposal committee to make a decision, but I am leaning against this change.

@bcmills bcmills changed the title cmd/go: go fmt can remove plus operator before variable cmd/gofmt: go fmt can remove plus operator before variable Mar 7, 2019
@bcmills bcmills changed the title cmd/gofmt: go fmt can remove plus operator before variable cmd/gofmt: remove plus operator before variable Mar 7, 2019
@bcmills
Copy link
Member

@bcmills bcmills commented Mar 7, 2019

If x is the floating-point value -0.0

Ironically, the floating-point value -0.0 is a case where gofmt arguably should remove the leading operator, since the language spec defines that constant to result in +0.0 instead of -0.0 as one might expect.

(https://play.golang.org/p/HJcHuXKuWL2)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.