From 0e6b9a7e267f9c04ed66dfac3c46bd420a485e19 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 4 Sep 2026 17:21:45 +0200 Subject: [PATCH] Fix #3568: read a record's member order off its generated members The order of a record's fields and properties has to be known, because Equals, GetHashCode, PrintMembers and the copy constructor are recognised by walking their bodies in lockstep with it. It was assumed to be every property followed by every field, so a record that declares a field before a property desynchronised all four at once: none was recognised as generated, all of them were emitted, and the auto-properties lost their backing fields to raw k__BackingField accesses - output that does not compile. The order is in the generated members themselves, but no single one has all of it: Equals compares everything that carries state and never a computed property, PrintMembers prints everything public and never a private field. Both follow declaration order, so the two sequences are merged along the members they share, which puts a private field and a computed property back in the right places relative to each other. Members neither of them mentions - EqualityContract, static members - keep the position they had. Where the two orders conflict, which can only happen for a member that one of them never sees, the equality order wins; nothing in the metadata says more, and the choice cannot change more than the order the members are printed in. Assisted-by: Claude:claude-opus-5:Claude Code --- .../TestCases/Pretty/Records.cs | 34 +++ .../CSharp/RecordDecompiler.cs | 201 +++++++++++++++++- 2 files changed, 229 insertions(+), 6 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Records.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Records.cs index 75f65536f..99e34e73e 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Records.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Records.cs @@ -243,6 +243,40 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty private string? WebValue2; } + + public record FieldBeforeProperty(int ID, string Text) + { + public int Field; + + public int Property { get; set; } + } + + public record FieldsAndPropertiesInterleaved(int ID) + { + public int First; + + public int Middle { get; set; } + + public int Last; + } + + public record PrivateFieldAndComputedProperty(int ID) + { + public int PublicField; + + private int privateField; + + public int Computed => privateField + PublicField; + + public int Auto { get; set; } + } + + public record DerivedWithInterleavedMembers(int B) : Base(B.ToString()) + { + public int Field; + + public int Property { get; set; } + } } #if CS100 diff --git a/ICSharpCode.Decompiler/CSharp/RecordDecompiler.cs b/ICSharpCode.Decompiler/CSharp/RecordDecompiler.cs index 373c14f54..28848f745 100644 --- a/ICSharpCode.Decompiler/CSharp/RecordDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/RecordDecompiler.cs @@ -375,18 +375,207 @@ namespace ICSharpCode.Decompiler.CSharp } } - static List DetectMemberOrder(ITypeDefinition recordTypeDef, Dictionary backingFieldToAutoProperty) + List DetectMemberOrder(ITypeDefinition recordTypeDef, Dictionary backingFieldToAutoProperty) { // For records, the order of members is important: // Equals/GetHashCode/PrintMembers must agree on an order of fields+properties. - // The IL metadata has the order of fields and the order of properties, but we - // need to detect the correct interleaving. - // We could try to detect this from the PrintMembers body, but let's initially - // restrict ourselves to the common case where the record only uses properties. + // The IL metadata has the order of fields and the order of properties, but not the + // interleaving of the two, so it is read back out of the members the compiler generated + // from the declaration order (issue #3568). var subst = recordTypeDef.AsParameterizedType().GetSubstitution(); - return recordTypeDef.Properties.Select(p => p.Specialize(subst)).Concat( + var allMembers = recordTypeDef.Properties.Select(p => p.Specialize(subst)).Concat( recordTypeDef.Fields.Select(f => (IField)f.Specialize(subst)).Where(f => !backingFieldToAutoProperty.ContainsKey(f)) ).ToList(); + // Equals compares every member that carries state, in declaration order, and skips a + // property without a backing field; PrintMembers prints every public member, in the same + // order, and skips a private one. Neither alone sees all of them, so the two sequences + // are merged along the members they share. + var equalityOrder = DetectOrderFromEquals(allMembers); + var printOrder = DetectOrderFromPrintMembers(allMembers); + var declarationOrder = equalityOrder != null && printOrder != null + ? MergeAlongCommonMembers(equalityOrder, printOrder) + : equalityOrder ?? printOrder; + if (declarationOrder == null) + { + // Nothing to read the order from: the properties-first order is right for a record + // that uses only properties, which is the common case. + return allMembers; + } + // A generated member mentions neither EqualityContract, nor a static member, nor a + // private one that carries no state. Those keep the position they had, and the members + // whose order was read off fill the positions that are left, in that order. + var ordered = new List(allMembers.Count); + int next = 0; + foreach (var member in allMembers) + { + if (declarationOrder.Contains(member)) + ordered.Add(declarationOrder[next++]); + else + ordered.Add(member); + } + return ordered; + } + + /// + /// Merges two orderings of overlapping subsets into one. Members the two disagree about - + /// which can only be members that appear in one of them - are ordered by + /// , whose sequence is the one the equality members follow. + /// + static List MergeAlongCommonMembers(List first, List second) + { + var result = new List(); + int i = 0, j = 0; + while (i < first.Count && j < second.Count) + { + if (first[i].Equals(second[j])) + { + result.Add(first[i]); + i++; + j++; + } + else if (!second.Skip(j).Contains(first[i])) + { + // Only the first sequence has this one, so it goes here. + result.Add(first[i]); + i++; + } + else if (!first.Skip(i).Contains(second[j])) + { + result.Add(second[j]); + j++; + } + else + { + // Both sequences still hold both members, in opposite orders. Only one of the + // two can be right and nothing here says which, so the equality order wins. + result.Add(first[i]); + i++; + } + } + foreach (var member in first.Skip(i).Concat(second.Skip(j))) + { + if (!result.Contains(member)) + result.Add(member); + } + return result; + } + + /// + /// The members compared by the generated Equals, in the order it compares them. + /// + List? DetectOrderFromEquals(List allMembers) + { + var equalsMethod = recordTypeDef.GetMethods( + m => m.Name == "Equals" && m.Parameters.Count == 1 && !m.IsStatic + && IsRecordType(m.Parameters[0].Type), + GetMemberOptions.IgnoreInheritedMembers).FirstOrDefault(); + if (equalsMethod == null) + return null; + var body = DecompileBody(equalsMethod); + if (body == null || body.Instructions.Count == 0) + return null; + if (!body.Instructions[0].MatchReturn(out var returnValue)) + return null; + if (returnValue.MatchLogicOr(out _, out var rhs)) + { + // this == other || ... + returnValue = rhs; + } + var order = new List(); + foreach (var condition in UnpackLogicAndChain(returnValue)) + { + // callvirt Equals(call get_Default(), ldfld k__BackingField(ldloc this), ...) + if (condition is not CallVirt { Method: { Name: "Equals" } } equalsCall) + continue; + if (equalsCall.Arguments.Count != 3) + continue; + if (!MatchMemberAccessOnThis(equalsCall.Arguments[1], allMembers, out var member)) + continue; + if (!order.Contains(member)) + order.Add(member); + } + return order.Count > 0 ? order : null; + } + + /// + /// The members printed by the generated PrintMembers, in the order it prints them. The names + /// come from the string constants it appends, which say "Name = " and ", Name = ". + /// + List? DetectOrderFromPrintMembers(List allMembers) + { + var printMembers = recordTypeDef.GetMethods( + m => m.Name == "PrintMembers" && m.Parameters.Count == 1 && !m.IsStatic, + GetMemberOptions.IgnoreInheritedMembers).FirstOrDefault(); + if (printMembers == null) + return null; + var body = DecompileBody(printMembers); + if (body == null) + return null; + var function = body.Ancestors.OfType().SingleOrDefault(); + var builder = function?.Variables.SingleOrDefault( + v => v.Kind == VariableKind.Parameter && v.Index == 0); + if (builder == null) + return null; + var order = new List(); + // The name and the " = " after it are one constant for a current compiler and separate + // appends for an older one, so consecutive constants are joined before being read. + string? pending = null; + foreach (var instruction in body.Instructions) + { + if (MatchStringBuilderAppend(instruction, builder, out var value) + && value.MatchLdStr(out string? text)) + { + pending += text; + continue; + } + if (pending != null && !AddMemberNamedBy(pending)) + return null; + pending = null; + } + if (pending != null && !AddMemberNamedBy(pending)) + return null; + return order.Count > 0 ? order : null; + + bool AddMemberNamedBy(string text) + { + if (!text.EndsWith(" = ", StringComparison.Ordinal)) + return true; // the separator alone, or a constant that names nothing + string name = text.Substring(0, text.Length - " = ".Length); + if (name.StartsWith(", ", StringComparison.Ordinal)) + name = name.Substring(2); + var member = allMembers.FirstOrDefault(m => m.Name == name); + if (member == null) + return false; + if (!order.Contains(member)) + order.Add(member); + return true; + } + } + + /// + /// The member a generated body reads off "this": a field directly, or the property a + /// backing field belongs to. + /// + bool MatchMemberAccessOnThis(ILInstruction inst, List allMembers, + [NotNullWhen(true)] out IMember? member) + { + member = null; + if (inst.MatchLdFld(out var target, out var field) || inst.MatchLdFlda(out target, out field)) + { + if (!target.MatchLdThis()) + return false; + if (backingFieldToAutoProperty.TryGetValue(field, out var property)) + member = property; + else + member = allMembers.FirstOrDefault(m => m.Equals(field)); + } + else if (inst is CallInstruction { Arguments: { Count: 1 } } getterCall + && getterCall.Arguments[0].MatchLdThis()) + { + member = allMembers.OfType().FirstOrDefault( + p => getterCall.Method.Equals(p.Getter)); + } + return member != null; } ///