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 System.Number.BigInteger.DivRem outputting an incorrect remainder #72115

Merged
merged 1 commit into from
Jul 22, 2022

Conversation

dakersnar
Copy link
Contributor

@dakersnar dakersnar commented Jul 13, 2022

Fixes #72113

@ghost ghost assigned dakersnar Jul 13, 2022
@EgorBo
Copy link
Member

EgorBo commented Jul 13, 2022

Perhaps, worth having a unit test?

@dakersnar
Copy link
Contributor Author

@EgorBo Can do, let me try to figure out InternalsVisableTo

@stephentoub
Copy link
Member

let me try to figure out InternalsVisableTo

We try to avoid InternalsVisibleTo, for a variety of reasons, e.g. it inhibits assembly-level trimming we do.

Why does this require internals access? If there's no way to trigger this via public surface area, I think it's fine to forego a test.

@dakersnar
Copy link
Contributor Author

@stephentoub That makes sense. At the very least, once I merge the double to decimal conversion code, that will include a test that hits this code path. Do you think that's sufficient?

@stephentoub
Copy link
Member

If there's no other way to trigger this bug via public surface area, sounds fine.

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dakersnar
Copy link
Contributor Author

/azp run runtime

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ghost
Copy link

ghost commented Jul 15, 2022

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

Issue Details

Fixes #72113

Author: dakersnar
Assignees: dakersnar
Labels:

area-System.Numerics

Milestone: -

@dakersnar
Copy link
Contributor Author

@jeffhandley I think we discussed wanting to get this into 7.0. What does that process look like at this point?

@dakersnar dakersnar merged commit bca7f94 into dotnet:main Jul 22, 2022
@stephentoub
Copy link
Member

stephentoub commented Jul 22, 2022

I think we discussed wanting to get this into 7.0. What does that process look like at this point?

Everything going into main right now will make it into .NET 7. No additional process required (for a few more weeks).

@ghost ghost locked as resolved and limited conversation to collaborators Aug 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System.Number.BigInteger.DivRem can output an incorrect remainder
3 participants