Pointer increments and compund assignements not always decompiled in a "pretty" way.

[ElektroKill]

Input code

unsafe class UIntCases {
	static void Sample1() {
		uint local = 0;
		uint* r = &local;
		uint g = (*r++) * (*r++);
		Console.WriteLine(g);
	}
	static void Sample1(uint param) {
		uint* r = &param;
		uint g = (*r++) * (*r++);
		Console.WriteLine(g);
	}
	static void Sample5() {
		uint local = 0;
		uint* r = &local;
		uint g = (*r += 5) * (*r += 5);
		Console.WriteLine(g);
	}
	static void Sample5(uint param) {
		uint* r = &param;
		uint g = (*r += 5) * (*r += 5);
		Console.WriteLine(g);
	}
	static void Sample52() {
		uint local = 0;
		uint* r = &local;
		uint g = (*(r += 5)) * (*(r += 5));
		Console.WriteLine(g);
	}
	static void Sample52(uint param) {
		uint* r = &param;
		uint g = (*(r += 5)) * (*(r += 5));
		Console.WriteLine(g);
	}
}
unsafe class ByteCases {
	static void Sample1() {
		byte local = 0;
		byte* r = &local;
		int g = (*r++) * (*r++);
		Console.WriteLine(g);
	}
	static void Sample1(byte param) {
		byte* r = &param;
		int g = (*r++) * (*r++);
		Console.WriteLine(g);
	}
	static void Sample5() {
		byte local = 0;
		byte* r = &local;
		int g = (*r += 5) * (*r += 5);
		Console.WriteLine(g);
	}
	static void Sample5(byte param) {
		byte* r = &param;
		int g = (*r += 5) * (*r += 5);
		Console.WriteLine(g);
	}
	static void Sample52() {
		byte local = 0;
		byte* r = &local;
		int g = (*(r += 5)) * (*(r += 5));
		Console.WriteLine(g);
	}
	static void Sample52(byte param) {
		byte* r = &param;
		int g = (*(r += 5)) * (*(r += 5));
		Console.WriteLine(g);
	}
}

Erroneous output

// ILSpyPointerIncrement.UIntCases
using System;

internal class UIntCases
{
	private unsafe static void Sample1()
	{
		uint local = 0u;
		uint* num = &local;
		uint* r = num + 1;
		uint num2 = *num;
		uint* num3 = r;
		r = num3 + 1;
		Console.WriteLine(num2 * *num3);
	}

	private unsafe static void Sample1(uint param)
	{
		uint* num = &param;
		uint* r = num + 1;
		uint num2 = *num;
		uint* num3 = r;
		r = num3 + 1;
		Console.WriteLine(num2 * *num3);
	}

	private unsafe static void Sample5()
	{
		uint local = 0u;
		uint* r = &local;
		Console.WriteLine((*r += 5u) * (*r += 5u));
	}

	private unsafe static void Sample5(uint param)
	{
		uint* r = &param;
		Console.WriteLine((*r += 5u) * (*r += 5u));
	}

	private unsafe static void Sample52()
	{
		uint local = 0u;
		uint* num = &local + 5;
		uint* r;
		Console.WriteLine(*num * *(r = num + 5));
	}

	private unsafe static void Sample52(uint param)
	{
		uint* num = &param + 5;
		uint* r;
		Console.WriteLine(*num * *(r = num + 5));
	}
}

// ILSpyPointerIncrement.ByteCases
using System;

internal class ByteCases
{
	private unsafe static void Sample1()
	{
		byte local = 0;
		byte* num = &local;
		byte* r = num + 1;
		Console.WriteLine(*num * *(r++));
	}

	private unsafe static void Sample1(byte param)
	{
		byte* num = &param;
		byte* r = num + 1;
		Console.WriteLine(*num * *(r++));
	}

	private unsafe static void Sample5()
	{
		byte local = 0;
		byte* r = &local;
		byte b;
		*r = (b = (byte)(*r + 5));
		byte num = b;
		*r = (b = (byte)(*r + 5));
		Console.WriteLine(num * b);
	}

	private unsafe static void Sample5(byte param)
	{
		byte* r = &param;
		byte b;
		*r = (b = (byte)(*r + 5));
		byte num = b;
		*r = (b = (byte)(*r + 5));
		Console.WriteLine(num * b);
	}

	private unsafe static void Sample52()
	{
		byte local = 0;
		byte* num = &local + 5;
		byte* r;
		Console.WriteLine(*num * *(r = num + 5));
	}

	private unsafe static void Sample52(byte param)
	{
		byte* num = &param + 5;
		byte* r;
		Console.WriteLine(*num * *(r = num + 5));
	}
}

Code does compile. Behavior before and after was not tested.

Details

• Product in use: ILSpy CI build from db7d507

[ElektroKill]

A more "complex" test case:

unsafe class MixedTests {
	static void Mixed1(uint[] y, uint l, uint* e) {
		uint h = 0;
		for (uint i = 0; i < l; i++) {
			*e ^= y[h & 0xf];
			y[h & 0xf] = (y[h & 0xf] ^ (*e++)) + 0x3dbb2819;
			h++;
		}
	}
}

Decompiled code:

internal class MixedTests
{
	private unsafe static void Mixed1(uint[] y, uint l, uint* e)
	{
		uint h = 0u;
		for (uint i = 0u; i < l; i++)
		{
			*e ^= y[h & 0xF];
			uint num = h & 0xF;
			uint num2 = y[h & 0xF];
			uint* num3 = e;
			e = num3 + 1;
			y[num] = (num2 ^ *num3) + 1035675673;
			h++;
		}
	}
}

[siegfriedpammer]

See also #947

[ElektroKill]

I went ahead and tested this with ILSpy 2.4 and the result for some of these test cases is slightly better:

namespace ILSpyPointerIncrement
{
	internal class ByteCases
	{
		private unsafe static void Sample1()
		{
			byte b = 0;
			byte* ptr = &b;
			int value = (int)(*(ptr++) * *(ptr++));
			Console.WriteLine(value);
		}

		private unsafe static void Sample1(byte param)
		{
			byte* ptr = &param;
			int value = (int)(*(ptr++) * *(ptr++));
			Console.WriteLine(value);
		}

		private unsafe static void Sample5()
		{
			byte b = 0;
			byte* ptr = &b;
			byte* expr_08 = ptr;
			int arg_1B_0 = (int)(*expr_08 += 5);
			byte* expr_12 = ptr;
			int value = arg_1B_0 * (int)(*expr_12 += 5);
			Console.WriteLine(value);
		}

		private unsafe static void Sample5(byte param)
		{
			byte* ptr = &param;
			byte* expr_06 = ptr;
			int arg_19_0 = (int)(*expr_06 += 5);
			byte* expr_10 = ptr;
			int value = arg_19_0 * (int)(*expr_10 += 5);
			Console.WriteLine(value);
		}

		private unsafe static void Sample52()
		{
			byte b = 0;
			byte* ptr = &b;
			int value = (int)(*(ptr += 5) * ptr[5]);
			Console.WriteLine(value);
		}

		private unsafe static void Sample52(byte param)
		{
			byte* ptr = &param;
			int value = (int)(*(ptr += 5) * ptr[5]);
			Console.WriteLine(value);
		}
	}
}

namespace ILSpyPointerIncrement
{
	internal class MixedTests
	{
		private unsafe static void Mixed1(uint[] y, uint l, uint* e)
		{
			uint num = 0u;
			for (uint num2 = 0u; num2 < l; num2 += 1u)
			{
				*e ^= y[(int)(num & 15u)];
				y[(int)(num & 15u)] = (y[(int)(num & 15u)] ^ *(e++)) + 1035675673u;
				num += 1u;
			}
		}
	}
}

namespace ILSpyPointerIncrement
{
	internal class UIntCases
	{
		private unsafe static void Sample1()
		{
			uint num = 0u;
			uint* ptr = &num;
			uint value = *(ptr++) * *(ptr++);
			Console.WriteLine(value);
		}

		private unsafe static void Sample1(uint param)
		{
			uint* ptr = &param;
			uint value = *(ptr++) * *(ptr++);
			Console.WriteLine(value);
		}

