-
-
Notifications
You must be signed in to change notification settings - Fork 64
Fix assignment expression decompilation to return single assignment instead of block #261
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.
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: hazzik <[email protected]>
…nstead of block Co-authored-by: hazzik <[email protected]>
Co-authored-by: hazzik <[email protected]>
Co-authored-by: hazzik <[email protected]>
Co-authored-by: hazzik <[email protected]>
Co-authored-by: hazzik <[email protected]>
Co-authored-by: hazzik <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you integrate tests into existing test patterns? I think there is assignment tests or something existing.
Co-authored-by: hazzik <[email protected]>
Integrated the assignment expression tests into the existing AssignmentTests.cs file using the standard Test() pattern from DecompilerTestsBase. The tests now follow the established patterns in the codebase and the separate AssignmentExpressionTests.cs file has been removed. (commit 8f6f182) |
Thanks, but it does not compile anymore |
…n.Assign Co-authored-by: hazzik <[email protected]>
Fixed the compilation errors in commit bc09c03. The issue was that C# doesn't allow assignment operators directly in expression trees (CS0832 error). Changed the tests to use Expression.Assign() to manually construct the expected assignment expressions instead of lambda syntax with assignments. Tests are now compiling and passing. |
Co-authored-by: hazzik <[email protected]>
Co-authored-by: hazzik <[email protected]>
…zeExpressionVisitor Co-authored-by: hazzik <[email protected]>
Assignment expressions were incorrectly decompiled as Block expressions containing both the assignment and the assigned value, instead of returning a single assignment expression.
Problem
When decompiling assignment expressions like:
The decompiled body was a Block expression containing:
(v.MyProperty = "I wish to assign this value")
"I wish to assign this value"
But it should be just the single assignment expression.
Root Cause
The C# compiler generates IL that duplicates the assigned value on the stack:
ldstr
- Load the string constantdup
- Duplicate it (two copies on stack)stloc.0
- Store one copy to local variablecallvirt set_Property
- Call setter with other copyldloc.0
- Load stored copy as return valueThe decompiler was processing both the assignment operation and the return value separately, creating a Block with both expressions.
Solution
Added optimization logic to
OptimizeExpressionVisitor.VisitBlock()
to detect when a block contains an assignment expression followed by the same value that was assigned. In this case, it returns only the assignment expression since assignment expressions already return the assigned value in expression trees.The fix uses direct expression equality (
==
) for reliable comparison, consistent with patterns used elsewhere in the codebase. This works because the C# compiler's 'dup' instruction results in the same expression object being referenced in both positions.The optimization properly belongs in the
OptimizeExpressionVisitor
rather than during decompilation, separating concerns between IL processing and expression tree optimization.The fix handles both simple constants and complex expressions:
v => v.MyProperty = "test"
v => v.StartDate = new DateTime(2023, 1, 1)
Tests Added
ShouldDecompilePropertyAssignmentExpression
- Tests string constant assignmentShouldDecompileComplexAssignmentExpression
- Tests complex expression assignmentFixes #260.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.