x/tools/go/analysis/passes/copylock: allows assignment of composite literal with a "lock"

[ghananigans]

The copylocks checker does not complain about

var a nonCopyableType
...
a = nonCopyableType{}

but does complain about

var a nonCopyableType
var b nonCopyableType
...
a = b

(as expected).

nonCopyableType looks something like:

type nonCopyableType struct {
    _ sync.NoCopy

    ...
}

where sync.NoCopy is
https://github.com/google/gvisor/blob/a97ceec01bc464c9efa5d8140906c46f637e9002/pkg/sync/nocopy.go#L22

I think this is because of

go/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/copylock/copylock.go

Line 74 in e10c94a

func checkCopyLocksAssign(pass *analysis.Pass, as *ast.AssignStmt) {

which allows assignment with a rhs composite literal, while ignoring the lhs.

[stamblerre]

/cc @matloob

[josharian]

Writing a zero value is both safe and useful. I believe that this is working as intended.

[tamird]

But overwriting a nonzero value isn't safe, is it?

…

On Tue, Dec 8, 2020, 09:46 Josh Bleecher Snyder ***@***.***> wrote: Writing a zero value is both safe and useful. I believe that this is working as intended. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#43044 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALWYPFVHMCKD43A6GVZKZLSTY34HANCNFSM4UQBHMXA> .

[josharian]

It is.

[tamird]

What? How can that possibly be safe? This is reproducible on the playground: https://play.golang.org/p/ZW9TPnAnEcQ

[josharian]

What? How can that possibly be safe?

Instead of rhetorical questions, it would help to be concrete. What is unsafe about it? As long as the write isn't concurrent (which is the race detector's domain), you are opting to reset the mutex to a clean, unlocked state.