Fix Double-to-Long overflow check

[MichaelJLiu]

Fixes #73032.

This PR addresses a bug in the VB.NET compiler.

Description

Any constant or literal Double should be convertible at compile time to Long if its value is within the range of the Long data type (approximately ±9.223E+18). However, if the value is a large positive number between &H7000000000000400L and &H7FFFFFFFFFFFFC00L, then the error "BC30439: Constant expression not representable in type 'Long'." occurs:

Const w As Long = CType(9.0E+18, Long)
'                       ~~~~~~~ BC30439
Const x As Long = &H7000000000000400L
Const y As Double = x
Const z As Long = CType(y, Long)
'                       ~ BC30439

In contrast, large negative numbers are unaffected:

Const w As Long = CType(-9.0E+18, Long) ' OK

Const x As Long = Long.MinValue
Const y As Double = x
Const z As Long = CType(y, Long) ' OK

Root Cause

Lines 603-609 of the CompileTimeCalculations.DetectFloatingToIntegralOverflow method check whether the input value is between &H7000000000000000L and &H8000000000000000UL, exclusive:

If sourceValue > &H7000000000000000L Then
    Dim temporary As Double = (sourceValue - &H7000000000000000L)

    If temporary < &H7000000000000000L AndAlso UncheckedCLng(temporary) > &H1000000000000000L Then
        Return False
    End If
Else
    Return False
End If

On line 606, however, the comparison UncheckedCLng(temporary) > &H1000000000000000L is incorrect; it should use < instead. This can be confirmed in two ways:

1. In the isUnsigned branch earlier in this same method, the analogous comparison uses the correct operator:

   Dim temporary As Double = (sourceValue - &HF000000000000000UL)
   
   If temporary < &H7000000000000000L AndAlso UncheckedCLng(temporary) < &H1000000000000000L Then
       Return False
   End If

2. Comments in this method reference the Shared Source CLI source file fjitdef.h, from which this code was presumably adapted. There, the comparison also uses the correct operator:

   if (val > I64(0x7000000000000000)) {
       double tmp = val - I64(0x7000000000000000);
       if (tmp < I64(0x7000000000000000) &&
           (__int64)tmp < I64(0x1000000000000000)) {
           goto success;
       }
   }
   else {
       goto success;
   }

Unit Tests

I added test cases for the minimum and maximum Double values that are convertible to Long: CDbl(&H8000000000000000L) (which has always worked) and CDbl(&H7FFFFFFFFFFFFC00L) (which was previously broken).

The original test case CDbl(&H70000000000000F0L) failed to catch this bug because CDbl rounded it down to &H7000000000000000L.

[AlekseyTs]

@MichaelJLiu Could you please open an issue and link this PR to it? Also, please include information whether the issue is present in the legacy (non-Roslyn) VB compiler. It is a part of the .NET Framework for Desktop and, usually, can be found as c:\Windows\Microsoft.NET\Framework\v4.0.30319\vbc.exe

[MichaelJLiu]

@AlekseyTs I've opened issue #73032 as requested, but I don't have permission to link it to this PR.

[AlekseyTs]

but I don't have permission to link it to this PR.

You already did by mentioning the issue by its number in your comment.

[cston]

Should these cases be added to ConstantExpressionConversions2 as well, at line 1169?

[cston]

@MichaelJLiu, thanks for the detailed description.

However, if the value is a large positive number between &H7000000000000400L and &H7FFFFFFFFFFFFC00L, then the error "BC30439: Constant expression not representable in type 'Long'." occurs:

For reference, the corresponding C# code succeeds (see sharplab.io):

const long w = (long)9.0E+18; // ok
const long x = 0x7000000000000400L;
const double y = x;
const long z = (long)y; // ok

[cston]

&H70000000000000F0L

Would it make sense to use the other value from the description, &H7000000000000400L, instead of this case?

[cston]

                }

Perhaps we should also have a separate specific test for #73032, that compiles and runs the code from that issue and checks the expected values:

Console.WriteLine(CType(9.0E+18, Long))
Console.WriteLine(CType(CDbl(&H7000000000000400L), Long))
Console.WriteLine(CType(CDbl(&H7FFFFFFFFFFFFC00L), Long))

If so, please include a <WorkItem()> attribute on that test that references the issue.

---

Refers to: src/Compilers/VisualBasic/Test/Semantic/Semantics/Conversions.vb:614 in e8946b3. [](commit_id = e8946b3, deletion_comment = False)

[cston]

Thanks @MichaelJLiu. Please avoid using force-push while the PR is being reviewed since it makes it difficult to see the differences between commits. We'll use "squash and merge" when merging.

[cston]

@AlekseyTs, @dotnet/roslyn-compiler, please review.

[AlekseyTs]

New TypeAndValue(doubleType, CDbl(&H70000000000000F0L)),

Please keep this value #Closed

[MichaelJLiu]

I don't think we should keep CDbl(&H70000000000000F0L) because it's misleading: the integer &H70000000000000F0L can't be represented exactly by a Double. As I noted in the PR description, CDbl rounds it down to &H7000000000000000L, which is why it failed to catch this bug.

[AlekseyTs]

Thanks for the explanation. Sounds reasonable.

[AlekseyTs]

New TypeAndValue(doubleType, CDbl(&H70000000000000F0L)),

Please keep this value #Closed

[AlekseyTs]

System.Console.WriteLine(CType(CDbl(&H8000000000000000L), Long))

Instead of outputting the values, I think we should compare them with result of conversions performed at runtime. For example it is not obvious to me that the values in the output are correct. I think the overflow check for VB is set to true by default, but it would be good to set this option explicitly (WithOverflowChecks). #Closed

[MichaelJLiu]

In each case, the Long argument to CDbl is the expected result of calling CType. So I could write

System.Console.WriteLine(CType(CDbl(&H8000000000000000L), Long) = &H8000000000000000L)

and verify that the output is True. Is that acceptable?

[AlekseyTs]

This is going to rely on compile time constant folding for equality, which would be desirable to avoid. The goal is to compare result of compile time constant folding with result that doesn't rely on constant folding. I have something like the following in mind:

 System.Console.WriteLine(CType(CDbl(&H8000000000000000L), Long) = ConvertToLong(CDbl(&H8000000000000000L)))


Function ConvertToLong(x as Double) As Long
    Return CType(x, Long)
End Function

[AlekseyTs]

Done with review pass (commit 1)

[AlekseyTs]

LGTM (commit 2)

[cston]

Thanks @MichaelJLiu!