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; } ///