Closed Bug 1508725 Opened 9 months ago Closed 9 months ago

Adjust mozilla::FloatingPoint<T>'s definition so only the barest details are specified for floating-point encodings, with every other number, bit mask, &c. mathematically derived

Categories

(Core :: MFBT, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: Waldo, Assigned: Waldo)

Details

Attachments

(1 file)

I've had this patch locally for awhile.  It adds a few new named constants and tries to generally make consistent how widths and such are named, for clearest use.  (No asking for "exponent shift" when really you want "significand width", for example.)  It also computes everything directly from the fewest possible numbers, which IMO helps to clarify that derived numbers are exactly that.

To be sure, this doesn't actually fix any bugs.  But it's clearer, and I'm reviewing patches these days that are better expressed with this set of field-names, and it's super-frustrating not just to look to my local tree when suggesting stuff to use (because I don't know what's landed and what hasn't).

And I suppose I ought say, I haven't compiled this patch in months, so I'll certainly do that before landing it just in case new DoubleTypeTraits or whatever uses have crept into the tree.
Attached patch PatchSplinter Review
Attachment #9026456 - Flags: review?(nfroyd)
Comment on attachment 9026456 [details] [diff] [review]
Patch

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

WFM.

::: mfbt/FloatingPoint.h
@@ +117,5 @@
> +  /** The bit-width of the significand component of T. */
> +  using Base::kSignificandWidth;
> +
> +  static_assert(1 + kExponentWidth + kSignificandWidth ==
> +                CHAR_BIT * sizeof(T),

Such genericity.
Attachment #9026456 - Flags: review?(nfroyd) → review+
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/91c06aa99424
Adjust mozilla::FloatingPoint<T>'s definition so only the barest details are specified for floating-point encodings, with every other number, bit mask, &c. mathematically derived.  Also add a bunch of documentation comments.  r=froydnj
https://hg.mozilla.org/mozilla-central/rev/91c06aa99424
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.