Closed Bug 1097512 Opened 5 years ago Closed 5 years ago

Allow using compound operators where the LHS and RHS are both CheckedInt<T>

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: kinetik, Assigned: kinetik)

Details

Attachments

(1 file)

I was surprised to run into this.  Is it not generally useful enough to have, is there something subtle I'm missing, or am I just the lucky person to discover a minor hole in the functionality?

This works as expected:

  CheckedInt<int> a;
  CheckedInt<int> b;

  a = a + b

...but:

  a += b;

...fails to compile due to the static_assert firing because detailed::IsSupported<CheckedInt<int>>::value is false.
Attached patch simple fixSplinter Review
It seems like a simple fix, but maybe there's a reason we don't want to support this.
Attachment #8521197 - Flags: review?(jwalden+bmo)
Comment on attachment 8521197 [details] [diff] [review]
simple fix

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

Probably just an oversight, I think.

Please also add some trivial checks of some sort to mfbt/tests/TestCheckedInt.cpp to test it does the right thing.  Make sure to have checks where the RHS is a valid CheckedInt, and where it's not a valid CheckedInt.
Attachment #8521197 - Flags: review?(jwalden+bmo) → review+
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/5a2271cb4a67
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.