Skip to content

fix: map fields with enum values, and default values in map entries - #1022

Open
jasperpotts wants to merge 2 commits into
mainfrom
1019-map-enum-values
Open

jasperpotts wants to merge 2 commits into
mainfrom
1019-map-enum-values

Conversation

@jasperpotts

@jasperpotts jasperpotts commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Description

Fixes #1019: generated code did not compile for a map whose values are an enum, such as map<int32, SomeEnum>.

Performance: no change; this is a correctness fix (compile error for enum map values, and protoc-compatible default values in map entries).

Also makes map entries with a default enum, float or double value write that value, as protoc does. This includes -0.0, so it fixes #1020 for map values only (see below).

Enum map values (#1019)

The model already declares these maps as Map<K, Integer>, holding the values' protobuf ordinals, and the writers already write them as ordinals. Only the parser and the JSON codec treated the values as enum constants. This PR makes them agree with the model rather than changing the model's API:

  • Parser: a map entry's enum value is read into an int and stored as its ordinal. Unrecognized values, such as one from a newer schema, are kept, and they round-trip byte for byte.
  • JSON codec: writes the enum name of each value (fromProtobufOrdinal(v).protoName()), and parses names back to ordinals (fromString(...).protoOrdinal()). This matches how singular enum fields are handled in JSON.
  • TestGenerator: generates ordinals for enum map values.

MapField.isEntryValue(Field) identifies a map entry's synthetic value field for the parser.

Default values in map entries

A map entry holds both a key and a value, and protoc writes both even when one is a default. PBJ already did this for int, bool, string and bytes values, but skipped enum values of 0 and float/double values of 0.0 and -0.0. So PBJ's bytes differed from protoc's, although they parse to the same map.

  • ProtoWriterTools.writeEnumProtoOrdinal, sizeOfEnum, writeFloat, writeDouble, sizeOfFloat and sizeOfDouble get skipDefault overloads, as do ProtoArrayWriterTools.writeEnumProtoOrdinal, writeFloat and writeDouble.
  • The stream writer, byte array writer, measureRecord and protobufSize() generators pass false for map entries.
  • The existing signatures keep their behaviour, so code generated for fields that are not in maps is unchanged.

#1020 outside maps

This PR does not change singular, repeated or wrapper (FloatValue, DoubleValue) fields. They still skip -0.0, because value == 0 is true for it. Fixing those means comparing bit patterns, as protoc does. To keep equals/hashCode consistent with the bytes written, generated equals and hashCode would also need to compare floats by bits. That changes the wire format of -0.0 values and model equality, so it is left for a separate decision.

Tests

  • New MessageWithEnumMaps and MessageWithFloatingPointMaps in map.proto. Their generated tests compare PBJ against protoc byte for byte and round-trip through JSON.
  • MapEnumValueTest:
    • values are ordinals that protoc reads as the right enum constants;
    • a value of 0 is written as protoc writes it;
    • unrecognized values 99 and -5 round-trip without loss through the stream writer, the byte array writer and protobufSize();
    • JSON uses the enum names.
  • MapFloatingPointValueTest: 0.0, -0.0, NaN and other values in float and double maps produce protoc's bytes from every writer and size method. The 0.0 and -0.0 cases fail without this change.
  • ProtoWriterToolsTest covers the new skipDefault overloads.
  • pbj-core build and the full integration build pass (164,008 tests).

Related

This lets the reverse writer's fallback for maps of enums in #1003 be tested.

🤖 Generated with Claude Code

The model stores a map's enum values as their protobuf ordinals, as
Map<K, Integer>, and the writers write them as ordinals, but the parser and
the JSON codec treated them as enum constants, so a map<K, SomeEnum> field did
not compile. The parser now keeps the ordinal, so unrecognized values
round-trip, and the JSON codec converts between ordinals and enum names.

Map entries also skipped an enum value of 0, where protoc, and PBJ for every
other value type, writes both key and value. The enum writers and size
methods get a skipDefault overload, used for map entries.

Fixes #1019

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
@jasperpotts jasperpotts added the Bug An error that causes the feature to behave differently than what was expected based on design. label Oct 30, 2026
@jasperpotts
jasperpotts requested a review from a team as a code owner October 30, 2026 20:13
@jasperpotts jasperpotts self-assigned this Oct 30, 2026
@jasperpotts
jasperpotts requested a review from a team as a code owner October 30, 2026 20:13
@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

JUnit Test Report

   545 files  ±0     545 suites  ±0   50s ⏱️ +7s
 1 774 tests +2   1 770 ✅ +2   4 💤 ±0  0 ❌ ±0 
11 050 runs  +2  11 022 ✅ +2  28 💤 ±0  0 ❌ ±0 

Results for commit cd4e615. ± Comparison against base commit 3d86e67.

♻️ This comment has been updated with latest results.

…does

Map entries hold both a key and a value, and protoc writes both even when one
is a default. PBJ already did for every value type except enums, fixed in the
previous commit, and float and double, whose writers skipped 0 and -0.0.
writeFloat, writeDouble, sizeOfFloat and sizeOfDouble get skipDefault
overloads, used for map entries, so the bytes match protoc's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jasper Potts <1466205+jasperpotts@users.noreply.github.com>
@jasperpotts jasperpotts changed the title fix: generate compiling code for map fields with enum values fix: map fields with enum values, and default values in map entries Oct 30, 2026
@jasperpotts

Copy link
Copy Markdown
Member Author

Hash and serialization impact

This PR changes the bytes written for some messages, and so any hash computed over them. A map entry whose value is a default enum (0), float or double (0.0, -0.0) now writes that value, as protoc does. Before, PBJ left it out. Both forms parse to the same map, but re-serializing such a message now produces different bytes, and a different hash, than before.

This does not affect any Hiero HAPI protobufs. None of the 399 .proto files they comprise (hedera-protobufs and hapi-protos, as used by pbj-integration-tests) declares a map field of any type. So consensus node and block node data is serialized exactly as before. Only projects that use maps with enum, float or double values see different bytes, and those bytes now match protoc's.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown

Integration Test Report

    623 files  + 4      623 suites  +4   50m 22s ⏱️ + 6m 10s
163 765 tests +17  163 765 ✅ +17  0 💤 ±0  0 ❌ ±0 
164 009 runs  +17  164 009 ✅ +17  0 💤 ±0  0 ❌ ±0 

Results for commit cd4e615. ± Comparison against base commit 3d86e67.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug An error that causes the feature to behave differently than what was expected based on design.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

float and double fields set to -0.0 are not written, unlike protoc Generated code does not compile for map fields with enum values

1 participant