Skip to content
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

short.Parse regression in .NET 8.0 from 7.0 #94971

Closed
duracellko opened this issue Nov 19, 2023 · 6 comments · Fixed by #106506
Closed

short.Parse regression in .NET 8.0 from 7.0 #94971

duracellko opened this issue Nov 19, 2023 · 6 comments · Fixed by #106506
Assignees
Labels
area-System.Numerics in-pr There is an active PR which will close this issue when it is merged regression-from-last-release
Milestone

Comments

@duracellko
Copy link

duracellko commented Nov 19, 2023

Description

Methods Int16.Parse and Int16.TryParse produce different result in .NET 7 and 8, when the input is decimal number. In .NET 7 numbers with decimal point are considered invalid and the methods returns error. However, in .NET 8 the methods return integer number, when input has more than 4 digits.

Reproduction Steps

Example:

short.TryParse("10000.1", NumberStyles.Number, CultureInfo.InvariantCulture, out var number);

Expected behavior

The function should return false in both .NET versions.

Actual behavior

The function returns true in .NET 8 and false in .NET 7.

Regression?

Yes, as described above.

Known Workarounds

I found the issue only in Int16. Int32 works the same in .NET 8 and 7.

Configuration

dotnet --info

.NET SDK:
Version: 8.0.100
Commit: 57efcf1350
Workload version: 8.0.100-manifests.8d38d0cc

Runtime Environment:
OS Name: Windows
OS Version: 10.0.22631
OS Platform: Windows
RID: win-x64
Base Path: C:\Program Files\dotnet\sdk\8.0.100\

.NET workloads installed:
Workload version: 8.0.100-manifests.8d38d0cc
[maui-windows]
Installation Source: VS 17.8.34309.116
Manifest Version: 8.0.3/8.0.100
Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.maui\8.0.3\WorkloadManifest.json
Install Type: Msi

[maccatalyst]
Installation Source: VS 17.8.34309.116
Manifest Version: 17.0.8478/8.0.100
Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.maccatalyst\17.0.8478\WorkloadManifest.json
Install Type: Msi

[ios]
Installation Source: VS 17.8.34309.116
Manifest Version: 17.0.8478/8.0.100
Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.ios\17.0.8478\WorkloadManifest.json
Install Type: Msi

[android]
Installation Source: VS 17.8.34309.116
Manifest Version: 34.0.43/8.0.100
Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.android\34.0.43\WorkloadManifest.json
Install Type: Msi

Host:
Version: 8.0.0
Architecture: x64
Commit: 5535e31

.NET SDKs installed:
8.0.100 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
Microsoft.AspNetCore.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
x86 [C:\Program Files (x86)\dotnet]
registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
Not set

global.json file:
Not found

Learn more:
https://aka.ms/dotnet/info

Download .NET:
https://aka.ms/dotnet/download

Other information

The problem appears only for values 10000 and higher in absolute value.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 19, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 19, 2023
@duracellko
Copy link
Author

I created small test project for demonstration.

image

All .NET 7 tests are green, but 50% of .NET 8 tests are red.

Project: ParseTests.zip

@karakasa
Copy link
Contributor

just FYI, you wrote Expected & Actual Behavior reversally.

@duracellko
Copy link
Author

just FYI, you wrote Expected & Actual Behavior reversally.

Thank you. I corrected it.

@karakasa
Copy link
Contributor

karakasa commented Nov 19, 2023

There may be another branch here to fail when digCount >= maxDigCount, ch != 0 and target type is integer .

if (digCount < maxDigCount)
{
number.Digits[digCount] = (byte)ch;
if ((ch != '0') || (number.Kind != NumberBufferKind.Integer))
{
digEnd = digCount + 1;
}
}
else if (ch != '0')
{
// For decimal and binary floating-point numbers, we only
// need to store digits up to maxDigCount. However, we still
// need to keep track of whether any additional digits past
// maxDigCount were non-zero, as that can impact rounding
// for an input that falls evenly between two representable
// results.
number.HasNonZeroTail = true;
}

int.TryParse("2147483646.1", NumberStyles.Number, CultureInfo.InvariantCulture, out var number) also returns true.

I'm unsure if it's a bug, UB or breaking change since you are using NumberStyles.Number. Anyway, the behavior is undocumented afaik and 10000.1/1000.1 have inconsistent results.

@stephentoub
Copy link
Member

cc: @tannergooding

@jkotas jkotas added area-System.Numerics and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 19, 2023
@ghost
Copy link

ghost commented Nov 19, 2023

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Methods Int16.Parse and Int16.TryParse produce different result in .NET 7 and 8, when the input is decimal number. In .NET 7 numbers with decimal point are considered invalid and the methods returns error. However, in .NET 8 the methods return integer number, when input has more than 4 digits.

Reproduction Steps

Example:

short.TryParse("10000.1", NumberStyles.Number, CultureInfo.InvariantCulture, out var number);

Expected behavior

The function should return false in both .NET versions.

Actual behavior

The function returns true in .NET 8 and false in .NET 7.

Regression?

Yes, as described above.

Known Workarounds

I found the issue only in Int16. Int32 works the same in .NET 8 and 7.

Configuration

dotnet --info

.NET SDK:
Version: 8.0.100
Commit: 57efcf1350
Workload version: 8.0.100-manifests.8d38d0cc

Runtime Environment:
OS Name: Windows
OS Version: 10.0.22631
OS Platform: Windows
RID: win-x64
Base Path: C:\Program Files\dotnet\sdk\8.0.100\

.NET workloads installed:
Workload version: 8.0.100-manifests.8d38d0cc
[maui-windows]
Installation Source: VS 17.8.34309.116
Manifest Version: 8.0.3/8.0.100
Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.maui\8.0.3\WorkloadManifest.json
Install Type: Msi

[maccatalyst]
Installation Source: VS 17.8.34309.116
Manifest Version: 17.0.8478/8.0.100
Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.maccatalyst\17.0.8478\WorkloadManifest.json
Install Type: Msi

[ios]
Installation Source: VS 17.8.34309.116
Manifest Version: 17.0.8478/8.0.100
Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.ios\17.0.8478\WorkloadManifest.json
Install Type: Msi

[android]
Installation Source: VS 17.8.34309.116
Manifest Version: 34.0.43/8.0.100
Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.android\34.0.43\WorkloadManifest.json
Install Type: Msi

Host:
Version: 8.0.0
Architecture: x64
Commit: 5535e31

.NET SDKs installed:
8.0.100 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
Microsoft.AspNetCore.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
x86 [C:\Program Files (x86)\dotnet]
registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
Not set

global.json file:
Not found

Learn more:
https://aka.ms/dotnet/info

Download .NET:
https://aka.ms/dotnet/download

Other information

The problem appears only for values 10000 and higher in absolute value.

Author: duracellko
Assignees: -
Labels:

area-System.Numerics, untriaged, needs-area-label

Milestone: -

@jeffhandley jeffhandley added this to the 9.0.0 milestone Jul 19, 2024
@jeffhandley jeffhandley added regression-from-last-release needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed untriaged New issue has not been triaged by the area owner labels Jul 19, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Aug 15, 2024
@jeffhandley jeffhandley removed the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Aug 20, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Numerics in-pr There is an active PR which will close this issue when it is merged regression-from-last-release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants