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

Add UIToA<uint64_t> #337

Merged
merged 2 commits into from
Dec 2, 2024
Merged

Add UIToA<uint64_t> #337

merged 2 commits into from
Dec 2, 2024

Conversation

chiphogg
Copy link
Contributor

@chiphogg chiphogg commented Dec 2, 2024

Turns out, using IToA for magnitudes was a little bit sloppy.
Magnitude values are always unsigned, which means there are meaningful
values that won't fit inside of an int64_t. Fortunately, adding all
those constants (#336) was a great stress test to expose this.

I think the most natural fix would be to add a UIToA, which can't
handle signed values, but which can handle the "upper half" of
uint64_t values. This way, we can have IToA delegate to UIToA for
the "integer magnitude" parts of the logic.

Why not just get rid of IToA altogether? Well, we do need it for
printing exponents, which can be negative.

Helps #90: this gets the test to pass on #336.

Turns out, using `IToA` for magnitudes was a little bit sloppy.
Magnitude values are always unsigned, which means there are meaningful
values that won't fit inside of an `int64_t`.  Fortunately, adding all
those constants (#336) was a great stress test to expose this.

I think the most natural fix would be to add a `UIToA`, which can't
handle signed values, but which _can_ handle the "upper half" of
`uint64_t` values.  This way, we can have `IToA` delegate to `UIToA` for
the "integer magnitude" parts of the logic.

Why not just get rid of `IToA` altogether?  Well, we do need it for
printing exponents, which can be negative.

Helps #90: this gets the test to pass on #336.
@chiphogg chiphogg added the release notes: 🐛 lib (bugfix) PR fixing a defect in the library code label Dec 2, 2024
@chiphogg chiphogg merged commit c966684 into main Dec 2, 2024
13 checks passed
@chiphogg chiphogg deleted the chiphogg/uitoa#90 branch December 2, 2024 17:44
@chiphogg chiphogg restored the chiphogg/uitoa#90 branch December 2, 2024 17:44
@chiphogg chiphogg deleted the chiphogg/uitoa#90 branch December 2, 2024 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: 🐛 lib (bugfix) PR fixing a defect in the library code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants