proposal: embed "noCopy" for bytes.Buffer and strings.Builder

[dotaheor]

Some new gophers may make mistakes by copying bytes.Buffer and strings.Builder values.
Embed a noCopy will let vet find such mistakes.

[dsnet]

Unfortunately, that will give the bytes.Buffer and strings.Builder types a Lock method. I think we need a cleaner way to tell the vet something is not copy-able.

[ianlancetaylor]

Note that strings.Builder already has a dynamic no-copy check.

[dotaheor]

@dsnet sorry, my mistake, the noCopy should be non-anonymous so that its methods will not be exported. Just like what sync.WaitGroup does.

@ianlancetaylor I didn't notice that strings.Builder has a dynamic no-copy check. But it looks the dynamic check doesn't work for some situations, and it increases the run-time burden.

[dsnet]

But it looks the dynamic check doesn't work for some situations, and it increases the run-time burden.

In what cases does it not work? Even if we add noCopy to strings.Builder, we would still want the dynamic check since vet is not run as part of the normal build process. Otherwise strings.Builder would provide too easy a way to accidentally (or even deliberately) step outside Go type safety.

[dotaheor]

If a Builder is copied before it writes anything, then the dynamic check doesn't work.

On the other hand, I don't understand what safety the dynamic check guarantees. I couldn't imagine any type unsafety caused by copying a Builder. It looks there are only some unexpected results from the String method calls.

[ianlancetaylor]

It's OK to copy a strings.Builder before writing anything, in the sense that both copies will work correctly.

The dynamic check is there to ensure that the String method returns a value that can not be modified by calls to the Write method in a copy of the Builder. In general lots of code assumes that Go strings can not be modified.

[ianlancetaylor]

Actually adding a Lock method does not sound like a good idea, as @dsnet mentioned above.

As described in cmd/vet/README, vet checks should be for problems that are correctness errors, frequent, and where the vet check can be precise. I guess that a small bytes.Buffer could have a correctness issue, in that there could be unanticipated aliasing of the small buffer. And a proper check could be precise. But I'm not sure this is a frequent problem.

[dotaheor]

OK, get it.

How about change the type of the Builder.buf field to *[]byte? So that we the dynamic check is not needed any more.

[ianlancetaylor]

Using a pointer instead of a slice would normally require an additional allocation, which would be unfortunate.

[dotaheor]

Yes, changing the field to a pointer also hurts the performance.
Thanks for the discussion.

Looks this issue is not very valuable, so I will close it.