VB -> C#: Converter ignores the three-valued logic for nullable types and relational operators.

[Yozer]

VB.Net input code

Public Sub Main()
    Dim x As Integer? = Nothing
    If x <> 10 Then
        Console.WriteLine("Fail!")
    Else
        Console.WriteLine("Ok!")
    End If
End Sub

This will print OK because: x <> 10 results in Nothing with Boolean? type.
Nothing is False so code will execute Else statement.

Erroneous output

public static void Main()
{
    int? x = default;
    if (x != 10 == true) {
        Console.WriteLine("Fail!");
    }
    else {
        Console.WriteLine("Ok!");
    }
}

Here the code will incorrectly print Fail because: x != 10 results in true and true is equal to true.

Expected output

  public static void Main()
  {
      int? x = default;
      if ((x == null ? null : x != 10) == true) {
          Console.WriteLine("Fail!");
      }
      else {
          Console.WriteLine("Ok!");
      }
  }

VB uses three-valued logic for nullable types. Any relational comparison (=, <>, >, >=, <, <=) that involves nullable types results in Boolean?.
If lhs or rhs is null then the entire expression will be null.
The expected output produces ugly code but I'm not sure if it's possible to maintain the same logic using something simpler as we need to keep bool? as the result of the comparison.
I will take a look if there is something in Microsoft.VisualBasic.dll that could simplify the code.

Comparing Nullable Types

Details

• Product in use: VS extension
• Version in use: b5e798b
• Did you see it working in a previous version, which?
• Any other relevant information to the issue, or your interest in contributing a fix.

[GrahamTheCoder]

The converter's actually not totally ignoring it by the way, there is some code and a few self verifying tests for dealing with this.
However my instinct was definitely wrong for what that VB code would do, so either I'd forgotten or never knew about that case which probably means the current implementation needs rethinking.

https://github.com/icsharpcode/CodeConverter/blob/master/Tests/TestData/SelfVerifyingTests/VBToCS/BooleanTests.vb

[Yozer]

Thanks, good to know that there is some code for nullable booleans.
I will definitely add more self-verifying tests for them and check if the current logic can be tweaked or new implementation is needed.

[GrahamTheCoder]

Yep it definitely needs a bunch more tests than it has!
I think in the general case you'd need to expand to x.HasValue ? x != 10 : null for the operator (with some consideration for whether x itself is an expression that shouldn't be reevaluated).
Then within the if statement, add the == true like now.
Obviously for a single operator condition you can reduce that down.

For the re-evaluatable expression part, there is some code that can help extract temp variables, but you might be better off trying to use some syntax that introduces a variable in scope along the lines of x is {} xVal ? xVal != 10 : null

So for this case you'd end up with x is {} xVal && xVal != 10 I think...which still isn't exactly pretty

Note: Because the decompilation doesn't show a call to the VB library I assumed (there I go again assuming) that there wasn't one, but there may well be.

Is the problem here only with the <> operator, or other things too?

[GrahamTheCoder]

If it's just that one operator, then we could add an extension method so we can use it like x.LessThanOrGreaterThan(10) (would be easier with the IComparisonOperators interface perhaps)
Or we might be able to use the VB built-in ones:
https://docs.microsoft.com/en-us/dotnet/api/microsoft.visualbasic.compilerservices.operators.compareobjectnotequal?view=net-6.0
https://docs.microsoft.com/en-us/dotnet/api/microsoft.visualbasic.compilerservices.operators.conditionalcompareobjectnotequal?view=net-6.0

This is a place they're already used: https://github.com/icsharpcode/CodeConverter/blob/master/CodeConverter/CSharp/BuiltInVisualBasicOperatorSubstitutions.cs#L253-L271

[Yozer]

Surprisingly the VB methods from Operators don't behave in the same way:

Dim b1 as Boolean? = Nothing
Dim b2 as Boolean? = True
Dim test1 = CompilerServices.Operators.CompareObjectEqual(b1, b2, False) ' False
Dim test2 = b1 = b2 ' Nothing
Dim test3 = CompilerServices.Operators.CompareObjectNotEqual(b1, b2, False) ' True
Dim test4 = b1 <> b2 ' Nothing

I'm afraid that all comparison operators are affected: <, >, <=, >=, =, <>:

Dim a As Integer? = Nothing
Dim b As Integer? = Nothing

If a = b Then Console.WriteLine("1") Else Console.WriteLine("2") ' In VB this will print 2 but after conversion 1

Looks like And, Or, AndAlso, OrElse will also behave differently in some cases.
Especially AndAlso and OrElse:

Public Function GetMeTheValue(x As Integer) As Boolean?
    If x = 1 Then Throw New Exception() Else Return Nothing
End Function

Public Sub Main()
    Dim result = GetMeTheValue(0) AndAlso GetMeTheValue(1)
End Sub

Above will throw an exception in VB but not in C# (after conversion).
AndAlso and OrElse, which use short-circuit evaluation, must evaluate their second operands when the first evaluates to Nothing.

I will start with writing some self-verifying tests and then try to generate something that will yield the same (or almost the same) IL.
From what I saw in my VB code it's not super common to run into these cases. Since is not that common then the converter could generate #error for cases that require additional variables.

[GrahamTheCoder]

Ah yes, I'd spotted in the decompilation that & was used rather than &&. Makes sense now

[GrahamTheCoder]

Yes, the error approach may make sense as a start or we could even just add a warning pragma and something that throws if a relevant input is null. The extreme of that approach would be to turn on nullable reference types (assuming it affects those types) and mark them as non null, which would also be a cool approach for string comparison. Likely a lot of work and probably will confuse people anyway though.

Interesting that the object comparison methods work differently, does that mean obj1 < obj2 never returns null? Or just that those methods are called after some other compiler generated checks?

[Yozer]

From what I read that tri-valued logic is only happening for operations involving Nullable<> types.
does that mean obj1 < obj2 never returns null - that's my understanding. The Shared Operator <() As Boolean will be called and depending on the logic it will return Boolean or throw an exception.

[Yozer]

Dim a As Boolean?
Dim b As Boolean?
Dim x = Operators.CompareObjectEqual(a, b, False) ' True 
Dim y = Operators.ConditionalCompareObjectEqual(a, b, False) ' True
Dim z = a = b 'Nothing

Nullables are value types so VB will box them before calling the methods:
Sharplab.io