cmd/vet: vet is reporting lock value copying on composite literal assignment

[fsouza]

The code is the following:

    r := hipacheRouter{prefix: "hipache"}
    err := r.AddBackend("tip")
    c.Assert(err, check.IsNil)
    config.Set("hipache:redis-server", "127.0.0.1:6380")
    defer config.Unset("hipache:redis-server")
    r = hipacheRouter{prefix: "hipache"}

go vet is reporting "assignment copies lock value to r: hipache.hipacheRouter" in the last line. The type hipacheRouter indeed contains a lock:

type hipacheRouter struct {
    sync.Mutex
    prefix string
    pool   *redis.Pool
}

But as stated in the code, I'm copying a composite literal to r, so go vet should not complain about that. Changing the variable to r2 and the attribution to a declaration makes vet happy. I've also tried to make the mutex a named field, and it didn't help.

I tried to reproduce it with a simple Go program and failed to do so. I have the impression that there's another violation going on, and vet is reporting lock copying.

It works on Go 1.5.2, and fails on Go 1.6beta1, both on linux_amd64 and darwin_amd64. Details are available on Travis:

• Go 1.5.2 build: https://travis-ci.org/tsuru/tsuru/jobs/97656688#L234
• Go 1.6beta1 build: https://travis-ci.org/tsuru/tsuru/jobs/97656689#L234

[ianlancetaylor]

It's easy to recreate this:

package p

import "sync"

type S struct {
    mu sync.Mutex
}

func F() {
    r := S{}
    r = S{}
    _ = r
}

There is currently no composite literal exception to the lock check. The declaration of r is considered OK by vet. The assignment to r is not.

We could change vet to permit the copy in the case of a composite literal where the uncopyable value is uninitialized. But I'm not sure if we should make that change, since I would probably tell somebody doing this to rewrite the code.

[fsouza]

If you think that the code should be rewritten, maybe the tool could report an error on the reason why it should be rewritten? BTW, what's the reason? The risk to orphan a locked mutex on reassign?

I'm changing the code, thanks!

[ianlancetaylor]

I would recommend rewriting because the code is copying a sync.Mutex value, which is forbidden. That copy is occurring whether the value is hidden in a composite literal or not. It happens to be OK to copy the zero value of a sync.Mutex, which is why the code works. But at least to me it still seems better to not copy.

[fsouza]

@ianlancetaylor assigning values is copying, even when I'm declaring and assigning, so the first line in the function F should be invalid as well?

[ianlancetaylor]

Yes, but we let it slide because anything else would be too painful.

[fsouza]

@ianlancetaylor I see, thanks for the clarification! I've updated my code already.

Please feel free to close this issue as invalid if you think that's appropriate.

[frederikcreemers]

I'm having a similar issue, where go vet is reporting a copied mutex, when I'm not actually copying anything as far as I can tell: https://stackoverflow.com/questions/45083715/go-vet-is-telling-me-im-copying-a-lock-when-i-dont-think-iam?noredirect=1#comment77140731_45083715

[bradfitz]

@bigblind, please post a self-contained minimal repro. (not just a few lines)