		private unsafe static void Sample5()
		{
			uint num = 0u;
			uint* ptr = &num;
			uint value = (*ptr += 5u) * (*ptr += 5u);
			Console.WriteLine(value);
		}

		private unsafe static void Sample5(uint param)
		{
			uint* ptr = &param;
			uint value = (*ptr += 5u) * (*ptr += 5u);
			Console.WriteLine(value);
		}

		private unsafe static void Sample52()
		{
			uint num = 0u;
			uint* ptr = &num;
			uint value = *(ptr += 5) * ptr[5];
			Console.WriteLine(value);
		}

		private unsafe static void Sample52(uint param)
		{
			uint* ptr = &param;
			uint value = *(ptr += 5) * ptr[5];
			Console.WriteLine(value);
		}
	}
}

The old decompiler does a better job with the Mixed1 method as well as the Sample1 methods.

[ElektroKill]

Hello, I've gone ahead and extended the test cases. I've also concluded that there is no difference in decompilation results between pointers to locals and pointers to parameters.

Updated test cases:

unsafe partial class PointerCompoundAssignTests {
	static void ByteParameterReferencePostIncrementBeforeDereference(byte param) {
		byte* r = &param;
		int g = (*r++) * (*r++);
		Console.WriteLine(g);
	}

	static void ByteParameterReferencePostIncrementAfterDereference(byte param) {
		byte* r = &param;
		int g = ((*r)++) * ((*r)++);
		Console.WriteLine(g);
	}

	static void ByteParameterReferencePreIncrementBeforeDereference(byte param) {
		byte* r = &param;
		int g = (*++r) * (*++r);
		Console.WriteLine(g);
	}

	static void ByteParameterReferencePreIncrementAfterDereference(byte param) {
		byte* r = &param;
		int g = (++(*r)) * (++(*r));
		Console.WriteLine(g);
	}

	static void ByteParameterReferenceCompundAssignAfterDereference(byte param) {
		byte* r = &param;
		int g = (*r += 5) * (*r += 5);
		Console.WriteLine(g);
	}

	static void ByteParameterReferenceCompundAssignBeforeDereference(byte param) {
		byte* r = &param;
		int g = (*(r += 5)) * (*(r += 5));
		Console.WriteLine(g);
	}

	static void UIntParameterReferencePostIncrementBeforeDereference(uint param) {
		uint* r = &param;
		uint g = (*r++) * (*r++);
		Console.WriteLine(g);
	}

	static void UIntParameterReferencePostIncrementAfterDereference(uint param) {
		uint* r = &param;
		uint g = ((*r)++) * ((*r)++);
		Console.WriteLine(g);
	}

	static void UIntParameterReferencePreIncrementBeforeDereference(uint param) {
		uint* r = &param;
		uint g = (*++r) * (*++r);
		Console.WriteLine(g);
	}

	static void UIntParameterReferencePreIncrementAfterDereference(uint param) {
		uint* r = &param;
		uint g = (++(*r)) * (++(*r));
		Console.WriteLine(g);
	}

	static void UIntParameterReferenceCompundAssignAfterDereference(uint param) {
		uint* r = &param;
		uint g = (*r += 5) * (*r += 5);
		Console.WriteLine(g);
	}

	static void UIntParameterReferenceCompundAssignBeforeDereference(uint param) {
		uint* r = &param;
		uint g = (*(r += 5)) * (*(r += 5));
		Console.WriteLine(g);
	}
}

unsafe class MixedTests {
	static void Mixed1(uint[] y, uint l, uint* e) {
		uint h = 0;
		for (uint i = 0; i < l; i++) {
			*e ^= y[h & 0xf];
			y[h & 0xf] = (y[h & 0xf] ^ (*e++)) + 0x3dbb2819;
			h++;
		}
	}
}

Decompilation output for the test cases above:

With Always inline local variables if possible option disabled:

internal class PointerCompoundAssignTests
{
	private unsafe static void ByteParameterReferencePostIncrementBeforeDereference(byte param)
	{
		byte* r = &param;
		int g = *(r++) * *(r++);
		Console.WriteLine(g);
	}

