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

Fix TryParseBigIntegerHexOrBinaryNumberStyle #97995

Merged
merged 4 commits into from
Feb 28, 2024

Conversation

kzrnm
Copy link
Contributor

@kzrnm kzrnm commented Feb 5, 2024

Since the - operator has higher precedence than the ^ operator, leading ^ signBits - signBits is always equal to leading.

Related #95543

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Feb 5, 2024
@ghost
Copy link

ghost commented Feb 5, 2024

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

Issue Details

Since the - operator has higher precedence than the ^ operator, leading ^ signBits - signBits is always equal to leading.

Author: kzrnm
Assignees: -
Labels:

area-System.Numerics, community-contribution

Milestone: -

Copy link
Member

@tannergooding tannergooding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Is there an explicit test we can add to cover this?

@kzrnm
Copy link
Contributor Author

kzrnm commented Feb 5, 2024

I think the test for values whose absolute value is greater than 0x7FFFFFFF and less than 0x100000000 can cover this.
I added those tests to Hex/Bin.

cd src\libraries\System.Runtime.Numerics\tests

# Passed
dotnet build /t:Test /p:XUnitOptions="-class System.Numerics.Tests.parseTest" 

# Failed
git checkout main ../src/System/Number.BigInteger.cs
dotnet build /t:Test /p:XUnitOptions="-class System.Numerics.Tests.parseTest"  

@huoyaoyuan
Copy link
Member

I'd like to add tests for non-edge cases that fall into this branch, both even and odd numbers.

Thank you again for discovering these! I'm really busy to complete things recently.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Numerics community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants