Skip to content

Conversation

@tannergooding
Copy link
Member

@tannergooding tannergooding commented Apr 13, 2022

Still pending are the equivalent APIs for getting the exponent/significand for floating-point numbers and the Create* API changes, will be in follow up PRs to help keep size down

@ghost ghost assigned tannergooding Apr 13, 2022
@ghost
Copy link

ghost commented Apr 13, 2022

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Apr 13, 2022

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Apr 13, 2022

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

Issue Details

null

Author: tannergooding
Assignees: tannergooding
Labels:

area-System.Numerics, new-api-needs-documentation

Milestone: -

@tannergooding
Copy link
Member Author

CC. @lewing @BrzVlad

This is triggering a mono-runtime assert for wasm and impacts the generic math API surface we want to preview at build.

Copy link
Contributor

@dakersnar dakersnar left a comment

Choose a reason for hiding this comment

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

Looks good!

@tannergooding
Copy link
Member Author

Worked around some test failures (and logged corresponding issues). Also fixed an error where these APIs shouldn't be on float/double/decimal/Half. The floating-point types are getting separate APIs to get the exponent and significand instead, so that the value can be understood.

@tannergooding tannergooding added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Apr 14, 2022
@tannergooding tannergooding force-pushed the generic-math branch 3 times, most recently from d5942ee to 5077eb9 Compare April 14, 2022 06:36
@tannergooding
Copy link
Member Author

No changes, just squashed commits together and rebased ontop of #68043 now that it's merged.

This is now "just" the GetShortestBitLength, GetByteCount, and WriteLittleEndian APIs + relevant tests and doesn't include the WASM disablement since #67999 repros even with them disabled

@tannergooding
Copy link
Member Author

Merge with main to pickup #68058

@tannergooding tannergooding merged commit c01a512 into dotnet:main Apr 15, 2022
@tannergooding tannergooding removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Apr 15, 2022
@ghost ghost locked as resolved and limited conversation to collaborators May 15, 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.

3 participants