	private unsafe static void ByteParameterReferencePostIncrementAfterDereference(byte param)
	{
		byte* r = &param;
		byte b = *r;
		*r = (byte)(b + 1);
		byte num = b;
		b = *r;
		*r = (byte)(b + 1);
		int g = num * b;
		Console.WriteLine(g);
	}

	private unsafe static void ByteParameterReferencePreIncrementBeforeDereference(byte param)
	{
		byte* r = &param;
		int g = *(++r) * *(++r);
		Console.WriteLine(g);
	}

	private unsafe static void ByteParameterReferencePreIncrementAfterDereference(byte param)
	{
		byte* r = &param;
		byte b = (byte)(*r + 1);
		*r = b;
		byte num = b;
		b = (byte)(*r + 1);
		*r = b;
		int g = num * b;
		Console.WriteLine(g);
	}

	private unsafe static void ByteParameterReferenceCompundAssignAfterDereference(byte param)
	{
		byte* r = &param;
		byte b;
		*r = (b = (byte)(*r + 5));
		byte num = b;
		*r = (b = (byte)(*r + 5));
		int g = num * b;
		Console.WriteLine(g);
	}

	private unsafe static void ByteParameterReferenceCompundAssignBeforeDereference(byte param)
	{
		byte* r = &param;
		int g = *(r += 5) * *(r += 5);
		Console.WriteLine(g);
	}

	private unsafe static void UIntParameterReferencePostIncrementBeforeDereference(uint param)
	{
		uint* r = &param;
		uint* intPtr = r;
		r = intPtr + 1;
		uint num = *intPtr;
		uint* intPtr2 = r;
		r = intPtr2 + 1;
		uint g = num * *intPtr2;
		Console.WriteLine(g);
	}

	private unsafe static void UIntParameterReferencePostIncrementAfterDereference(uint param)
	{
		uint* r = &param;
		uint g = (*r)++ * (*r)++;
		Console.WriteLine(g);
	}

	private unsafe static void UIntParameterReferencePreIncrementBeforeDereference(uint param)
	{
		uint* r = &param;
		uint g = *(++r) * *(++r);
		Console.WriteLine(g);
	}

	private unsafe static void UIntParameterReferencePreIncrementAfterDereference(uint param)
	{
		uint* r = &param;
		uint g = ++(*r) * ++(*r);
		Console.WriteLine(g);
	}

	private unsafe static void UIntParameterReferenceCompundAssignAfterDereference(uint param)
	{
		uint* r = &param;
		uint g = (*r += 5u) * (*r += 5u);
		Console.WriteLine(g);
	}

	private unsafe static void UIntParameterReferenceCompundAssignBeforeDereference(uint param)
	{
		uint* r = &param;
		uint g = *(r += 5) * *(r += 5);
		Console.WriteLine(g);
	}
}

internal class MixedTests
{
	private unsafe static void Mixed1(uint[] y, uint l, uint* e)
	{
		uint h = 0u;
		for (uint i = 0u; i < l; i++)
		{
			*e ^= y[h & 0xF];
			uint num = h & 0xF;
			uint num2 = y[h & 0xF];
			uint* intPtr = e;
			e = intPtr + 1;
			y[num] = (num2 ^ *intPtr) + 1035675673;
			h++;
		}
	}
}

With Always inline local variables if possible option enabled:

internal class PointerCompoundAssignTests
{
	private unsafe static void ByteParameterReferencePostIncrementBeforeDereference(byte param)
	{
		byte* intPtr = &param;
		byte* r = intPtr + 1;
		Console.WriteLine(*intPtr * *(r++));
	}

	private unsafe static void ByteParameterReferencePostIncrementAfterDereference(byte param)
	{
		byte* r = &param;
		byte b = *r;
		*r = (byte)(b + 1);
		byte num = b;
		b = *r;
		*r = (byte)(b + 1);
		Console.WriteLine(num * b);
	}

	private unsafe static void ByteParameterReferencePreIncrementBeforeDereference(byte param)
	{
		byte* intPtr = &param + 1;
		byte* r;
		Console.WriteLine(*intPtr * *(r = intPtr + 1));
	}

