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/cover: Avoid importing atomic package if not used #17817

Closed
dhananjay92 opened this issue Nov 6, 2016 · 4 comments
Closed

cmd/cover: Avoid importing atomic package if not used #17817

dhananjay92 opened this issue Nov 6, 2016 · 4 comments

Comments

@dhananjay92
Copy link
Contributor

@dhananjay92 dhananjay92 commented Nov 6, 2016

What did you do?

$ cat ~/tmp/test/haha.go
package haha

type A struct{
	i int
}

$ go tool cover -mode=atomic ~/tmp/test/haha.go
package haha

import _cover_atomic_ "sync/atomic"

type A struct {
	i int
}

var _ = _cover_atomic_.AddUint32

var GoCover = struct {
	Count     [0]uint32
	Pos       [3 * 0]uint32
	NumStmt   [0]uint16
} {
	Pos: [3 * 0]uint32{
	},
	NumStmt: [0]uint16{
	},
}

What did you expect to see?

If not needed, sync/atomic shouldn't be imported. Expected to see instrumented code like below

$ go tool cover -mode=atomic ~/tmp/test/haha.go
package haha

type A struct {
	i int
}

var GoCover = struct {
	Count     [0]uint32
	Pos       [3 * 0]uint32
	NumStmt   [0]uint16
} {
	Pos: [3 * 0]uint32{
	},
	NumStmt: [0]uint16{
	},
}

What did you see instead?

sync/atomic was imported, even though it's not used (well, except for blank assignment, but that's to prevent unused compiler errors).

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Nov 6, 2016

Why does it matter to you?

@dhananjay92
Copy link
Contributor Author

@dhananjay92 dhananjay92 commented Nov 6, 2016

This is not critical, but it's better if we can avoid importing unused dependencies. Looks like it's easier to do: golang.org/cl/32763

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 6, 2016

CL https://golang.org/cl/32763 mentions this issue.

@dhananjay92
Copy link
Contributor Author

@dhananjay92 dhananjay92 commented Nov 6, 2016

As discussed in the CL, this adds more complexity for less value. Closing the issue.

@dhananjay92 dhananjay92 closed this Nov 6, 2016
@golang golang locked and limited conversation to collaborators Nov 6, 2017
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
3 participants
You can’t perform that action at this time.