Skip to content

Allow reordered record primary constructor assignments #4052

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sonyps5201314 wants to merge 1 commit into from
Closed

Allow reordered record primary constructor assignments #4052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions ICSharpCode.Decompiler.Tests/TestCases/Pretty/Records.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@ public record PrimaryCtorWithField(int A, string B)
public double C = 1.0;
public string D = A + B;
}
#if EXPECTED_OUTPUT
public record PrimaryCtorWithReorderedPropertyInitializers(int First, int Second);

@siegfriedpammer siegfriedpammer Aug 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should this be the expected output? The user wrote explicit auto properties, so we should keep them verbatim. The check for monotonically increasing parameterIndex values you deleted was deliberate.

The decompiler is supposed to produce C# code that matches the underlying IL as closely as possible. That is one of the core goals of ILSpy. We are not going to change that.

@sonyps5201314 sonyps5201314 Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I misunderstood the purpose of the monotonic parameterIndex check: the reversed initialization order is evidence that these are explicit auto-properties, so collapsing them into synthesized positional properties would lose source/IL fidelity. The expected output in my test was therefore wrong, and removing the check moves the decompiled result away from ILSpy's core goal.

I will close this PR instead of trying to preserve that normalization. The broader reference branch will remain available, but this change will not be proposed for merging. Thank you for the clarification.

#else
public record PrimaryCtorWithReorderedPropertyInitializers(int First, int Second)
{
public int Second { get; init; } = Second;
public int First { get; init; } = First;
}
#endif
public record PrimaryCtorWithInParameter(in int A, in string B);
public record PrimaryCtorWithProperty(int A, string B)
{
Expand Down
21 changes: 6 additions & 15 deletions ICSharpCode.Decompiler/CSharp/RecordDecompiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,6 @@ bool IsPrimaryConstructor(Block body, IMethod method, IMethod unspecializedMetho
if (body.Instructions.Count < addonInst)
return false;

int parameterIndex = 0;

for (int i = 0; i < body.Instructions.Count - addonInst; i++)
{
if (!body.Instructions[i].MatchStFld(out var target, out var field, out var valueInst))
Expand All @@ -306,16 +304,14 @@ bool IsPrimaryConstructor(Block body, IMethod method, IMethod unspecializedMetho
continue;
if (valueInst.MatchLdLoc(out var v))
{
if (!ValidateParameter(v, parameterIndex))
if (!ValidateParameter(v))
return false;
parameterIndex = v.Index!.Value;
}
else if (valueInst.MatchLdObj(out valueInst, out _) && valueInst.MatchLdLoc(out v))
{
if (!ValidateParameter(v, parameterIndex))
if (!ValidateParameter(v))
return false;
parameterIndex = v.Index!.Value;
if (method.Parameters[parameterIndex].ReferenceKind is ReferenceKind.None)
if (method.Parameters[v.Index!.Value].ReferenceKind is ReferenceKind.None)
{
return false;
}
Expand All @@ -324,7 +320,7 @@ bool IsPrimaryConstructor(Block body, IMethod method, IMethod unspecializedMetho
{
continue;
}
IParameter parameter = unspecializedMethod.Parameters[parameterIndex];
IParameter parameter = unspecializedMethod.Parameters[v.Index!.Value];
if (primaryCtorParameterToAutoProperty.ContainsKey(parameter))
{
continue;
Expand All @@ -343,17 +339,12 @@ bool IsPrimaryConstructor(Block body, IMethod method, IMethod unspecializedMetho
var returnInst = body.Instructions.LastOrDefault();
return returnInst != null && returnInst.MatchReturn(out var retVal) && retVal.MatchNop();

bool ValidateParameter(ILVariable v, int expectedMinimumIndex)
bool ValidateParameter(ILVariable v)
{
if (v.Kind != VariableKind.Parameter)
return false;
Debug.Assert(v.Index.HasValue);
if (v.Index < 0 || v.Index >= unspecializedMethod.Parameters.Count)
return false;
var parameter = unspecializedMethod.Parameters[v.Index.Value];
if (primaryCtorParameterToAutoProperty.ContainsKey(parameter))
return true;
return v.Index >= expectedMinimumIndex;
return v.Index >= 0 && v.Index < unspecializedMethod.Parameters.Count;
}
}

Expand Down
Loading