	private unsafe static void ByteParameterReferencePreIncrementAfterDereference(byte param)
	{
		byte* r = &param;
		byte b = (byte)(*r + 1);
		*r = b;
		byte num = b;
		b = (byte)(*r + 1);
		*r = b;
		Console.WriteLine(num * b);
	}

	private unsafe static void ByteParameterReferenceCompundAssignAfterDereference(byte param)
	{
		byte* r = &param;
		byte b;
		*r = (b = (byte)(*r + 5));
		byte num = b;
		*r = (b = (byte)(*r + 5));
		Console.WriteLine(num * b);
	}

	private unsafe static void ByteParameterReferenceCompundAssignBeforeDereference(byte param)
	{
		byte* intPtr = &param + 5;
		byte* r;
		Console.WriteLine(*intPtr * *(r = intPtr + 5));
	}

	private unsafe static void UIntParameterReferencePostIncrementBeforeDereference(uint param)
	{
		uint* intPtr = &param;
		uint* r = intPtr + 1;
		uint num = *intPtr;
		uint* intPtr2 = r;
		r = intPtr2 + 1;
		Console.WriteLine(num * *intPtr2);
	}

	private unsafe static void UIntParameterReferencePostIncrementAfterDereference(uint param)
	{
		uint* r = &param;
		Console.WriteLine((*r)++ * (*r)++);
	}

	private unsafe static void UIntParameterReferencePreIncrementBeforeDereference(uint param)
	{
		uint* intPtr = &param + 1;
		uint* r;
		Console.WriteLine(*intPtr * *(r = intPtr + 1));
	}

	private unsafe static void UIntParameterReferencePreIncrementAfterDereference(uint param)
	{
		uint* r = &param;
		Console.WriteLine(++(*r) * ++(*r));
	}

	private unsafe static void UIntParameterReferenceCompundAssignAfterDereference(uint param)
	{
		uint* r = &param;
		Console.WriteLine((*r += 5u) * (*r += 5u));
	}

	private unsafe static void UIntParameterReferenceCompundAssignBeforeDereference(uint param)
	{
		uint* intPtr = &param + 5;
		uint* r;
		Console.WriteLine(*intPtr * *(r = intPtr + 5));
	}
}

internal class MixedTests
{
	private unsafe static void Mixed1(uint[] y, uint l, uint* e)
	{
		uint h = 0u;
		for (uint i = 0u; i < l; i++)
		{
			*e ^= y[h & 0xF];
			uint num = h & 0xF;
			uint num2 = y[h & 0xF];
			uint* intPtr = e;
			e = intPtr + 1;
			y[num] = (num2 ^ *intPtr) + 1035675673;
			h++;
		}
	}
}

Summary of testing results:

The decompiler fails to produce a pretty result for MixedTests.Mixed1 no matter the local inlining option.
When the local inlining option is disabled, the following test cases yield "not pretty" results:

• ByteParameterReferencePostIncrementAfterDereference
• ByteParameterReferencePreIncrementAfterDereference
• ByteParameterReferenceCompundAssignAfterDereference
• UIntParameterReferencePostIncrementBeforeDereference

When the local inlining option is enabled, the following test cases yield "not pretty" results:

• ByteParameterReferencePostIncrementBeforeDereference
• ByteParameterReferencePostIncrementAfterDereference
• ByteParameterReferencePreIncrementBeforeDereference
• ByteParameterReferencePreIncrementAfterDereference
• ByteParameterReferenceCompundAssignAfterDereference
• ByteParameterReferenceCompundAssignBeforeDereference
• UIntParameterReferencePostIncrementBeforeDereference
• UIntParameterReferencePreIncrementBeforeDereference
• UIntParameterReferenceCompundAssignBeforeDereference

From these results, it is safe to assume that the Always inline local variables if possible negatively impacts the decompilation results when dealing with post/pre increments and compound assignments with pointers involved. Is this negative impact of Always inline local variables if possible intended behavior or is it a bug in the decompiler causing these inferior decompilation results?

