proposal: structs: add NoUnkeyedLiterals, DoNotImplement, DoNotCompare, DoNotCopy

[bjorndm]

Proposal Details

#66408 has been accepted and will add a new struct package in the standard library.

There are few other marker types we should consider to add to this structs package: namely

package structs

type NoUnkeyedLiterals struct{}
type DoNotImplement interface{
    DoNotImplementInternal(DoNotImplement) 
}
type DoNotCompare [0]func()
type DoNotCopy [0]sync.Mutex

These are actually pretty common since the Go protobuf code uses this kind of pseudo directives: https://github.com/protocolbuffers/protobuf-go/blob/master/internal/pragma/pragma.go
The Go protobuf generator includes these in all types generated so they should make up a significant part of the Go corpus.

Furthermore as shown in #67196 currently the lack of these marker types lessens the readability of Go code. There are likely many Go projects that could use these marker types.So I think they will pass the bar of being common enough to be included in the standard library. Adding them to the standard library also allows to improve go vet detection in the case of DoNotCopy.

So please consider adding them.

[Jorropo]

That a lot of unrelated things that I have unrelated feelings about.
I think NoUnkeyedLiterals and DoNotImplement deserve their own issues.

Actually can be summarized:
Your proposal for structs include 3 non-structs and seems to go against #66408's spirit, my understanding is that they should impact how the compiler do things,
for example [0]sync.Mutex doesn't actually do anything at compile time or runtime (that a go vet check).
I would rather like to see struct{} and some formal definition for what DoNotCopy is meant to do.

[bjorndm]

The implementation of these marker types is taken from the go protobuf library, however, it might be that with compiler support this can be implemented differently.

The implementations may not be structs but they are to be used in structs. So I think this is the correct package to place them in.

As for their meanings:

NoUnkeyedLiterals can be included in a struct to prevent initializing it with an unkeyed literal.

DoNotImplement prevents trivial interface implementation.

DoNotCompare can be included in a struct to prevent comparing it with the == operator.

DoNotCopy can be included in a struct to prevent copying it with the = operator.

[Jorropo]

Except than DoNotCompare I think they are on the order of: https://github.com/golang/proposal/blob/master/go2-language-changes.md

And the template would help with the details like: should this be a compiletime or runtime error ? or what kind of problems this is trying to solve.

---

DoNotCopy can be included in a struct to prevent copying it with the = operator.

This doesn't fit enough with the rest of the language yet,
« copying » and « copy » is only ever used in regard to slices in the spec.
The lack of formal copy definition make it unclear what should happen in theses edge-cases:

type T struct{
 _ structs.DoNotcopy
}

var x [3]T

copy(x[:], x[1:]) // is that a copy ? according to `sync` yes and if `T` was a mutex it would panic or do horrible things after being used

type T struct{
 _ structs.DoNotcopy
}

func F(t T) {}
F(T{}) // is that a copy ? according to `sync` yes and if `T` was a mutex it would panic or do horrible things after being used

I think this also need to explain how copy-ability becomes part of generic functions signatures:

type T struct{
 _ structs.DoNotcopy
}

func A[T any]() {
 var leakv T
 var b T
 leakv = b
 _ = leakv
}
func B[T any]() {
 var leakp *T
 var b T
 leakp = &b
 _ = leakp
}

A[T]() // shouldn't work
B[T]() // should work

[zephyrtronium]

@Jorropo see #66408 (comment):

@bjorndm , yep. That's one of the reasons we proposed structs for this. It would be a great place for further marker types. Those, of course, would be separate proposals.

I am not sure what makes you think the proposed types are language changes. Aside from DoNotImplement (which is about interfaces, not structs), all of them work as intended today exactly as they are defined in the proposal. DoNotCopy in particular interacts with the copylocks analyzer, and that is sufficient for how it is used in practice. I don't understand this proposal as suggesting to elevate it to the type system itself.

[Jorropo]

Ok I guess I was wrong on the intent, but like do we want structs that don't do anything at runtime nor compiletime in the std ?
Even if go vet time feature is good enough, there are many non trivial edge cases it does not catch.
Adding exactly the feature that exists right now would mean we would have something that kinda work, sometimes, when you use the right tooling, and don't do anything fancy like copying structs using the copy builtin.
Is that good enough ?
I think we should decide what that means in regard to the langage and use that to fix the edgecases in go vet or add the checks to the compiler.

[bjorndm]

What I propose is now already in use for years for the go grpc and protobuf library, and other libraries like Ebitengine also use this often. The effects of these types is to force a compile error or vet check to prevent incorrect use of structs. The main reason to add this to stdlib is to increase the legibility and because of this wide use.

[apparentlymart]

I don't mean to say that this necessarily disqualifies the additions in this proposal, but they feel quite different to what was accepted in #66408:

• They are not part of the language spec directly, and instead achieve their effect only by exploiting existing rules in the spec (or, in the case of DoNotCopy, a go vet rule).
• They do not hint the compiler to select a different layout for other fields of the type they are embedded in.

By this comment I only mean to say that I don't think the acceptance of that proposal is particularly relevant to evaluating this one. That doesn't mean that these ideas aren't valuable on their own merits.

I'm not sure, yet. It does feel nice to give these little "type hacks" real names so that folks encountering them in unfamiliar code will get some clues as to what they do, but something with more explicit support in the compiler would presumably allow for better compile-time error messages.

I suppose in principle they could be accepted as pure library additions while leaving compiler implementations the option of recognizing them and generating specialized error messages purely as an implementation detail, rather than as something required by spec. Is there any existing precedent for such a thing?

[bjorndm]

Yes, I don't think this should require any changes to the go language spec. Rather, that Go compiler implementations may use these tag types to provide better error messages as an implementation detail.

[adonovan]

There are several interconnected issues here. There are really four separate subproposals, one per declaration; many of these already have their own history of rejected proposals, which should be linked and summarized.

For example, these all relate to DoNotCopy:

• runtime: add NoCopy documentation struct type? #8005
• proposal: annotation to generalize lostcancel for other kinds of cleanup function #65682
• x/tools/go/analysis/passes/copylock: allows assignment of composite literal with a "lock" #43044
• proposal: Go 2: NoCopy: official support in compiler (not just vet linter) #23764
• sync/atomic: remove noCopy from Value #21504

And then there's the question of whether these new types are intended as

• pragmas to change the compiler's behavior to make it stricter, or just
• annotations for static analysis tools such as go vet (and a central place to hang documentation).

If annotations: there have been a number of request for things like this, and it is something we would like to address in the coming year. One open question is whether all annotations should be declared in a uniform way, simplifying the task of processing them (at the cost of making them harder to declare). For example, the annotation for NoUnkeyedLiterals, to pick one, could be expressed something like this:

package unkeyed

import "annotate" // hypothetical future standard package

type NoUnkeyedLiterals struct{}

func _() {
    annotate.Type[NoUnkeyedLiterals]() // tell the analysis tools that the NoUnkeyedLiterals type is special
}

(Let's not get distracted by the many different choices possible here.)

[bjorndm]

Well I grouped these 4 together because of their common definition in the protobuf library. But if it becomes unwieldy it could be split up. I'm proposing this more for annotations and documentation. So if structs is not a good place for these marker types then an annotations package may be fine as well. Although I am not sure if generics will be needed. The current definitions work fine as well.

[ldemailly]

There are really four separate subproposals, one per declaration; many of these already have their own history of rejected proposals, which should be linked and summarized.

• proposal: Go 2: NoCopy: official support in compiler (not just vet linter) #23764

So #23764 (I didn't remember I filled this ages ago already) wasn't rejected it just died of old age, can we reopen it?

I agree NoCopy is a bit buried in this proposal

[ianlancetaylor]

#23764 was rejected: #23764 (comment)

I think we should restart NoCopy with a new proposal. I agree with others that I don't think it's helpful for this proposal to include different unrelated ideas.

[ldemailly]

A specific proposal was made (in term of goal) and details were added and not followed up after that comment, I think it was clear enough but someone that will be able to answer the apparently needed implementation details above what was discussed in #23764 should open a new one then - please link it here so I can follow

[ianlancetaylor]

Thanks, but, with respect, a goal is not a proposal. The best proposals have specific ideas for how to achieve their goals. The place to develop those specific ideas is a forum (see https://go.dev/wiki/Questions), not the issue tracker. Thanks again.