Repository navigation
Fix #380: Blackbird records skip stock creator for default absent components - #381
Merged
cowtowncoder merged 7 commits intoNov 14, 2026
Merged
Conversation
…bsent components Stock gives an absent component the local's initial value (null, or zero for a primitive) when it is not required, has no null-handling override, and its deserializer is ALWAYS_NULL or a stock scalar. Construct directly in that case unless FAIL_ON_MISSING_CREATOR_PROPERTIES is enabled.
Member
|
3.3 is the right branch -- excellent. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
stevenschlansker
left a comment
Contributor
There was a problem hiding this comment.
Claude Opus 5.5 reviewer:
#381: no slow path when a record component is missing (fixes #380)
The bytecode change is fine. The bug is in how absentIsDefault() decides which missing components can use the direct path. For a non-primitive component, it only checks that getNullAccessPattern() == ALWAYS_NULL. Stock fills a missing
component with getAbsentValue(), and that is not always null:
- BaseScalarOptionalDeserializer, the parent of the three primitive Optional* deserializers, extends StdScalarDeserializer. It inherits ALWAYS_NULL but overrides getNullValue to return the empty Optional.
- The probe test record Opts(String s, OptionalInt i, OptionalLong l, OptionalDouble d) read from {"s":"x"} gives this:
- Stock: i=OptionalInt.empty, l=OptionalLong.empty, d=OptionalDouble.empty
- #381: i=null, l=null, d=null
- The same happens for a generic Envelope<OptionalInt>, so the bug reaches the envelope use case behind both issues.
- The same probe passes on #379 alone, so #381 causes it.
Suggested fix: apply the absent shortcut only where Blackbird knows the deserializer:
- the stock scalar kinds from STOCK_SCALAR_DESERS;
- CHILD codecs, whose BeanDeserializerBase reports ALWAYS_NULL and does not override the null or absent value.
Keep Kind.STOCK on the slow path. That still covers #380's String err case.
- Alternative: require that the deserializer overrides neither getNullValue nor getAbsentValue, checked by reflection. The allowlist above is simpler.
- Test: add OptionalInt/OptionalLong/OptionalDouble and a converting deserializer to BBCodecRecordAbsentTest.
Emit one seen-mask check for record fast-path construction, followed by the FAIL_ON_MISSING_CREATOR_PROPERTIES check only when some component may be absent. Add OptionalInt/OptionalLong/OptionalDouble components to BBCodecRecordAbsentTest: their absent value is the empty Optional, which relies on the databind#6437 fix (CONSTANT, not ALWAYS_NULL, null access). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ALWAYS_NULL describes getNullValue only; a deserializer may still return a non-null getAbsentValue, so a STOCK component keeps the stock creator. Allow the direct path only for the stock scalar kinds and generated child codecs, whose absent values are known, and test a custom deserializer with ALWAYS_NULL null access and a non-null absent value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Describe in the README which absent record components keep the direct constructor call, test FAIL_ON_MISSING_CREATOR_PROPERTIES toggled per call through ObjectReader, note that absentIsDefault relies on the exact STOCK_SCALAR_DESERS classes, and reflow the _recordBuild comment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The existing records always left a STOCK component absent alongside the default ones, so the direct path for absent int/long/boolean and nested codec components, and its per-call FAIL_ON_MISSING_CREATOR_PROPERTIES check, went untested. Add a Defaults record whose constructor records whether the stock creator called it, and assert direct construction for documents leaving out any subset of its components. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cowtowncoder
approved these changes
Nov 14, 2026
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 #380.
A missing component no longer forces
_recordBuildwhen stock would give it the local's initial value: notrequired, no null-handling override, and anALWAYS_NULLdeserializer (or a stock scalar, for primitives).FAIL_ON_MISSING_CREATOR_PROPERTIESstill takes the stock path. Other components, such asOptional,JsonNodeand@JsonSetter(nulls = ...), are unchanged.Reproducer from #378, record with one absent
String: 475 ns / 1536 B → 375 ns / 1128 B, same as all components present (379 ns / 1120 B).BBCodecRecordAbsentTestcompares with stock across component kinds and the relevant features.