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.

Sign up for GitHub

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

Jump to bottom

cmd/gofmt: Remove redundant parenthesis in foo = (1 << 42) #26706

Open
RalphCorderoy opened this issue Jul 31, 2018 · 5 comments
Open

cmd/gofmt: Remove redundant parenthesis in foo = (1 << 42) #26706

RalphCorderoy opened this issue Jul 31, 2018 · 5 comments
Labels
FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@RalphCorderoy
Copy link

go version go1.10.3 linux/amd64

Reviewing CLs showed lists of const bit-masks that were foo = 1 << 42, etc., all had redundant parenthesis around the (1 << 42). The author said they were copying similar ones in existing source. gofmt doesn't remove them. It does remove the outer pair of double ones, i.e. ((1 << 314)) in https://play.golang.org/p/V6jz7iKHJwt

I think simple cases like foo = (1 << 42) can always have them removed. That's the style followed by most of the stdlib, but not all. Having pointed it out on the CL, Martin Möhrmann is fixing those other cases that influenced the author, e.g. https://go-review.googlesource.com/126775, but it would be nice if gofmt avoided the possibility.
@martisch
Copy link
Contributor

I send CLs to make the style of internal/cpu consistent. I do not plan to rewrite any other occurrences in the Go std library.

@agnivade
Copy link
Contributor

/cc @griesemer

@agnivade agnivade added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. FeatureRequest labels Jul 31, 2018
@agnivade agnivade added this to the Unplanned milestone Jul 31, 2018
@mvdan
Copy link
Member

mvdan commented Jul 31, 2018

This can already be done with gofmt, as documented in the examples https://golang.org/cmd/gofmt/#hdr-Examples:

gofmt -r '(a) -> a' -w *.go

Perhaps the -s flag could take care of this, but I don't think that the regular gofmt is designed to do cleanups like these. All the changes that add or remove syntax should be done by -r or -s, following the current design.

@RalphCorderoy
Copy link
Author

Is there a standard list of -r that are applied to golang.org source for consistency?
The suggested -r '(a) -> a' rewrites more than I'm suggesting. It: anywhere the () are redundant. Me: the redundant () when they're outermost on an ExpressionList.

@ianlancetaylor
Copy link
Contributor

I agree that this could be done if the -s option is given, but not otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@martisch @agnivade @RalphCorderoy @ianlancetaylor @mvdan