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: format regression #25161

Closed
dvyukov opened this issue Apr 29, 2018 · 5 comments
Closed

cmd/gofmt: format regression #25161

dvyukov opened this issue Apr 29, 2018 · 5 comments

Comments

@dvyukov
Copy link
Member

@dvyukov dvyukov commented Apr 29, 2018

Here is snippet of code:

package foo

var linuxOopses = []*oops{
	&oops{
		[]*regexp.Regexp{
			compile("INFO: lockdep is turned off"),
			compile("INFO: Stall ended before state dump start"),
			compile("INFO: NMI handler .* took too long to run"),
			compile("_INFO::"),                                       // Android can print this during boot.
			compile("INFO: sys_.* is not present in /proc/kallsyms"), // pkg/host output in debug mode
			compile("INFO: no syscalls can create resource"),         // pkg/host output in debug mode
		},
	},
}

With 1.10 formatting produces no diff, however with tip I am getting diff:

$ gofmt -d /tmp/format.go
diff -u /tmp/format.go.orig /tmp/format.go
--- /tmp/format.go.orig	2018-04-29 12:01:23.844573882 +0200
+++ /tmp/format.go	2018-04-29 12:01:23.844573882 +0200
@@ -6,7 +6,7 @@
 			compile("INFO: lockdep is turned off"),
 			compile("INFO: Stall ended before state dump start"),
 			compile("INFO: NMI handler .* took too long to run"),
-			compile("_INFO::"),                                       // Android can print this during boot.
+			compile("_INFO::"), // Android can print this during boot.
 			compile("INFO: sys_.* is not present in /proc/kallsyms"), // pkg/host output in debug mode
 			compile("INFO: no syscalls can create resource"),         // pkg/host output in debug mode
 		},

This breaks our CI.

go version devel +e0d37a33ab Sun Apr 29 09:38:32 2018 +0000 linux/amd64

@agnivade agnivade changed the title gofmt: format regression cmd/gofmt: format regression Apr 29, 2018
@agnivade agnivade added the NeedsFix label Apr 29, 2018
@agnivade agnivade added this to the Go1.11 milestone Apr 29, 2018
@agnivade
Copy link
Contributor

@agnivade agnivade commented Apr 29, 2018

@dvyukov
Copy link
Member Author

@dvyukov dvyukov commented Apr 29, 2018

To clarify the problem: we have several Go versions on CI and users using a mix of Go versions. CI checks that all code is formatted.

@agnivade
Copy link
Contributor

@agnivade agnivade commented Apr 29, 2018

bisected to 542ea5a

@jimmyfrasche
Copy link
Member

@jimmyfrasche jimmyfrasche commented Apr 29, 2018

@dvyukov see also the discussion in #22695

dvyukov added a commit to google/syzkaller that referenced this issue Apr 29, 2018
There is some regression in Go formatting on tip.
I am constantly getting diffs after formatting.
Filed: golang/go#25161
@griesemer
Copy link
Contributor

@griesemer griesemer commented Apr 30, 2018

Specific to this example: The "_INFO::" entry is just one char below the threshold. If it were "__INFO::" (for instance), the comment would be aligned. I'm not saying you should change it; but there's a threshold and this code simply is below (or above) it. This is simply "unfortunate" in this case.

Regarding the layout algorithm: It could be better, and perhaps in the future we will have a better approach (which is also why we don't want to freeze gofmt). We could trivially change the threshold but that simply moves the problem from this example to another example.

More generally: We do not want to freeze gofmt. See #22695 (comment) .

I'm going to close this as "unfortunate".

@griesemer griesemer closed this Apr 30, 2018
@griesemer griesemer added Unfortunate and removed NeedsFix labels Apr 30, 2018
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
dvyukov added a commit to dvyukov/syzkaller that referenced this issue Mar 14, 2019
Differences in code formatting between Go versions cause constant
problems for us (golang/go#25161).
Currently we support 1.9 and 1.10. Switch to newer 1.11 and 1.12.

Fixes google#1013
dvyukov added a commit to dvyukov/syzkaller that referenced this issue Mar 14, 2019
Differences in code formatting between Go versions cause constant
problems for us (golang/go#25161).
Currently we support 1.9 and 1.10. Switch to newer 1.11 and 1.12.

Fixes google#1013
dvyukov added a commit to dvyukov/syzkaller that referenced this issue Mar 14, 2019
Differences in code formatting between Go versions cause constant
problems for us (golang/go#25161).
Currently we support 1.9 and 1.10. Switch to newer 1.11 and 1.12.

Fixes google#1013
dvyukov added a commit to google/syzkaller that referenced this issue Mar 14, 2019
Differences in code formatting between Go versions cause constant
problems for us (golang/go#25161).
Currently we support 1.9 and 1.10. Switch to newer 1.11 and 1.12.

Fixes #1013
@golang golang locked and limited conversation to collaborators Apr 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.