Repository navigation
Serialization V2 generator: native TimeSpan and small scalars (G-3), stacked on #8726 - #8727
Conversation
A registered formatter (custom, or the built-in Address/ActorPath ones) now serves collection elements, dictionary keys and dictionary values, not only fields, for every collection shape the generator supports. The wire form is the array or map the collection already uses, with each element exactly what the formatter writes at field position. A collection keeps its structure through extraction even when a member is not natively representable; validation reports what formatter resolution left unusable (AKKASG003/AKKASG014), with the same diagnostics as before. Part of akkadotnet#8715
TimeSpan, float, short, byte, sbyte, ushort, uint, ulong and char are now native, as fields, Nullable fields, and collection elements, keys and values. Wire encodings: TimeSpan is the MessagePack integer (int64) of its Ticks; float is float32; short/sbyte/byte/ushort/uint/ulong use the smallest MessagePack integer encoding, as int and long do; char is the unsigned integer of its UTF-16 code unit. Reads range-check and throw a MessagePackSerializationException naming the type instead of truncating. AKKASG007 for a type from another assembly no longer claims the generator cannot read referenced schemas, and no longer suggests declaring a BCL type in this assembly. Closes akkadotnet#8715
There was a problem hiding this comment.
Self-review of G-3 only (the commits on top of #8726). Inline comments have the detail.
What the tests prove
- All nine types round-trip at their limits as a field, nullable, and collection element.
- Exact bytes match the decided encodings.
- Size hints equal the bytes written.
- Reads throw on out-of-range wire values.
Overlap
Should_WriteTheDocumentedEncoding...and the nullable all-null byte check repeat two snapshots.Should_UseTheSmallestIntegerEncoding...tests MessagePack itself.
Not covered
- Range-check throws outside field position (nullable, collections).
uint64values beyondint64intosbyte,byte,ushort,uint,char;floatbelow-float.MaxValue; a non-integer wire type into the narrow types.- Error text checked for
shortonly. TimeSpanbad-wire values (its read threwOverflowExceptionfor auint64abovelong.MaxValue).- An absent field defaulting.
Fixed since this review (cbff3de): TimeSpan reads now throw MessagePackSerializationException naming the type; one theory covers every type at field, nullable, list and dictionary-value position with every bad-wire case and checks the message; duplicate tests removed; snapshot theory renamed. Absent-field defaulting is still untested.
| // and throws a MessagePackSerializationException naming the target type, never truncating. | ||
| // --------------------------------------------------------------------------------------------- | ||
|
|
||
| private static long ReadSignedInteger(ref MessagePackReader reader, string targetType) |
There was a problem hiding this comment.
Range checks live here, in the base-class readers the generated code calls (ReadInt16, ReadByte, ReadChar, ReadSingle, and so on). We read the widest type (long or ulong), wrap MessagePack's OverflowException (negative-to-unsigned, or beyond 64 bits) in a MessagePackSerializationException, then compare to the target's min/max and throw naming the type. Writers need no helper because MessagePackWriter.Write already picks the smallest encoding.
| case FieldKind.Double: | ||
| w.Raw("SizeOfDouble(").Value(value).Raw(")"); | ||
| break; | ||
| case FieldKind.Single: |
There was a problem hiding this comment.
Size hints for the narrow types go through SizeOfInt32/SizeOfInt64/SizeOfUInt64, and float is a fixed 5 bytes. This matches what writer.Write(...) emits, and the SizeHint == ToBinary().Length tests in NativeScalarSpec are what prove the two stay in step.
| _ => throw new ArgumentOutOfRangeException(nameof(caseName), caseName, "Unknown scalar case.") | ||
| }; | ||
|
|
||
| [Theory(DisplayName = "Should_RoundTripEveryScalar_When_ValuesAreAtTheirLimits")] |
There was a problem hiding this comment.
We round-trip every new scalar at min, max, zero, negative, encoding-size boundaries, NaN and -Infinity (plus a lone surrogate char) because it proves each type survives write then read at the values where the MessagePack width changes. We use float.Epsilon and '\uD800' to prove float32 keeps denormals and char is a raw UTF-16 code unit, not a rune.
| recovered.Char.Should().Be(message.Char, caseName); | ||
| } | ||
|
|
||
| [Theory(DisplayName = "Should_ReportExactSizeHint_When_ScalarsAreAtTheirLimits")] |
There was a problem hiding this comment.
We assert SizeHint == ToBinary().Length over the same cases because it proves the generated size expression picks the same width as the writer for every type, via an exact length match.
| } | ||
| """; | ||
|
|
||
| [Fact(DisplayName = "Should_EmitByteIdenticalOutput_When_NativeScalarsAreFieldsAndCollectionMembers")] |
There was a problem hiding this comment.
We run the generator on a corpus of Required, Optional and Positions messages and byte-compare the output to ScalarGoldenSerializer...verified.txt (664 lines) because it pins the generated code so any emitter change shows as a diff. It proves the output is unchanged, not that it is right; correctness comes from the round-trip tests. Positions covers list element, list of nullable, char key and TimeSpan key with nullable float value.
| failure.Should().BeNull(); | ||
| } | ||
|
|
||
| [Fact(DisplayName = "Should_CompileCleanly_When_GeneratedFromNativeScalarCorpus")] |
There was a problem hiding this comment.
We compile the same corpus and expect no warnings or errors under #nullable enable because the golden compare would not notice invalid C#.
Add a test that runs a formatter returning UnknownSize at element, key and value position. Drop hand-built byte tests that the formatter-* wire snapshots already pin, the one-formatter-field golden check, and the AKKASG-free control diagnostics test. Rename the snapshot theory so it names formatter shapes only.
…ialization-v2-generator-g1-g3
…e, trim duplicates TimeSpan now reads through a range-checked helper like the other types, so a wire value beyond int64 or of the wrong type throws a MessagePackSerializationException naming System.TimeSpan. The integer and float readers also name the type when the wire holds the wrong MessagePack type. The out-of-range theory now covers every type at field, Nullable, list element and dictionary value position, with uint64-beyond-int64, below -float.MaxValue and wrong-wire-type cases, and checks the message names the type. Drop the test of MessagePackWriter's own integer widths and the all-null byte check the scalars-nullable-all-null snapshot already pins. Rename the scalar snapshot theory.
Aaronontheweb
left a comment
Aaronontheweb
left a comment
There was a problem hiding this comment.
LGTM - hardened encoding and testing for scalar values
…ion-v2-generator-g1-g3 # Conflicts: # docs/articles/serialization/source-generated-serialization.md
1 participant
Second of two stacked PRs for #8715, on top of #8726. It includes the G-1 commit from that PR (formatters inside collections) until that merges, so review the top commit only. This one makes
TimeSpan,float,short,byte,sbyte,ushort,uint,ulongandcharnative, as fields,Nullablefields, and collection elements, keys and values.Changes
TimeSpanis the MessagePack integer (int64) of itsTicks.floatisfloat32(doublestaysfloat64).short,sbyte,byte,ushort,uintandulonguse the smallest MessagePack integer, asintandlongdo.charis the unsigned integer of its UTF-16 code unit.MessagePackSerializationExceptionnaming the type; nothing is truncated. Afloatread from afloat64that does not fit is rejected too.AkkaSerializerandMessagePackSizes.SizeOfSingle/SizeOfUInt64(extend-only; the V2 API is not in the approved API files).[AkkaSerializable]in the type's own assembly.Testing
ArteryControlMessageSerializer(generated) passes its tests.Breaking changes
None. These types failed to compile before, and output for existing types is byte-identical.
Closes #8715