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

Flesh out algorithms for addition, subtraction, multiplication, division, and remainder #143

Merged
merged 56 commits into from
May 30, 2024

Conversation

jessealama
Copy link
Collaborator

We define a new AO that takes the exact values that addition (subtraction, etc.) would return and computes a Decimal128 value. The computation is done as IEEE 754 specifies: we do the operations as though we had unlimited precision and range, and then try to bring that ideal result down into the limited world of Decimal128. The idea is that if the computation returns a valid result, then just return it. Otherwise, massage the result by either dividing or multiplying by 10 and shifting the exponent accordingly until we get something that can be represented as a Decimal128 value.

Copy link
Member

@littledan littledan left a comment

Choose a reason for hiding this comment

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

Looks great, feel free to land in this form as it's already a big improvement! Just some comments on form below; I haven't really reviewed the algorithms.

spec.emu Outdated Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
spec.emu Show resolved Hide resolved
spec.emu Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
spec.emu Show resolved Hide resolved
spec.emu Outdated Show resolved Hide resolved
1. Assert: _n_ is not 0.
1. Let _digits_ be the unique decimal string representation of _n_ without leading zeroes.
1. If _q_ < 0, then
1. Let _trailingZeroes_ be the String *"0"* repeated abs(_n_) -_q_ times.
Copy link
Member

Choose a reason for hiding this comment

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

Space after - ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in #144

@jessealama jessealama marked this pull request as ready for review May 30, 2024 15:21
1. If _coefficient_ × 10<sup>_intendedExponent_</sup> is an integer _n_ such that 0 < abs(_n_) < 10<sup>34</sup>, return « _coefficient, _intendedExponent_ ».
1. Throw a *RangeError* exception.
1. If _intendedExponent_ < 0, return ? EnsureDecimal128Value(_coefficient_ / 10, _intendedExponent_ + 1, _roundingMode_).
1. Otherwise, return ? EnsureDecimal128Value(_coefficient_ × 10, _intendedExponent_ + 1, _roundingMode_).
Copy link
Member

Choose a reason for hiding this comment

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

It'd be a little cleaner if this were written non-recursively, by explicitly doing 10 to the appropriate power

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can redo the AO to work non-recursively. Thinking of it recursively is a useful heuristic tool for me.

@jessealama jessealama merged commit 2715d7c into main May 30, 2024
@jessealama jessealama deleted the add-addition-algorithm branch May 30, 2024 15:24
</emu-alg>
<emu-note>
<p>This operation follows the specification of the conversion of IEEE 754 Decimal128 values to strings (external character sequences) discussed in Section 5.12 of <emu-xref href="#sec-bibliography">IEEE 754-2019</emu-xref>.</p>
</emu-note>
</emu-clause>

<emu-clause id="sec-decimal.prototype.tobigint">
<h1>Decimal128.prototype.toBigInt ( )</h1>
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a separate method, or an overloading of the BigInt constructor? (IMO this question shouldn't block Stage 2)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wasn't clear on how to do this -- if we're overloading the BigInt constructor, we can drop this, of course. I'm not sure which way to go. I'm happy with dropping one approach or the other.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see, you already do overload the BigInt constructor. Yes, I think this method should be dropped (at the cost of polyfilling being more difficult)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good. We can also drop the toNumber method for the same reason.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done in #144

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants