vet checks for any (*T).Lock(). Perhaps we should limit it to a handful of known types (sync.Mutex, sync.RWMutex)?

/cc @robpike

This is related to #8005, which made the decision to embed a noCopy struct with a Lock method, which vet checks for. Russ' comment from Mar 1, 2016 explains the current status.

Ah, okay. Thanks for the context.
Where does vet check for noCopy? Couldn't find it with grep "noCopy" -r src/cmd/vet.

It doesn't check for noCopy, I think it just checks for the Lock method, which the noLock struct has. The point being, we made Lock a special "no copy" method. Personally, I think it is worth revisiting whether noCopy.Lock was the right approach or if we should expand "no copy" detection to be more general to things other than locks.

From this issue report, it seems that there are types with Lock methods that okay to copy. From #8005, it seems that there is a clear demand for declaring types (other than locks) as "no copy". Thus, it feels strange to me that having a Lock method == "no copy".

For history, this decision was from Rob's comment on the original copylocks vet CL.

This is the first complaint about this in three years, and having Lock == "no copy" was actually a really useful solution to #8005, so I think it is worth understanding a bit more before altering this.

@dlecorfec can you provide some context about what Lock means in your program?

I see. From what I understand,

Set A = all types that transitively embed sync.noCopy
Set B = all types with (*T).Lock()

cmd/vet should warn on A. But it uses B as a proxy for A, and ends up warning for types in B-A set too.

How difficult is it to determine whether type T transitively embeds type V? If it's not difficult, we should do that. Otherwise, leave it as is on the ground of this being infrequent false-positive.

Context: a Job struct (for a video encoding job), with a Lock() method which inserts a SQL row in a video_encoding_lock table in order to prevent multiple workers (on multiple servers) from processing the same job.

Notwithstanding this possibly dubious architecture (legacy stuff, slightly older than Go itself), I can easily rename this method if this cmd/vet behaviour is intended.

Being new to this video code base, I was confused by the warning because the struct didn't contain a mutex and I didn't see its Lock method at first sight :)

Thanks, @dlecorfec. The lock is external, making it ok to copy; makes sense.

I think that we should take this as a data point, recommend renaming the Lock method for now, and wait to see whether this issue arises again.

Agree with @josharian. Good data point, but for now you need to rename your Lock method. Sorry.

Done, no problem, thanks all for your time!