serenity
serenity copied to clipboard
SerenityOS
Calculator: Move towards a Calculator without any numerical errors (part 1)
I'm splitting #11623 in multiple PR to ease the review process.
This PR includes all following changes:
- [x] Support any number
- [x] Support flawless +, -, *, /, inv
- [x] Display rounded numbers
- [ ] Enhance user interaction with rounding
Hope the experience wasn't that terrible! Thank you very much for reviewing this. It was my first "big" PR for SerenityOS, I will be very happy to see it merged.
I also had to remove all BigInteger::create_from() occurrences and replace them with direct call to the constructor (due to #15029).
Hello!
One or more of the commit messages in this PR do not match the SerenityOS code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why.
Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.
The issue seems to be that it doesn't think size_t and u64 are the same type. (They should be, unless the Mac build is 32 bit?) It sees an overload for u64, and one for double, but size_t is neither of those so it can't decide.
I think you can fix it by changing BigFraction.cpp:39 from
auto fraction_length = UnsignedBigInteger(fraction_part_view.length());
to
auto fraction_length = UnsignedBigInteger(static_cast<u64>(fraction_part_view.length()));
so it knows to call the u64 one.
It used to be constructed with an initializer-list, so last push was for turning braces into parenthesis. I thought that it would do the trick.
The manual cast will work but I wanted to try my fix first to avoid it. Looks like I have no other choice now, I will add it tomorrow when I go back home.
Constructing something with () or {} is the same internally, they just both exist because () is ambiguous syntax sometimes.
Not to be confused with a constructor that takes an initializer_list argument, or the C-style {.foo = 1, .bar = 2} initialization. :cry:
My bad, I meant list initialization and not initializer_list. And there is a difference, as list initialization doesn't allow narrowing (I had conversion in mind, it's why I tested it).
Anyway, it makes no difference, as converting size_t to u64 is not considered as narrowing.
This is very much postmortem, but was commit 4ab8ad2ed2817dd7bf92d819f58ed324654b6db7 still valid? SignedBigInteger can't be negative zero anymore after commit b0d6399f60760e25a55ec9e8e95a1ad322b74b22. Asking because if it became possible again, that's a bug.
I wrote this commit back in January, so before b0d6399, it is indeed probably useless now.