From 3671c70aa1379ea6e80a64ea8f4d5bb1731bcee0 Mon Sep 17 00:00:00 2001 From: Loretta Date: Tue, 19 May 2026 08:32:39 +0200 Subject: [PATCH] Fix SGen ref-handling asymmetry; add regression tests Refactored AcBinarySourceGenerator to use RefAwareEmitPredicate for all ref-handling switch decisions, ensuring child property ref-marker logic is based solely on child compile-time flags. Fixed deserialization drift when parent disables ref-handling but child enables it. Added regression tests and new test models to verify correct round-trip behavior for duplicate child references in collections and dictionaries. Improved XML docs and updated conventions for summary tags. Added SGen string round-trip tests for medium UTF-8/ASCII cases. --- .claude/settings.local.json | 4 +- .../AcBinarySourceGenerator.GenReader.cs | 62 +++++++------ .../AcBinarySourceGenerator.GenWriter.cs | 12 ++- .../AcBinarySourceGenerator.Models.cs | 59 ++++++++++++ .../AcBinarySerializerIIdReferenceTests.cs | 91 ++++++++++++++++++- ...SerializerSGenRuntimeCompatibilityTests.cs | 68 ++++++++++++++ .../TestModels/SharedTestOrderModels.cs | 59 ++++++++++++ .../AcBinarySerializableAttribute.cs | 15 +-- docs/CONVENTIONS.md | 4 + 9 files changed, 331 insertions(+), 43 deletions(-) diff --git a/.claude/settings.local.json b/.claude/settings.local.json index a22f6e1..a19f281 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -109,7 +109,9 @@ "Bash(stat -c '%y %s %n' \\\\ *)", "Bash(xargs stat -c '%y %s %n')", "Bash(xargs -I {} stat -c '%y %s %n' {})", - "Bash(xargs -I {} stat -c '%y %n' {})" + "Bash(xargs -I {} stat -c '%y %n' {})", + "Bash(find \"H:/Applications\" -maxdepth 4 -name \"*.sln\" -o -name \"*.slnx\" -o -name \"*.slnf\" 2>/dev/null | head -20)", + "Bash(rm -rf \"H:/Applications/Mango/Source/FruitBankHybridApp/FruitBank.Common/obj\"; dotnet build \"H:/Applications/Mango/Source/FruitBankHybridApp/FruitBank.Common/FruitBank.Common.csproj\" -c Debug -p:EmitCompilerGeneratedFiles=true 2>&1 | tail -15)" ] } } diff --git a/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.GenReader.cs b/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.GenReader.cs index 59da42c..4336163 100644 --- a/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.GenReader.cs +++ b/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.GenReader.cs @@ -59,7 +59,7 @@ public partial class AcBinarySourceGenerator foreach (var p in ci.Properties) { sb.AppendLine(); - EmitReadProp(sb, p, " ", ci.EnableMetadata, ci.EnableInternString, ci.EnableRefHandling); + EmitReadProp(sb, p, " ", ci.EnableMetadata, ci.EnableInternString); } sb.AppendLine(" }"); @@ -85,7 +85,7 @@ public partial class AcBinarySourceGenerator /// Markered types: read type code byte, then dispatch. /// Mirrors the serializer's EmitProp symmetry. /// - private static void EmitReadProp(StringBuilder sb, PropInfo p, string i, bool enableMetadata, bool enableInternString, bool enableRefHandling) + private static void EmitReadProp(StringBuilder sb, PropInfo p, string i, bool enableMetadata, bool enableInternString) { var a = $"obj.{p.Name}"; @@ -152,15 +152,15 @@ public partial class AcBinarySourceGenerator break; case PropertyTypeKind.Complex: - EmitReadComplex(sb, p, a, tc, i + " ", enableRefHandling); + EmitReadComplex(sb, p, a, tc, i + " "); break; case PropertyTypeKind.Collection: - EmitReadCollection(sb, p, a, tc, i + " ", enableInternString, enableRefHandling); + EmitReadCollection(sb, p, a, tc, i + " ", enableInternString); break; case PropertyTypeKind.Dictionary: - EmitReadDictionary(sb, p, a, tc, i + " ", enableInternString, enableRefHandling); + EmitReadDictionary(sb, p, a, tc, i + " ", enableInternString); break; default: @@ -274,7 +274,7 @@ public partial class AcBinarySourceGenerator /// Non-nullable + no ref → ZERO branches (tc consumed but ignored). /// No SGen → runtime fallback via ReadValueGenerated. /// - private static void EmitReadComplex(StringBuilder sb, PropInfo p, string a, string tc, string i, bool enableRefHandling) + private static void EmitReadComplex(StringBuilder sb, PropInfo p, string a, string tc, string i) { if (!p.HasGeneratedWriter) { @@ -299,11 +299,13 @@ public partial class AcBinarySourceGenerator var reader = p.WriterClassName!.Replace("_GeneratedWriter", "_GeneratedReader"); var cast = $"({p.TypeNameForTypeof})"; - // Ref-aware switch ONLY when both (a) the parent type opts into ref handling via EnableRefHandlingFeature - // (otherwise no Complex property of this type's reader will ever see an ObjectRef* marker — writer never - // emits them on this type) AND (b) the child type subtree may emit ref markers (ChildNeedsRefScan). - // Either flag false → ZERO-branch path (Object / FixObj only). - if (!enableRefHandling || !p.ChildNeedsRefScan) + // Ref-aware switch decision routed through RefAwareEmitPredicate — single source of truth shared + // with the writer-side EmitDirectCollectionWrite + the sibling EmitReadCollectionElement. The + // decision depends EXCLUSIVELY on the child compile-time fact `ChildNeedsRefScan` — the parent + // EnableRefHandlingFeature flag is NOT a factor here (it governs only the parent's SELF-tracking + // emit in the scan pass, not the marker dispatch for child property reads). Asymmetry-bug fix: + // see AcBinarySerializerIIdReferenceTests.Serialize_RefMarkerCollectionElement_ParentRefHandlingFeatureOff_DriftReproduction. + if (!RefAwareEmitPredicate.ChildEmitsRefMarker(p)) { // Compile-time proven: child never tracked → only Object (+ Null for nullable) in stream // Inline: parent creates instance, calls ReadProperties directly (mirrors EmitDirectObjectWrite) @@ -395,12 +397,12 @@ public partial class AcBinarySourceGenerator /// Known collection kind + inlineable element → inline Array loop with direct element reads. /// Else → runtime fallback via ReadValueGenerated. /// - private static void EmitReadCollection(StringBuilder sb, PropInfo p, string a, string tc, string i, bool enableInternString, bool enableRefHandling) + private static void EmitReadCollection(StringBuilder sb, PropInfo p, string a, string tc, string i, bool enableInternString) { // Check if we can inline: known collection shape + inlineable element type if (p.CollectionKind != null && CanInlineCollectionRead(p)) { - EmitReadCollectionInline(sb, p, a, tc, i, enableInternString, enableRefHandling); + EmitReadCollectionInline(sb, p, a, tc, i, enableInternString); return; } @@ -426,7 +428,7 @@ public partial class AcBinarySourceGenerator /// Wire format: [Dictionary][VarUInt count][key₁ value₁ key₂ value₂ ...]. /// Keys and values are read inline when their types are known (primitive/string/Complex+SGen). /// - private static void EmitReadDictionary(StringBuilder sb, PropInfo p, string a, string tc, string i, bool enableInternString, bool enableRefHandling) + private static void EmitReadDictionary(StringBuilder sb, PropInfo p, string a, string tc, string i, bool enableInternString) { var s = p.Name; var keyType = p.DictKeyTypeName ?? "object"; @@ -473,11 +475,14 @@ public partial class AcBinarySourceGenerator sb.AppendLine($"{i} {valReader}.Instance.ReadProperties(rv_{s}, context);"); sb.AppendLine($"{i} dv_{s} = rv_{s};"); sb.AppendLine($"{i} }}"); - // ObjectRefFirst / ObjectRef cases — only emit when both (a) the parent type opts into - // ref handling and (b) the dict-value subtree may emit ref markers. Either flag false → - // skip these branches (writer never emits them; reader handles unknown markers via the - // fallback ReadValueGenerated path below). ACCORE-BIN-T-K9M3 Phase C step 2. - if (enableRefHandling && p.DictValueNeedsRefScan) + // ObjectRefFirst / ObjectRef cases — routed through RefAwareEmitPredicate. Single source of + // truth shared with EmitReadComplex / EmitReadCollectionElement / EmitDirectCollectionWrite. + // The decision depends EXCLUSIVELY on the dict-value compile-time fact `DictValueNeedsRefScan` + // — the parent EnableRefHandlingFeature flag is NOT a factor here (it governs only the parent's + // SELF-tracking emit in the scan pass, GenWriter.cs:140). Symmetric with the writer-side + // dict-value emit. Asymmetry-bug fix: see AcBinarySerializerIIdReferenceTests + // .Serialize_RefMarkerCollectionElement_ParentRefHandlingFeatureOff_DriftReproduction. + if (RefAwareEmitPredicate.DictValueEmitsRefMarker(p)) { sb.AppendLine($"{i} else if ({vtc} == BinaryTypeCode.ObjectRefFirst)"); sb.AppendLine($"{i} {{"); @@ -613,7 +618,7 @@ public partial class AcBinarySourceGenerator /// Reads count + loops with direct element reads (Complex with SGen, or primitive/string/enum). /// Eliminates per-element: ReadValue dispatch, ReadObjectCore dict lookup, Activator.CreateInstance. /// - private static void EmitReadCollectionInline(StringBuilder sb, PropInfo p, string a, string tc, string i, bool enableInternString, bool enableRefHandling) + private static void EmitReadCollectionInline(StringBuilder sb, PropInfo p, string a, string tc, string i, bool enableInternString) { var isComplexElement = p.ElementKind == PropertyTypeKind.Complex && p.ElementHasGeneratedWriter; var elemType = p.ElementFullTypeName!; @@ -640,7 +645,7 @@ public partial class AcBinarySourceGenerator sb.AppendLine($"{i} for (var ri_{s} = 0; ri_{s} < cnt_{s}; ri_{s}++)"); sb.AppendLine($"{i} {{"); if (isComplexElement) - EmitReadCollectionElement(sb, p.ElementWriterClassName!.Replace("_GeneratedWriter", "_GeneratedReader"), elemType, $"({elemType})", $"ri_{s}", s, i + " ", isArray: true, p.ElementNeedsRefScan, enableInternString, enableRefHandling); + EmitReadCollectionElement(sb, p.ElementWriterClassName!.Replace("_GeneratedWriter", "_GeneratedReader"), elemType, $"({elemType})", $"ri_{s}", s, i + " ", isArray: true, p.ElementNeedsRefScan, enableInternString); else EmitReadNonComplexCollectionElement(sb, p, $"ri_{s}", s, i + " ", isArray: true, null, enableInternString); sb.AppendLine($"{i} }}"); @@ -655,7 +660,7 @@ public partial class AcBinarySourceGenerator sb.AppendLine($"{i} for (var ri_{s} = 0; ri_{s} < cnt_{s}; ri_{s}++)"); sb.AppendLine($"{i} {{"); if (isComplexElement) - EmitReadCollectionElement(sb, p.ElementWriterClassName!.Replace("_GeneratedWriter", "_GeneratedReader"), elemType, $"({elemType})", $"ri_{s}", s, i + " ", isArray: false, p.ElementNeedsRefScan, enableInternString, enableRefHandling, p.CollectionAddMethod); + EmitReadCollectionElement(sb, p.ElementWriterClassName!.Replace("_GeneratedWriter", "_GeneratedReader"), elemType, $"({elemType})", $"ri_{s}", s, i + " ", isArray: false, p.ElementNeedsRefScan, enableInternString, p.CollectionAddMethod); else EmitReadNonComplexCollectionElement(sb, p, $"ri_{s}", s, i + " ", isArray: false, p.CollectionAddMethod, enableInternString); sb.AppendLine($"{i} }}"); @@ -666,7 +671,7 @@ public partial class AcBinarySourceGenerator sb.AppendLine($"{i} for (var ri_{s} = 0; ri_{s} < cnt_{s}; ri_{s}++)"); sb.AppendLine($"{i} {{"); if (isComplexElement) - EmitReadCollectionElement(sb, p.ElementWriterClassName!.Replace("_GeneratedWriter", "_GeneratedReader"), elemType, $"({elemType})", $"ri_{s}", s, i + " ", isArray: false, p.ElementNeedsRefScan, enableInternString, enableRefHandling); + EmitReadCollectionElement(sb, p.ElementWriterClassName!.Replace("_GeneratedWriter", "_GeneratedReader"), elemType, $"({elemType})", $"ri_{s}", s, i + " ", isArray: false, p.ElementNeedsRefScan, enableInternString); else EmitReadNonComplexCollectionElement(sb, p, $"ri_{s}", s, i + " ", isArray: false, null, enableInternString); sb.AppendLine($"{i} }}"); @@ -681,7 +686,7 @@ public partial class AcBinarySourceGenerator /// SGen reader = non-metadata mode → no ObjectWithMetadata fallback. /// !needsRefScan → only Object/Null possible → 1 branch per element. /// - private static void EmitReadCollectionElement(StringBuilder sb, string reader, string elemTypeName, string elemCast, string indexVar, string propSuffix, string i, bool isArray, bool needsRefScan, bool enableInternString, bool enableRefHandling, string? addMethod = null) + private static void EmitReadCollectionElement(StringBuilder sb, string reader, string elemTypeName, string elemCast, string indexVar, string propSuffix, string i, bool isArray, bool needsRefScan, bool enableInternString, string? addMethod = null) { var etc = $"etc_{propSuffix}"; sb.AppendLine($"{i}var {etc} = context.ReadByte();"); @@ -690,9 +695,12 @@ public partial class AcBinarySourceGenerator var assignNull = isArray ? $"col_{propSuffix}[{indexVar}] = null!;" : $"col_{propSuffix}.{addCall}(null!);"; var assignExpr = isArray ? $"col_{propSuffix}[{indexVar}] = re_{propSuffix};" : $"col_{propSuffix}.{addCall}(re_{propSuffix});"; - // Ref-aware switch ONLY when both the parent type opts in (EnableRefHandlingFeature) and the - // element subtree may emit ref markers (needsRefScan). Either flag false → ZERO-branch path. - if (!enableRefHandling || !needsRefScan) + // Ref-aware switch decision routed through RefAwareEmitPredicate — single source of truth shared + // with the writer-side EmitDirectCollectionWrite + EmitReadComplex. The decision depends + // EXCLUSIVELY on the element compile-time fact `needsRefScan` — the parent EnableRefHandlingFeature + // flag is NOT a factor here. Asymmetry-bug fix: + // see AcBinarySerializerIIdReferenceTests.Serialize_RefMarkerCollectionElement_ParentRefHandlingFeatureOff_DriftReproduction. + if (!RefAwareEmitPredicate.ElementEmitsRefMarker(needsRefScan)) { // No ref tracking → only Object, FixObj or Null in stream — inline ReadProperties // FixObj slot: populate slot cache to keep _nextRuntimeSlot in sync. diff --git a/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.GenWriter.cs b/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.GenWriter.cs index 290b191..f44ab04 100644 --- a/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.GenWriter.cs +++ b/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.GenWriter.cs @@ -904,18 +904,24 @@ public partial class AcBinarySourceGenerator var elemRefSuffix = p.ElementIsIId ? "IId" : "All"; - if (!p.ElementNeedsRefScan && !p.ElementEnableMetadata) + // Ref-aware emit decision routed through RefAwareEmitPredicate — symmetric counterpart to the + // reader-side EmitReadCollectionElement / EmitReadComplex guards. Single source of truth so the + // writer-emit (which marker variants may appear on the wire) and the reader-emit (which marker + // variants are decoded) NEVER drift apart on the same PropInfo. + var elementEmitsRefMarker = RefAwareEmitPredicate.ElementEmitsRefMarker(p); + + if (!elementEmitsRefMarker && !p.ElementEnableMetadata) { // Compile-time proven: no ref, no metadata. Combined check+inc before marker write. sb.AppendLine($"{i} if (context.TryEnterRecursion(hasTruncatePath: true)) continue;"); sb.AppendLine($"{i} context.WriteByte(BinaryTypeCode.Object);"); sb.AppendLine($"{i} {writer}.Instance.WriteProperties({e}, context);"); } - else if (p.ElementNeedsRefScan && !p.ElementEnableMetadata) + else if (elementEmitsRefMarker && !p.ElementEnableMetadata) { sb.AppendLine($"{i} if (context.WriteObjectRefMarker{elemRefSuffix}()) {writer}.Instance.WriteProperties({e}, context);"); } - else if (!p.ElementNeedsRefScan && p.ElementEnableMetadata) + else if (!elementEmitsRefMarker && p.ElementEnableMetadata) { sb.AppendLine($"{i} if (context.WriteObjectMetaMarker({e}, {writer}.s_wrapperSlot)) {writer}.Instance.WriteProperties({e}, context);"); } diff --git a/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.Models.cs b/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.Models.cs index e7b347d..aa8a39c 100644 --- a/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.Models.cs +++ b/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.Models.cs @@ -248,3 +248,62 @@ internal enum PropertyTypeKind NullableBoolean, NullableSingle, NullableDouble, NullableDecimal, NullableDateTime, NullableDateTimeOffset, NullableTimeSpan, NullableGuid, NullableEnum } + +/// +/// Single source of truth for the compile-time decision: does the SGen-emit need a full ref-aware +/// switch (Object / ObjectRefFirst / Null / ObjectRef / FixObj) for a +/// given Complex property or collection element, OR can it use the zero-branch path +/// (Object / Null / FixObj only)? +/// +/// Predicate semantics: the decision depends EXCLUSIVELY on whether the child +/// element subtree may emit ref markers — captured by PropInfo.ChildNeedsRefScan / +/// PropInfo.ElementNeedsRefScan. The parent-level EnableRefHandlingFeature flag is +/// NOT a factor here — that flag governs only the parent's SELF-tracking emit in the scan +/// pass (GenWriter.cs line 140), it does NOT suppress marker dispatch for child element +/// properties of THIS type. +/// +/// Writer / reader symmetry — invoked from BOTH sides so the compile-time decision is +/// identical at every call site: +/// +/// GenReader.EmitReadComplex — guards zero-branch vs full ref-aware switch. +/// GenReader.EmitReadCollectionElement — same guard for collection-element dispatch. +/// GenReader.EmitReadDictionary — same guard for dictionary-value dispatch. +/// GenWriter.EmitDirectCollectionWrite — guards Object-only vs +/// WriteObjectRefMarker* (runtime decide) emit on the writer side. +/// +/// +/// Why a generator-only helper, not a runtime helper — the result is inlined into +/// the generated code as either the zero-branch ag or the full-switch ag. The predicate runs +/// once per emit-site at generation time; the runtime code has zero overhead from this abstraction +/// (no method call, no branch on the runtime hot path). +/// +/// Regression target: AcBinarySerializerIIdReferenceTests.Serialize_RefMarkerCollectionElement_ParentRefHandlingFeatureOff_DriftReproduction. +/// +internal static class RefAwareEmitPredicate +{ + /// + /// Reader-side decision for a Complex property (EmitReadComplex) — does the + /// emit need a full ref-aware switch on p.ChildNeedsRefScan? + /// + internal static bool ChildEmitsRefMarker(PropInfo p) => p.ChildNeedsRefScan; + + /// + /// Reader-side decision for a collection element (EmitReadCollectionElement) and + /// writer-side decision for the same element (EmitDirectCollectionWrite) — keyed on + /// p.ElementNeedsRefScan. + /// + internal static bool ElementEmitsRefMarker(PropInfo p) => p.ElementNeedsRefScan; + + /// + /// Reader-side overload for EmitReadCollectionElement when only the bool flag is in + /// scope (e.g. when PropInfo is unrolled at the call site). Same semantics — kept as + /// a thin overload so EVERY call site routes through this predicate, not the raw field. + /// + internal static bool ElementEmitsRefMarker(bool elementNeedsRefScan) => elementNeedsRefScan; + + /// + /// Reader-side decision for a dictionary value (EmitReadDictionary) — keyed on + /// p.DictValueNeedsRefScan. Symmetric with the Complex / Collection-element overloads. + /// + internal static bool DictValueEmitsRefMarker(PropInfo p) => p.DictValueNeedsRefScan; +} diff --git a/AyCode.Core.Tests/Serialization/AcBinarySerializerIIdReferenceTests.cs b/AyCode.Core.Tests/Serialization/AcBinarySerializerIIdReferenceTests.cs index 3b16287..409aeff 100644 --- a/AyCode.Core.Tests/Serialization/AcBinarySerializerIIdReferenceTests.cs +++ b/AyCode.Core.Tests/Serialization/AcBinarySerializerIIdReferenceTests.cs @@ -555,10 +555,99 @@ public class AcBinarySerializerIIdReferenceTests Assert.AreEqual(2, result[1].Id); Assert.AreEqual(1, result[1].ParentCategoryId); - + Assert.AreEqual(3, result[2].Id); Assert.AreEqual(1, result[2].ParentCategoryId); } #endregion + + #region SGen-emit writer/reader ref-handling asymmetry — regression target + + /// + /// Target test for the SGen-emit writer/reader asymmetry hypothesis — covers BOTH + /// collection-element AND dictionary-value ref-marker paths in a single graph. + /// + /// Setup: + /// + /// TestRefAsymParent [AcBinarySerializable(false)] — parent EnableRefHandlingFeature=false. + /// TestRefAsymChild [AcBinarySerializable(true)] — child IId<int>, all features ON. + /// Same child instance referenced twice in the parent's Children list + /// AND twice as VALUES in the parent's ChildrenMap dictionary. + /// Runtime ReferenceHandling=All + Interning=All (via Default options). + /// MarkerDecimal property AFTER the list — drift detection slot (decimal = 16 fixed bytes). + /// MarkerDecimal2 property AFTER the dictionary — second drift detection slot, + /// catches the symmetric dict-value emit asymmetry (EmitReadDictionary:482). + /// + /// + /// + /// Expected if the asymmetry-hypothesis holds: the writer (runtime via + /// WriteObjectGenerated bridge) emits ObjectRefFirst+ObjectRef for the duplicates; the SGen + /// reader-emit's zero-branch path (parent flag false guarding out the ref-aware switch) + /// misreads the VarUInt cacheIdx as a property-marker byte → DECIMAL_DRIFT exception or + /// value-mismatch on MarkerDecimal / MarkerDecimal2. + /// + /// + /// Expected if the hypothesis is WRONG: the test passes — different fix direction needed. + /// + /// + [TestMethod] + public void Serialize_RefMarkerCollectionElement_ParentRefHandlingFeatureOff_DriftReproduction() + { + var sharedChild = new TestRefAsymChild { Id = 1, Name = "Shared" }; + var parent = new TestRefAsymParent + { + Id = 100, + Children = new List { sharedChild, sharedChild }, + MarkerDecimal = 999.99m, + ChildrenMap = new Dictionary + { + { 10, sharedChild }, + { 20, sharedChild }, + }, + MarkerDecimal2 = 888.88m, + }; + + var options = AcBinarySerializerOptions.Default; // RefHandling=All, Interning=All + options.UseGeneratedCode = true; + + var bytes = AcBinarySerializer.Serialize(parent, options); + + // Sanity check: did the writer actually emit an ObjectRef marker for the duplicates? + var objectRefCount = CountObjectRefs(bytes, writeBinaryToConsole: false); + Console.WriteLine($"Wire size: {bytes.Length}, ObjectRef occurrences: {objectRefCount}"); + + var result = AcBinaryDeserializer.Deserialize(bytes, options); + + Assert.IsNotNull(result, "Deserialize returned null — wire corruption"); + Assert.AreEqual(parent.Id, result.Id, "Parent.Id mismatch — possible drift before the list"); + + // --- Collection-element path (EmitReadCollectionElement) --- + Assert.IsNotNull(result.Children, "Children list was null after round-trip"); + Assert.AreEqual(2, result.Children.Count, "Children count mismatch"); + Assert.IsNotNull(result.Children[0]); + Assert.IsNotNull(result.Children[1]); + Assert.AreEqual(sharedChild.Id, result.Children[0].Id, "Children[0].Id mismatch"); + Assert.AreEqual(sharedChild.Name, result.Children[0].Name, "Children[0].Name mismatch"); + Assert.AreEqual(sharedChild.Id, result.Children[1].Id, "Children[1].Id mismatch — drift on the duplicate"); + Assert.AreEqual(sharedChild.Name, result.Children[1].Name, "Children[1].Name mismatch — drift on the duplicate"); + Assert.AreEqual(parent.MarkerDecimal, result.MarkerDecimal, + "MarkerDecimal drift — wire-position desync after the Children list (smoking gun for collection-element SGen-emit asymmetry)"); + + // --- Dictionary-value path (EmitReadDictionary) --- + Assert.IsNotNull(result.ChildrenMap, "ChildrenMap was null after round-trip"); + Assert.AreEqual(2, result.ChildrenMap.Count, "ChildrenMap count mismatch"); + Assert.IsTrue(result.ChildrenMap.ContainsKey(10), "ChildrenMap missing key=10"); + Assert.IsTrue(result.ChildrenMap.ContainsKey(20), "ChildrenMap missing key=20"); + Assert.IsNotNull(result.ChildrenMap[10]); + Assert.IsNotNull(result.ChildrenMap[20]); + Assert.AreEqual(sharedChild.Id, result.ChildrenMap[10].Id, "ChildrenMap[10].Id mismatch"); + Assert.AreEqual(sharedChild.Name, result.ChildrenMap[10].Name, "ChildrenMap[10].Name mismatch"); + Assert.AreEqual(sharedChild.Id, result.ChildrenMap[20].Id, "ChildrenMap[20].Id mismatch — drift on the dict-value duplicate"); + Assert.AreEqual(sharedChild.Name, result.ChildrenMap[20].Name, "ChildrenMap[20].Name mismatch — drift on the dict-value duplicate"); + Assert.AreEqual(parent.MarkerDecimal2, result.MarkerDecimal2, + "MarkerDecimal2 drift — wire-position desync after the ChildrenMap dictionary (smoking gun for dict-value SGen-emit asymmetry)"); + } + + #endregion } diff --git a/AyCode.Core.Tests/Serialization/AcBinarySerializerSGenRuntimeCompatibilityTests.cs b/AyCode.Core.Tests/Serialization/AcBinarySerializerSGenRuntimeCompatibilityTests.cs index 9bf128a..ae52d53 100644 --- a/AyCode.Core.Tests/Serialization/AcBinarySerializerSGenRuntimeCompatibilityTests.cs +++ b/AyCode.Core.Tests/Serialization/AcBinarySerializerSGenRuntimeCompatibilityTests.cs @@ -66,6 +66,74 @@ public class AcBinarySerializerSGenRuntimeCompatibilityTests } } + /// + /// Regression test: SGen ↔ SGen round-trip with non-ASCII multi-byte ProductName above the + /// StringSmall threshold (utf8Len > 255 byte). Engages the StringMedium tier (marker 94, + /// fixed-width header [marker:1][charLen:16][utf8Len:16][bytes]). After ProductName in + /// TestOrderItemBase come Quantity (int) + UnitPrice (decimal) — any writer/reader byte-count + /// asymmetry in the StringMedium path surfaces as a UnitPrice corruption (DECIMAL_DRIFT) or + /// Quantity skew. The [AcStringIntern(true)] attribute on ProductName means the first occurrence + /// emits StringInternFirstMedium (marker 105) for the InternFirst tier. + /// + [TestMethod] + public void Serialize_MediumStringUtf8_OnProductName_SGenRoundTrip() + { + // 300 chars × 2 byte (Hungarian 'á' = 2 byte UTF-8) = 600 byte UTF-8 → StringMedium (or + // StringInternFirstMedium for the first occurrence under interning). + var mediumUtf8 = new string('á', 300); + + foreach (var optionFactory in GetOptionFactories()) + { + var options = optionFactory(); + options.UseGeneratedCode = true; + + var order = BenchmarkTestDataProvider + .CreateTestDataSets() + .Cast>() + .First(x => x.Name.StartsWith("Small")).Order; + + foreach (var item in order.Items) item.ProductName = mediumUtf8; + + var bytes = AcBinarySerializer.Serialize(order, options); + var roundTrip = AcBinaryDeserializer.Deserialize(bytes, options); + + AssertOrderEquivalent(order, roundTrip, + $"WireMode={options.WireMode}, Refs={options.ReferenceHandling}, Interning={options.UseStringInterning}"); + } + } + + /// + /// Regression test: SGen ↔ SGen round-trip with pure ASCII ProductName above the FixStrAscii inline + /// limit (>31 chars). Engages StringAscii (marker 167) — writer detects ASCII via + /// bytesWritten == charLength post-encode, reader byte→char widens directly without UTF-8 decode. + /// Same drift-surface as the UTF-8 variant: UnitPrice / Quantity after ProductName in TestOrderItemBase. + /// + [TestMethod] + public void Serialize_MediumStringAscii_OnProductName_SGenRoundTrip() + { + // 500 chars × 1 byte = 500 byte ASCII → StringAscii (167) tier. + var mediumAscii = new string('X', 500); + + foreach (var optionFactory in GetOptionFactories()) + { + var options = optionFactory(); + options.UseGeneratedCode = true; + + var order = BenchmarkTestDataProvider + .CreateTestDataSets() + .Cast>() + .First(x => x.Name.StartsWith("Small")).Order; + + foreach (var item in order.Items) item.ProductName = mediumAscii; + + var bytes = AcBinarySerializer.Serialize(order, options); + var roundTrip = AcBinaryDeserializer.Deserialize(bytes, options); + + AssertOrderEquivalent(order, roundTrip, + $"WireMode={options.WireMode}, Refs={options.ReferenceHandling}, Interning={options.UseStringInterning}"); + } + } + private static IEnumerable> GetTargetDataSets() { // SGen↔Runtime compatibility test depends on TestOrder_All_True graphs (the AssertOrderEquivalent diff --git a/AyCode.Core.Tests/TestModels/SharedTestOrderModels.cs b/AyCode.Core.Tests/TestModels/SharedTestOrderModels.cs index 3b54477..2fa2892 100644 --- a/AyCode.Core.Tests/TestModels/SharedTestOrderModels.cs +++ b/AyCode.Core.Tests/TestModels/SharedTestOrderModels.cs @@ -194,3 +194,62 @@ public sealed partial class TestMeasurementPoint_All_False UserPreferences_All_False> { } + +// ============================================================================ +// MIXED family — drift reproduction (SGen-emit asymmetry check). +// Mirrors the FruitBank ProductDto / OrderDto / GenericAttributeDto attribute: +// [AcBinarySerializable(false, true, false, true, false, false)] +// meta=false, IdTracking=true, RefHandling=FALSE, Intern=true, Filter=false, Poly=false +// +// Parent: EnableRefHandlingFeature=FALSE ◀ the asymmetry trigger +// Child: EnableRefHandlingFeature=true (IId, all features ON) +// +// Hypothesis (confirmed): the SGen reader-emit guard for collection-element / Complex / +// dictionary-value dispatch (EmitReadCollectionElement / EmitReadComplex / EmitReadDictionary) +// used to check the PARENT-level enableRefHandling flag. The writer-emit only depends on +// CHILD-level flags (ElementNeedsRefScan / DictValueNeedsRefScan / runtime +// UseTypeReferenceHandling). With runtime ReferenceHandling=All + duplicate child instances, +// the writer runtime emits ObjectRefFirst / ObjectRef, but the reader's zero-branch path +// couldn't decode them → DECIMAL_DRIFT on MarkerDecimal after the Children list or +// ChildrenMap dictionary. +// +// Fix: parent-flag removed from reader guards; routing through RefAwareEmitPredicate +// (single source of truth shared with writer-side EmitDirectCollectionWrite). +// +// The existing _All_True family tests don't exercise this path because +// EnableRefHandlingFeature=true on the parent → reader emitted the full +// ref-aware switch → never hit the zero-branch bug. +// ============================================================================ + +[MemoryPackable] +[AcBinarySerializable(false, true, false, true, false, false)] +[MessagePackObject] +public sealed partial class TestRefAsymParent : AyCode.Core.Interfaces.IId +{ + [Key(0)] + public int Id { get; set; } + + [Key(1)] + public System.Collections.Generic.List? Children { get; set; } + + [Key(2)] + public decimal MarkerDecimal { get; set; } + + [Key(3)] + public System.Collections.Generic.Dictionary? ChildrenMap { get; set; } + + [Key(4)] + public decimal MarkerDecimal2 { get; set; } +} + +[MemoryPackable] +[AcBinarySerializable(true)] +[MessagePackObject] +public sealed partial class TestRefAsymChild : AyCode.Core.Interfaces.IId +{ + [Key(0)] + public int Id { get; set; } + + [Key(1)] + public string Name { get; set; } = ""; +} diff --git a/AyCode.Core/Serializers/Attributes/AcBinarySerializableAttribute.cs b/AyCode.Core/Serializers/Attributes/AcBinarySerializableAttribute.cs index 6ba8d6a..78977f8 100644 --- a/AyCode.Core/Serializers/Attributes/AcBinarySerializableAttribute.cs +++ b/AyCode.Core/Serializers/Attributes/AcBinarySerializableAttribute.cs @@ -51,17 +51,10 @@ public sealed class AcBinarySerializableAttribute : Attribute public bool EnableIdTrackingFeature { get; } /// - /// When true (default): the SGen-emitted code emits non-IId reference tracking - /// (wrapper.TryTrackInt32(GetHashCode(...)) in the scan pass when ReferenceHandling - /// = All) AND the reader-side ObjectRef / ObjectRefFirst / ObjectWithMetadataRefFirst - /// case-emit on every Complex / Collection-element / Dictionary-value property of this type. - /// When false: both emit blocks are omitted. Significantly reduces scan-pass - /// cost — the per-instance hash-track lookup is eliminated; combined with - /// EnableIdTrackingFeature = false the scan pass for this type degenerates to a primitive-property - /// iteration only. Reader-side switch-dispatch shrinks by 2-3 cases per Complex/Collection/Dict - /// property (smaller jump table, better branch predictor, smaller IL). The runtime - /// ReferenceHandling option is silently ignored for instances of this type. Use only when - /// the type is never reference-shared across the serialized graph. + /// Controls emit of this type's non-IId hash-based reference tracking (self-registration in the + /// scan pass, paired with runtime ReferenceHandling). Scope: ONLY this type's self-tracking + /// — does NOT govern marker dispatch for child properties (child marker emit follows the child + /// type's own facts, symmetric writer/reader). See class remarks for the general flag semantics. /// public bool EnableRefHandlingFeature { get; } diff --git a/docs/CONVENTIONS.md b/docs/CONVENTIONS.md index 4f180b1..9ce1760 100644 --- a/docs/CONVENTIONS.md +++ b/docs/CONVENTIONS.md @@ -11,6 +11,10 @@ > **Workspace-wide note:** the framework-only **class prefix mandate** (`Ac` for AyCode.*, `Mg` for Mango.Nop.*; product/consumer repos un-prefixed) is an architectural rule — see `ARCHITECTURE.md#class-prefix--framework-only-mandate`. The first bullet above is the AyCode.Core-specific instance of that rule. +## XML Documentation + +`` — brief, developer-facing, readable in VS IntelliSense tooltip. NO implementation details, NO wire-format / byte-level / perf specifics — those live in `docs/TOPIC/*.md`. Add `` only when usage is non-obvious; otherwise omit. + ## Patterns - **Extension methods over instance methods** for CRUD — clean interfaces, composable impls.