proposal: allow eliding "_" fields in unkeyed struct literals

[dr2chase]

Proposal Details

Change treatment of _ fields so that they may be omitted from unkeyed struct literals and (if omitted) do not count as unexported fields. In the case of a struct with multiple _ fields, either all must be present or all must be omitted, to avoid ambiguity.

The reason for this is to make it easier to use _ fields for things like establishing field alignment, modifying layout, and other type attributes. Without this change, _ fields require providing pointless initial "values" in unkeyed literals, and make it impossible to use unkeyed literals across package boundaries (it may be bad style, but it is legal Go and probably exists -- a particular use case is Windows). One specific motivation was looking at the work required and effects on code/tools/usability to add structs.HostLayout annotations to the code generated by cgo; it was messy and unpleasant.

This breaks no Go code, so it is compatible.

It also "potentially affects" very little code anyway; unkeyed literals for structs with _ fields are rare. I ran an ecosystem analysis of 44,000 Go projects that were imported 5 or more times. Of those, there were only 137 instances of an unkeyed literal "initializing" a _ field. Of that 137, 127 were in tests, of of that 127, 75 were literals for the same struct type. Of the 10 "non-test" examples, it turned out to really be only 5 because of duplication in the report, appearing in only 3 p[rojects, and 2 of that 3 involved VNC (i.e., derivative code).

In any case where the export-forbidding properties of _ were desirable, renaming that field to any other export-forbidding name (e.g., __ or underscore) obtains the same effect.

This will require a change to the Go compiler, and to tools that analyze Go code, so that both forms of struct literals are recognized.

[ianlancetaylor]

For example, given

type S struct {
    F1 bool
    _   int
    F2 bool
}

code would be permitted to write

var s = S{true, false}

Currently that is not permitted. With this proposal, it would be permitted. In particular, since F1 and F2 are exported, it would be permitted in a different package.

Let me know if that is not right.

[timothy-king]

One specific motivation was looking at the work required and effects on code/tools/usability to add structs.HostLayout annotations to the code generated by cgo; it was messy and unpleasant.

More details on what "messy and unpleasant" looked like in practice would be helpful here. I don't think I understand the motivation yet.

[rittneje]

In any case where the export-forbidding properties of _ were desirable, renaming that field to any other export-forbidding name (e.g., __ or underscore) obtains the same effect.

While perhaps not literally a breaking change, this feels like a breaking change in spirit.

[dr2chase]

@timothy-king It was messy in its interactions with gopls, and there were spurious (false positive) errors. I do not recall the exact details, but it caused me to abandon work on that change to cgo.

The motivation is that we think these annotations should be added to platform specific code, but anytime that an API exports a platform data structure, it will force what is effectively a breaking change to their API. I.e., the annotation creates a problem, and I would prefer that it not create a problem.

Allowing these annotations gives us a way to note when a structure's layout matters, and therefore also when it does not matter, so that the compiler can do a better job of arranging structures for both size and pointer locality (which affects the memory bandwidth requirements of GC in some cases). That would let us remove a by-hand optimization task, and remove a constraint of field-ordering (which in theory means that programmers can arrange them more "naturally", whatever that might mean).

There have been other proposals for things we might do with zero-sized annotation fields, some of them are probably worthwhile, and letting _ fields be ignored in this way will reduce the barrier to use of such fields whenever they might appear in a public API.

The reason for using zero-sized annotation fields (in general) is that when such types cannot be mixed (because of layout changes, or use restrictions, or whatever annotations we come up with), the presence of differing fields acts through the existing type system to keep them separate; this approach causes less breakage/surprise in the Go tools, than, say, adding a magic //go:whatever annotation to a type.

[ianlancetaylor]

Assuming we understand the problem correctly, there is another approach that could be used for cgo. cgo could rewrite all instances of C.S{v1, v2} to become C.S{structs.HostLayout{}, v1, v2}. If we did that, would this proposal still be necessary or useful?

In particular we are concerned that adding this rule complicates the language in a way that will puzzle and confuse future readers.

-- for @golang/proposal-review

[dr2chase]

@ianlancetaylor

I think I tried that, and it was messy. I do not recall the exact nature of the mess, but it was bad enough that I came up with this proposal instead. I refreshed the CLs where I did that work, and I will try to find the problem that made me give up. They are here: https://go-review.googlesource.com/c/go/+/647400

My arguments against "puzzle and confuse" are:

• it is not a large change, so the confusion would not be large.
• there's very little code that uses unkeyed struct literals for structures with _ fields
• if necessary, long-term we could change it to require elision in some near-future next version of Go; this could not cause large problems reading older code with the _ initializers in it because that is so rare.

[rittneje]

there's very little code that uses unkeyed struct literals for structures with _ fields

Given that one of the primary motivations for _ fields is to prevent unkeyed literals (which this proposal breaks), I don’t think this is a point in favor of this change.

[dr2chase]

I'm still working on the cgo CLs, but I also gathered some data from an ecosystem analysis. I was curious how many cross package and cross-module unkeyed literals already existed in real code, and there were more than I expected, given that vet flags all of these (the first set is literally the vet check, wrapped into a standalone analysis tool):

Across the 68700+ github projects with 3 or more imports, there were:

• 92559 cross-package unkeyed literals
• 49259 cross-package unkeyed literals, excluding "_test"
• 2803 types constructed ( excluding "_test" )
• 1136 projects (excluding "_test" )

Considering only unkeyed literals constructing types from packages in other modules:

• 20879 cross-package unkeyed literals
• 15096 cross-package unkeyed literals, excluding "_test"
• 444 types constructed ( excluding "_test" )
• 536 projects ( excluding "_test" )

I think this data supports this proposal -- if any of these hundreds of types is later tagged for layout purposes, it will break the importing code.

Regarding "preventing unkeyed literals", there are plenty of (literally countless) other ways to do this than _, but _ is often used for that now.

I think we're balancing two hypothetical risks.

In one scenario, if we adopt this proposal and make _ fields optional in unkeyed literals:

• P had used a _ field in struct type S to prevent unkeyed literals, but that no longer "works".
• importer C creates a previously impossible unkeyed literal for S.
• a later change in P.S breaks C.

This is a temporary risk, because until such an importer exists, P can change _ to _x to restore the previous guarantee of no unkeyed literals in importers.

In the other scenario in which _ fields remain non-optional:

• C has an unkeyed literal P.S (see above).
• P decides to add a tag field to P.S, which breaks C.

Based on this observation, wondering how often _ is used to prevent unkeyed literal dependence on field order in importers, I ran another analysis (same 3+ imports rule) to count how often a _ field was the only package-private field; there were about 32,000 of those occurring in 354 projects. 32,000 is a lot.

I then went and looked at a random sample of 0.001 of these:

                        _ [4]byte
                        _ int // for godoc to show unexported fields
                        _ uint32     // unused (must be zero)
                        _ struct{} `validate:"requiredAnyOf=message;type"`
                        _ struct{} `cbor:",toarray"`
                        _ struct{} `kafka:"min=v2,max=v3,tag"`
 (11x, same project)  	_ struct{} `type:"structure" json:",omitempty"`
 (18x, same project) 	_ struct{} `type:"structure"`
                        _ [0]func()

The first thing that jumped out at me was that the sample was overwhelmed by a few projects, and these uses were overwhelmingly machine-generated.

Another interesting thing was the use of [0]func() as a "this type cannot be compared" tag. This is a thing people are already doing, the distinction between "layout" and "use" tags is looking a little more arbitrary. It is my opinion that in the future we will want to give these things names that signal their intent; there's no documentation for [0]func() that signals that this prevents comparison, "you just gotta know" and there's plenty of people reading code who don't know the last detail of what things mean. If it were instead structs.NoCompare with a godoc etc, the answer is just a tool-tip away.

But, given the unusual over-occurrence of just two projects in the random sample, what's the distribution?
It turns out that 90% of these only-private-field uses occur in just 12 projects, of which three seem to be duplicates.
HALF are in github.com/journeymidnight/aws-sdk-go, and the next 25% are in github.com/volcengine/volcengine-go-sdk.

  sum count project
...
 3012   157 google.golang.org/protobuf
 3222   210 github.com/exoscale/egoscale
 3450   228 github.com/alphabill-org/alphabill
 3692   242 github.com/2mf8/LagrangeGo
 4000   308 github.com/LagrangeDev/LagrangeGo
 4353   353 github.com/aws/amazon-ecs-agent/ecs-agent
 4738   385 github.com/Mrs4s/MiraiGo
 5123   385 github.com/Sclock/MiraiGo
 5508   385 github.com/eya46/MiraiGo
 6105   597 github.com/ProtocolScience/AstralGo
 6901   796 github.com/bufbuild/protovalidate-go
14041  7140 github.com/volcengine/volcengine-go-sdk
31910 17869 github.com/journeymidnight/aws-sdk-go

Machine-generated code is overwhelmingly the common case for these, so we can mitigate the risk of enabling future fragile literals by modifying the code generators to use a different private field name.

[ianlancetaylor]

if any of these hundreds of types is later tagged for layout purposes, it will break the importing code.

But as you also point out, people are already today tagging structs to mark them as non-comparable. And the importers of these structs are apparently willing to live with that risk, and they are ignoring a vet check telling them that they are doing something wrong.

And I don't see any special reason to believe that layout tags will be widely used. For the particular cases of syscall and x/sys and cgo it's obviously important. But for arbitrary Go code, it's not clear to me that it is important.

So I feel that the problem is the intersection of two unlikely cases: people ignoring vet warnings, and people who want struct layout tags.

So we're talking about a benefit to what I think is a fairly small number of people—and in fact we don't know for sure that there is any case at all—compared to the cost of a rather odd language change.