Problem with nullable types

[lambertlb]

As i mentioned in a previous thread, I want to contribute. I have started a comprehensive test project where i am going through every C# feature and creating tests. I have a ways to go but have found an issue dealing with nullable types. Following are a couple of examples extracted from my tests. I will continue making more tests by examining the C# specs so let me know if there is anything i can do to help.

Original code:
private void UInt64NullableTests()
{
UInt64? value = 0;
LocalAssert(~value == UInt64.MaxValue);
}

private void UInt32NullableTests()
{
UInt32? value = 0;
LocalAssert(~value == UInt32.MaxValue);
}

Converted code:
private void UInt64NullableTests()
{
LocalAssert(~new ulong?(0uL) == (ulong?)(-1));
}

private void UInt32NullableTests()
{
LocalAssert(~new uint?(0u) == (uint?)(-1));
}

[siegfriedpammer]

I want to contribute. I have started a comprehensive test project where i am going through every C# feature and creating tests.

That is great news! We are really looking forward to a more complete test suite.

See #829 for the list of features currently supported by the decompiler.
If you want to integrate these tests into our build, see Unit Tests in the wiki. (Note: the part about checking in the generated IL files is obsolete. I will update the info soon.)

Thank you very much for your help!

[siegfriedpammer]

@lambertlb In the above code, what would be the expected behavior?

Using local variables in these test cases might not be a good idea, because they will get eliminated by the compiler most of the time.

I am not quite sure, whether it would be better to use uint.MaxValue instead of -1 (which is 0xFFFFFFFF ofc). The cast could be omitted in any case.

[lambertlb]

The issue seems to be only with unsigned nullable types uint and ulong. The translator works just fine with other types. Following is a small test to illustrate.
Original code

static void Main(string[] args)
{
    UInt32? value = 0;
    Debug.Assert(~value == UInt32.MaxValue);
    UInt64? value2 = 0;
    Debug.Assert(~value2 == UInt64.MaxValue);
    UInt16? value3 = 0;
    Debug.Assert((UInt16)~value3 == (UInt16)UInt16.MaxValue);
    UInt32 value4 = 0;
    Debug.Assert(~value4 == UInt32.MaxValue);
    UInt64 value5 = 0;
    Debug.Assert(~value5 == UInt64.MaxValue);
    UInt16 value6 = 0;
    Debug.Assert((UInt16)~value6 == UInt16.MaxValue);
}

Translated code

private static void Main(string[] args)
{
    Debug.Assert(~new uint?(0u) == (uint?)(-1));
    Debug.Assert(~new ulong?(0uL) == (ulong?)(-1));
    Debug.Assert((ushort)(~new ushort?(0)).Value == ushort.MaxValue);
    uint value8 = 0u;
    Debug.Assert(~value8 == uint.MaxValue);
    ulong value7 = 0uL;
    Debug.Assert(~value7 == ulong.MaxValue);
    ushort value6 = 0;
    Debug.Assert((ushort)(~value6) == ushort.MaxValue);
}

The error above is for trying to convert -1 to uint?.
Note UInt16 did work properly.
Even though the local variable was eliminated it still would work if the translator would have used MaxValue instead of -1 as it did for the tests that passed as follows

private static void Main(string[] args)
{
    Debug.Assert(~new uint?(0u) == uint.MaxValue));
    Debug.Assert(~new ulong?(0uL) == ulong.MaxValue);

I have been trying to debug this but i get lost on the recursive plunge translations. I have used recursive plunge algorithms in the past to make compilers but they tend to get hard to follow. I normally use table lookup algorithms now.

[dgrunwald]

To understand the recursion in ExpressionBuilder, make sure to read the comment on the class.
It usually isn't difficult to understand which node's translation violates the post-condition, so then one can focus on that single translation and ignore all other recursion levels.

Looking at the translation of the == operator, the left hand side is converted correctly:
inst.Left = {bit.not.lifted(newobj Nullable..ctor(ldc.i4 0)) at IL_0014}
=> left = {~new uint? (0u)} : {[OperatorResolveResult System.Nullable1[[System.UInt32]]]}`

And so is the right hand side:
inst.Right = {ldc.i4 -1 at IL_0039}
=> right = {-1} : {[ConstantResolveResult System.Int32 = -1]}

So the problem is with the == operator itself.

ExpressionBuilder.TranslateCeq first tries the obvious ~new uint? (0u) == -1, but correctly detects that such a comparison would be 64-bits whereas the IL calls for a 32-bit comparison.
So it decides to convert both sides to type uint? in order to get the correct comparison.
The actual problem then is that the call right.ConvertTo(targetType, this) does not manage to correctly convert the value.

A cast (uint)-1 is valid and evaluates to the desired value; so I find it a bit weird that the C# compiler does not accept the cast (uint?)-1.

[dgrunwald]

Ah actually (uint)-1 and (uint?)1 are both equally valid: only within an explicitly unchecked context.
ILSpy marks the cast as "needs to occur in an unchecked context", but then thinks it's sufficient that the top-level context is implicitly unchecked.

This problem only occurs for nullables, because ILSpy never emits a (uint)-1 cast -- instead it detects this cast to result in a compile-time constant, and directly emits the resulting constant (uint.MaxValue) instead. But this doesn't work here because the C# spec explicitly excludes nullables from the definition of compile-time constants.

[dgrunwald]

Normal int -> uint? casts are also fine in an implicitly unchecked context; it's just in the special case of a compile-time constant where the C# compiler requires an explicitly unchecked context.
But compile-time constant integers cannot be null, so we could just convert with an intermediate step int -> uint -> uint?. The first half of that chain would be a constant conversion, so the result would be (uint?)uint.MaxValue.

It's also possible to improve TranslateCeq to only cast to uint instead of uint?, but on its own that change would only hide the bug in ConvertTo().