Closed Bug 1445024 Opened 2 years ago Closed 2 years ago

Implement mozilla::Wrapping{Add,Subtract}

Categories

(Core :: MFBT, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: Waldo, Assigned: Waldo)

Details

Attachments

(3 files)

No description provided.
Some of the comments here are getting a little duplicate-y -- don't worry, I do a little consolidation in the next patch after this.
Attachment #8958209 - Flags: review?(nfroyd)
1 files changed, 65 insertions(+), 92 deletions(-)
Attachment #8958210 - Flags: review?(nfroyd)
Attachment #8958211 - Flags: review?(nfroyd)
Attachment #8958209 - Flags: review?(nfroyd) → review+
Attachment #8958210 - Flags: review?(nfroyd) → review+
Comment on attachment 8958211 [details] [diff] [review]
Implement mozilla::WrappingSubtract

Review of attachment 8958211 [details] [diff] [review]:
-----------------------------------------------------------------

This code works for me.

::: mfbt/WrappingOperations.h
@@ +196,5 @@
> + * For N-bit unsigned integer types, this is equivalent to subtracting the two
> + * numbers, then taking the result mod 2**N:
> + *
> + *   WrappingSubtract(uint32_t(42), uint32_t(17)) is 29 (29 mod 2**32);
> + *   WrappingSubtract(uint8_t(5), uint8_t(20)) is -241 (-15 mod 2**8).

Surely subtracting two unsigned integers doesn't produce a signed integer?  (I mean, it *does*, mathematically, but in fixed-width arithmetic with unsigned types I don't think that's exactly what you mean...)  At the very least, this example is in contravention of the statement immediately after it.
Attachment #8958211 - Flags: review?(nfroyd) → review+
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/491149fb04c7
Implement mozilla::WrappingAdd.  r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/99f24cb57f64
Consolidate some WrappingOperations.h comments and implementation bits.  r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/82c94b926450
Implement mozilla::WrappingSubtract.  r=froydnj
https://hg.mozilla.org/mozilla-central/rev/491149fb04c7
https://hg.mozilla.org/mozilla-central/rev/99f24cb57f64
https://hg.mozilla.org/mozilla-central/rev/82c94b926450
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.