[ElektroKill]

Is there a chance for this issue to be fixed in ILSpy 8?

[ElektroKill]

I came up with a potential fix for the UIntParameterReferencePostIncrementBeforeDereference test code not decompiling in into a post-increment.
The first if condition which contains a check which verifies whether the binary.Right instruction is ldc.i4 1 could be modified to use PointerArithmeticOffset.Detect to detect the proper operation for pointer compound assignments.

ILSpy/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs

Lines 872 to 884 in 3d117a5

	if (UnwrapSmallIntegerConv(value, out var conv) is BinaryNumericInstruction binary)
	{
	if (!binary.Left.MatchLdLoc(tmpVar) || !(binary.Right.MatchLdcI(1) || binary.Right.MatchLdcF4(1) || binary.Right.MatchLdcF8(1)))
	return false;
	if (!(binary.Operator == BinaryNumericOperator.Add || binary.Operator == BinaryNumericOperator.Sub))
	return false;
	if (!ValidateCompoundAssign(binary, conv, targetType, context.Settings))
	return false;
	context.Step("TransformPostIncDecOperator (builtin)", inst);
	finalizeMatch?.Invoke(context);
	inst.Value = new NumericCompoundAssign(binary, target, targetKind, binary.Right,
	targetType, CompoundEvalMode.EvaluatesToOldValue);
	}

If this is applied then the assertions in ExpressionBuilder.HandleCompoundAssignment would have to be modified as well.

ILSpy/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

Lines 1901 to 1920 in 3d117a5

	if (inst.EvalMode == CompoundEvalMode.EvaluatesToOldValue)
	{
	Debug.Assert(op == AssignmentOperatorType.Add || op == AssignmentOperatorType.Subtract);
	Debug.Assert(inst.Value.MatchLdcI(1) || inst.Value.MatchLdcF4(1) || inst.Value.MatchLdcF8(1));
	UnaryOperatorType unary;
	ExpressionType exprType;
	if (op == AssignmentOperatorType.Add)
	{
	unary = UnaryOperatorType.PostIncrement;
	exprType = ExpressionType.PostIncrementAssign;
	}
	else
	{
	unary = UnaryOperatorType.PostDecrement;
	exprType = ExpressionType.PostDecrementAssign;
	}
	resultExpr = new UnaryOperatorExpression(unary, target)
	.WithILInstruction(inst)
	.WithRR(new OperatorResolveResult(target.Type, exprType, target.ResolveResult));
	}

The second assertion would have to be modified to also use PointerArithmeticOffset when we are dealing with pointer post-increments.
Another solution would be to set the Value of the NumericCompoundAssign instruction to the output of PointerArithmeticOffset.Detect to avoid the need for modifying the assertions in ExpressionBuilder. Not sure which approach here is the best.

Please let me know if this is an appropriate fix for the issue with UIntParameterReferencePostIncrementBeforeDereference. If it is I can open a PR with these changes.

[dgrunwald]

The first approach makes sense.

NumericCompoundAssign is (like most of the ILAst) supposed to have IL semantics, so it doesn't make sense to use the number of elements there.

[ElektroKill]

Hi, I have identified the cause for the ByteParameterReferencePostIncrementAfterDereference, ByteParameterReferencePreIncrementAfterDereference, and ByteParameterReferenceCompundAssignAfterDereference test methods not decompiling nicely.

The issue is the checks performed in the IsImplicitTruncation method and ValidateCompoundAssign methods and the code generated by the compiler for these methods.
These methods perform a check based on the primitive type of instruction in these places:

ILSpy/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs

Lines 543 to 546 in 4ebe075

	else if (value is Conv conv)
	{
	return conv.TargetType != type.ToPrimitiveType();
	}

ILSpy/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs

Lines 576 to 579 in 4ebe075

	if (inferredType.Kind != TypeKind.Unknown)
	{
	return !(inferredType.GetSize() <= type.GetSize() && inferredType.GetSign() == type.GetSign());
	}

ILSpy/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs

Lines 235 to 236 in 4ebe075

	if (conv != null && !(conv.TargetType == targetType.ToPrimitiveType() && conv.CheckForOverflow == binary.CheckForOverflow))
	return false; // conv does not match binary operation

This direct comparison is problematic if we look at the code generated by the C# compiler for these methods.

In the raw IL it is possible to observe a mismatch between primitive types which causes the check to fail. This mismatch is caused by the fact that IL does not have unsigned variations of stind. This issue with the checks cannot be observed on UInt32 compound assignments as it is not a small integer type and is thus excluded from the IsImplcitTruncation checks. The compound assignments on UInt32 also do not include a conv so the condition in ValidateCompoundAssign is skipped. This means that this problem not only occurs with byte but with any unsigned small integer type and char.

ILSpy/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs

Lines 516 to 522 in 4ebe075

	if (!type.IsSmallIntegerType())
	{
	// Implicit truncation in ILAst only happens for small integer types;
	// other types of implicit truncation in IL cause the ILReader to insert
	// conv instructions.
	return false;
	}

This problem is the reason for the ugly decompilation on all 3 test methods mentioned at the start of the comment due to the mismatch in primitive types.

[ElektroKill]

A potential fix for the problem identified above:

Change conditions to only compare type size and not whether it is signed or unsigned:

1. ILSpy/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs

   Lines 543 to 546 in 4ebe075

   	else if (value is Conv conv)
   	{
   	return conv.TargetType != type.ToPrimitiveType();
   	}

   
   Changed return statement to:

   return conv.TargetType.GetSize() != type.ToPrimitiveType().GetSize();

2. ILSpy/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs

   Lines 576 to 579 in 4ebe075

   	if (inferredType.Kind != TypeKind.Unknown)
   	{
   	return !(inferredType.GetSize() <= type.GetSize() && inferredType.GetSign() == type.GetSign());
   	}

   
   Change return statement to:

   return !(inferredType.GetSize() <= type.GetSize());

3. ILSpy/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs

   Lines 235 to 236 in 4ebe075

   	if (conv != null && !(conv.TargetType == targetType.ToPrimitiveType() && conv.CheckForOverflow == binary.CheckForOverflow))
   	return false; // conv does not match binary operation

   
   Change if statement to:

   if (conv != null && !(conv.TargetType.GetSize() == targetType.ToPrimitiveType().GetSize() && conv.CheckForOverflow == binary.CheckForOverflow))
   	return false; // conv does not match binary operation

All tests pass with these changes applied.

Not sure if this is the right way to approach this issue. I'm open to feedback on my suggestion. If this fix is appropriate I can open a pull request with it and some test cases for it.

[ElektroKill]

I'm still awaiting feedback on my proposed fix for the issue :p

[dgrunwald]

1+2: I don't think those suggestions are correct.
IsImplicitTruncation should return true only if stobj type(..., value) evaluates to the same value as value.
Note that if type is a small integer, stobj will truncate value, then store it, then widen again back to I4 based on the signedness of type.
stobj System.Byte(..., conv i1(ldc.i4 130)) will evaluate to the I4 130, but the input value (the conv) evaluates to the I4 -126 instead. So the function must return false.

[dgrunwald]

On the IL level, stobj only has a single form for both signed and unsigned bytes, and that form only truncates and doesn't evaluate to any result at all.
But in order to support it as an inline expression, we've extended the semantics, adding the re-widening, which matches how C# handles an assignment expression: call F(stobj sbyte(v, ldc.i4 130)) will turn into F(v = -126), so we can't use that if the original IL called F(130).

Removing the sign comparison would make the transform incorrect. If the transform notices that only a sign mismatch prevents it from working, the transform could change the type of the stobj (changing only the sign has no effect if the result of the stobj isn't used yet), and then successfully apply the transformation. But without changing the type of the stobj, your proposal isn't correct.

[ElektroKill]

Would the following approach be valid?

1. Leave IsImplicitTruncation like it is called now but extend it to return more information about the truncation.
2. Call it the first time with the same parameters as currently done
3. If the only reason for the function returning true is a mismatch in the sign of the type, change the stobj type to the opposite sign type and continue as if it was not a truncation.

[dgrunwald]

Yes, that approach should work.