Repository navigation
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
…alue
extension_or_default fell back to i32::default() for a singular enum
extension with no [default = ...], so a proto2 extension of
enum Level { LOW = 1; HIGH = 2; } read as 0, a number the enum does not
declare. The same happened for an editions enum with enum_type = CLOSED.
An unset field of that enum already reads as LOW, through the generated
Default impl, reflection and the map codec.
Codegen now passes Extension::with_default a fn returning the enum's
first value via its Default impl, so extern_path enums work too. Open
enums start at 0 and repeated extensions have no default, so neither
changes.
v0ropaev
force-pushed
the
fix/extension-closed-enum-default
branch
from
October 28, 2026 10:16
c5644b3 to
c34450f
Compare
This branch has not been deployed
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.
A proto2 extension of an enum that doesn't start at zero reads as 0 when it's absent and has no
[default = ...].On main
Carrier::default().extension_or_default(&LEVEL)returns 0, which isn't aLevel. An editions enum withoption features.enum_type = CLOSEDand values 3 and 4 does the same and gives 0 instead of 3. Everywhere else buffa already reads an absent closed enum as its first value: the generatedDefault, reflection since #535, and the map codec. That's also the default the descriptor gives the extension.The cause is in
buffa-codegen/src/extension.rs, which only builds a default fn whendefault_valueis non-empty. These extensions getExtension::new, soextension_or_defaultfalls through toi32::default().Now a singular extension whose enum resolves as closed gets
Extension::with_defaultwith a fn returningEnumeration::to_i32(&<Level as Default>::default()). That's the expression reflection already uses (reflect_view::enum_default_number). Going throughDefaultkeeps it working for anextern_pathenum whose values codegen can't see, which is also why it isn't aconst fn. Open enums are left alone because their first value has to be 0, and repeated extensions have no default. A closed enum that does start at 0, likeColorin ext_json.proto, gets a fn that returns 0. That's harmless, and I didn't special-case it because codegen only records first values when enum_type overrides are configured.enum_extension_doc_mentions_typenow uses theColorfromgen_in_pkg, since it pointed at an enum missing from its context and the closed-enum path now resolves it.There are two new codegen unit tests and two buffa-test cases on real generated code (proto2 in custom_options.proto, editions in group_ext.proto with an open enum next to it as a control). With only the tests applied on main, the closed-enum codegen test fails and the buffa-test pair fails with
left: 0, right: 1andleft: 0, right: 3.fmt, clippy with
-D warningsand the buffa-codegen, buffa and buffa-test suites pass on 1.95.0. No checked-in generated code changes, since none of the bundled protos declares an enum extension.