Support protobuf editions and the protoc-gen-go Opaque API - #170
Open
ThoolooExpress wants to merge 1 commit into
Open
ThoolooExpress wants to merge 1 commit into
ThoolooExpress wants to merge 1 commit into
Conversation
Fixes planetscale#149. Supersedes planetscale#163, which sets the plugin's feature flags but makes no generator changes, so it advertises editions support that produces code which does not compile. The plugin now advertises FEATURE_SUPPORTS_EDITIONS alongside FEATURE_PROTO3_OPTIONAL, with EDITION_PROTO2 as the minimum, since vtproto supports proto2, and EDITION_2024 as the maximum. ## Implementation notes ### Toolchain I did not bump the version of protoc used repo-wide. It stays at 21.12, which does not support editions. However, it's not necessary to _use_ editions in order to support them. All of the existing gencode in this repo stays the same. To get around this problem for tests, I checked in the editions gencode from my local protoc and updated the Makefile to refresh it if needed. If requested, I can bump the toolchain repo-wide, but this will be a heavier-weight change, since modern protoc can't be vendored the same way. ### Nullability flag Each singular scalar field carries a "nullable" flag through the generator: it decides whether the field is a Go pointer or a value, which impacts gencode. That flag now comes from field.Desc.HasPresence(). The old test was file.Desc.Syntax() == Proto3 combined with a synthetic-oneof check. This only works for proto2 and proto3. An editions file reports Syntax() as Editions and has no synthetic oneofs, so a field with field_presence = IMPLICIT failed both halves of that test. HasPresence() gives the same answers as the old test on proto2 and proto3 input, so the goldens do not move. ### Hidden field access The opaque API introduces internal fields, names like `xxx_hidden_<name>` that back the setters and getters. I tried to do everything through the front door so that upstream layout changes couldn't break us, but doing so would defeat most of vtproto's optimizations by forcing a lot of additional method calls, and breaking retained-slice optimizations. I consolidated all of the hidden field accesses through the new generator/layout.go. When upstream inevitably breaks us, layout mismatches are compile errors rather than silent bugs, with one exception: the presence-bit index, which is recomputed from the same field order protoc-gen-go uses. It's unlikely upstream will significantly change this order, but if they do it should be caught when the checked-in opaque API gencode is updated. There are some places that supporting the opaque API is not 100% mechanical: - equal compares presence before values, because the values are no longer pointers. - clone copies the whole XXX_presence array in one statement. - pool keeps the *[]*T pointer across m.Reset() so the slice capacity survives, and stops retaining byte buffers for fields with a presence bit, where restoring an empty non-nil slice would contradict the bit Reset() cleared. ### Wrapping and lazy decoding I decided not to support `wrap=true` with the Opaque API, since the wrapper package cannot reach unexported fields. I also reject `[lazy = true]` fields entirely for now, rather than risk silently mis-generating lazy decoding semantics. Both cases are covered by generator validation tests. ### Build annotations Hybrid files get a //go:build !protoopaque constraint, matching the variant protoc-gen-go emits without that tag, so the helpers are absent rather than broken under -tags protoopaque. ### API levels Both plugins must be given the same default_api_level and any apilevelM<file> overrides. If they disagree, vtproto generates against the wrong struct layout; the failure is a compile error, but worth being aware of. ### Testing Tests cover opaque (proto3 with default_api_level=API_OPAQUE), editions 2023 and 2024, and hybrid. The editions target takes protoc from $PROTOC and is not part of genall, so the vendored protoc stays put; its output is checked in and CI compiles and runs it.
There was a problem hiding this comment.
Pull request overview
Adds protobuf Editions and protoc-gen-go Opaque/Hybrid API support to vtprotobuf.
Changes:
- Advertises Editions 2023–2024 support and validates unsupported configurations.
- Adapts generation to opaque field layouts, presence bits, and hidden oneof fields.
- Adds generated fixtures, tests, CI coverage, build targets, and documentation.
Reviewed changes
Copilot reviewed 19 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
Validates Hybrid opaque-tag builds. |
README.md |
Documents Editions and API-level constraints. |
Makefile |
Generates Opaque, Hybrid, and Editions fixtures. |
generator/generator.go |
Advertises Editions and validates inputs. |
generator/generator_test.go |
Tests validation failures. |
generator/layout.go |
Centralizes API-specific field layout logic. |
features/clone/clone.go |
Supports cloning opaque layouts. |
features/equal/equal.go |
Handles opaque presence and fields. |
features/marshal/marshalto.go |
Marshals opaque field storage. |
features/pool/pool.go |
Resets pooled opaque messages. |
features/size/size.go |
Sizes opaque fields. |
features/unmarshal/unmarshal.go |
Decodes into opaque storage. |
testproto/opaque/opaque.proto |
Defines Opaque API fixtures. |
testproto/opaque/opaque.pb.go |
Provides generated Opaque types. |
testproto/opaque/opaque_vtproto.pb.go |
Provides optimized Opaque helpers. |
testproto/opaque/opaque_test.go |
Tests Opaque behavior and pooling. |
testproto/hybrid/hybrid.proto |
Defines Hybrid API fixtures. |
testproto/hybrid/hybrid.pb.go |
Provides open-layout Hybrid types. |
testproto/hybrid/hybrid_protoopaque.pb.go |
Provides tagged opaque Hybrid types. |
testproto/hybrid/hybrid_vtproto.pb.go |
Provides tagged Hybrid helpers. |
testproto/hybrid/hybrid_test.go |
Tests Hybrid round trips. |
testproto/editions/editions.proto |
Defines Edition 2024 fixtures. |
testproto/editions/editions.pb.go |
Provides generated Edition 2024 types. |
testproto/editions/editions_vtproto.pb.go |
Provides Edition 2024 helpers. |
testproto/editions/editions2023.proto |
Defines Edition 2023 fixtures. |
testproto/editions/editions2023.pb.go |
Provides generated Edition 2023 types. |
testproto/editions/editions2023_vtproto.pb.go |
Provides Edition 2023 helpers. |
testproto/editions/editions_test.go |
Tests Editions presence and required fields. |
Files not reviewed (9)
- testproto/editions/editions.pb.go: Generated file
- testproto/editions/editions2023.pb.go: Generated file
- testproto/editions/editions2023_vtproto.pb.go: Generated file
- testproto/editions/editions_vtproto.pb.go: Generated file
- testproto/hybrid/hybrid.pb.go: Generated file
- testproto/hybrid/hybrid_protoopaque.pb.go: Generated file
- testproto/hybrid/hybrid_vtproto.pb.go: Generated file
- testproto/opaque/opaque.pb.go: Generated file
- testproto/opaque/opaque_vtproto.pb.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+194
to
+199
| if !repeated && p.UsesPresenceBit(field) && !p.FieldStorageIsPointer(field) { | ||
| // Unset and set-to-zero differ only in the presence bit. | ||
| p.P(`if `, p.FieldPresent("this", field), ` != `, p.FieldPresent("that", field), ` {`) | ||
| p.P(`return false`) | ||
| p.P(`}`) | ||
| nullable = false |
Comment on lines
+191
to
+214
| func presenceIndex(field *protogen.Field) int { | ||
| idx := 0 | ||
| for _, f := range field.Parent.Fields { | ||
| if f == field { | ||
| break | ||
| } | ||
| if f.Oneof == nil || isLastOneofField(f) { | ||
| idx++ | ||
| } | ||
| } | ||
| return idx | ||
| } | ||
|
|
||
| func numPresenceFields(message *protogen.Message) int { | ||
| if len(message.Fields) == 0 { | ||
| return 0 | ||
| } | ||
| return presenceIndex(message.Fields[len(message.Fields)-1]) + 1 | ||
| } | ||
|
|
||
| func isLastOneofField(field *protogen.Field) bool { | ||
| fields := field.Oneof.Fields | ||
| return fields[len(fields)-1] == field | ||
| } |
mattlord
reviewed
Aug 28, 2026
mattlord
left a comment
mattlord
left a comment
Contributor
There was a problem hiding this comment.
-
Blocking: I agree with Copilot’s open
features/equal/equal.gopresence/default-value concern. -
Blocking: I also agree with Copilot’s open
generator/layout.gopresence-index ordering concern. -
Blocking: I think that we should support or explicitly reject Editions’
DELIMITEDmessage encoding. TheGroupKindpaths infeatures/marshal/marshalto.go:371,features/size/size.go:161, andfeatures/unmarshal/unmarshal.go:483assume singular storage, and unmarshal checkswireinstead ofgroupFieldWire. A valid singularDELIMITEDfield fails withunexpected EOF, while a repeated field generates uncompilable calls such as[]*Child.SizeVT(). I think we should fix allocation/list handling and group framing, or reject this feature, with singular and repeated generation/round-trip tests. No?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #149. Supersedes #163, which sets the plugin's feature flags but makes no generator changes, so it advertises editions support that produces code which does not compile.
The plugin now advertises FEATURE_SUPPORTS_EDITIONS alongside FEATURE_PROTO3_OPTIONAL, with EDITION_PROTO2 as the minimum, since vtproto supports proto2, and EDITION_2024 as the maximum.
Implementation notes
Toolchain
I did not bump the version of protoc used repo-wide. It stays at 21.12, which does not support editions. However, it's not necessary to use editions in order to support them. All of the existing gencode in this repo stays the same.
To get around this problem for tests, I checked in the editions gencode from my local protoc and updated the Makefile to refresh it if needed.
If requested, I can bump the toolchain repo-wide, but this will be a heavier-weight change, since modern protoc can't be vendored the same way.
Nullability flag
Each singular scalar field carries a "nullable" flag through the generator: it decides whether the field is a Go pointer or a value, which impacts gencode.
That flag now comes from field.Desc.HasPresence(). The old test was file.Desc.Syntax() == Proto3 combined with a synthetic-oneof check. This only works for proto2 and proto3. An editions file reports Syntax() as Editions and has no synthetic oneofs, so a field with field_presence = IMPLICIT failed both halves of that test. HasPresence() gives the same answers as the old test on proto2 and proto3 input, so the goldens do not move.
Hidden field access
The opaque API introduces internal fields, names like
xxx_hidden_<name>that back the setters and getters. I tried to do everything through the front door so that upstream layout changes couldn't break us, but doing so would defeat most of vtproto's optimizations by forcing a lot of additional method calls, and breaking retained-slice optimizations.I consolidated all of the hidden field accesses through the new generator/layout.go. When upstream inevitably breaks us, layout mismatches are compile errors rather than silent bugs, with one exception: the presence-bit index, which is recomputed from the same field order protoc-gen-go uses. It's unlikely upstream will significantly change this order, but if they do it should be caught when the checked-in opaque API gencode is updated.
There are some places that supporting the opaque API is not 100% mechanical:
Wrapping and lazy decoding
I decided not to support
wrap=truewith the Opaque API, since the wrapper package cannot reach unexported fields. I also reject[lazy = true]fields entirely for now, rather than risk silently mis-generating lazy decoding semantics. Both cases are covered by generator validation tests.Build annotations
Hybrid files get a //go:build !protoopaque constraint, matching the variant protoc-gen-go emits without that tag, so the helpers are absent rather than broken under -tags protoopaque.
API levels
Both plugins must be given the same default_api_level and any apilevelM overrides. If they disagree, vtproto generates against the wrong struct layout; the failure is a compile error, but worth being aware of.
Testing
Tests cover opaque (proto3 with default_api_level=API_OPAQUE), editions 2023 and 2024, and hybrid. The editions target takes protoc from $PROTOC and is not part of genall, so the vendored protoc stays put; its output is checked in and CI compiles and runs it.
AI Usage
Vtprotobuf doesn't seem to have any kind of policy on AI coding tools, but for the sake of transparency, I used both Claude Code and the Zed agent backed by GPT 5.5 while working on this. This was a heavily human-in-the loop change, although I did lean heavily on the agents to help me understand the nuances of vtproto's implementation. The description was initially written by Claude Opus but was proofread and touched up by me.