Add mozilla::SpecificNaNBits and use it in JS::GenericNaN and some other functions

RESOLVED FIXED in Firefox 52

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: arai, Assigned: arai)

Tracking

Trunk
mozilla52
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(3 attachments)

Assignee

Comment 1

3 years ago
Things to do here:
  * Add mozilla::SpecificNaNBits and use create CanonicalizedNaNBits on global
  * Use CanonicalizedNaNBits in JS::GenericNaN
  * Make it possible to detect canonicalized NaN without creating the double JS::Value
Summary: Cleanup NaN handling in Value.h → Add mozilla::SpecificNaNBits and use it in JS::GenericNaN and some other functions
Assignee

Comment 2

3 years ago
Applied the comment in bug 1304191.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Attachment #8803689 - Flags: review?(jwalden+bmo)
Assignee

Comment 4

3 years ago
it won't need NaN handling, so we can just call Value::fromDouble, and it doesn't ivalidate constexpr.
(actually I'm not sure why constexpr is set only to this overload tho)
Attachment #8803691 - Flags: review?(jwalden+bmo)
Assignee

Comment 5

3 years ago
btw, what's the point of adding IntegerConstantValue separated from Int32Value or NumberValue? (bug 1304191 comment #41)
can't we just make NumberValue constexpr?
Attachment #8803689 - Flags: review?(jwalden+bmo) → review+
Attachment #8803690 - Flags: review?(jwalden+bmo) → review+
Attachment #8803691 - Flags: review?(jwalden+bmo) → review+
(In reply to Tooru Fujisawa [:arai] from comment #5)
> can't we just make NumberValue constexpr?

We could.  And maybe we should, because why not?

> what's the point of adding IntegerConstantValue separated from
> Int32Value or NumberValue? (bug 1304191 comment #41)

But because NumberValue has that double version, not *every* overload is guaranteed to be constant.  So in code that *demands* constexpr, using NumberValue is a bit of a footgun, because you don't *know* that using it will give constexpr behavior.  Maybe your expression's type will be changed at some point, and then suddenly you have a new static initializer.

Wherefore, JS::IntegerConstantValue that *only* has constexpr overloads.

This is possibly a little paranoid-ish, but can you imagine WebIDL codegen wanting to take the static-initializer risk if we can guarantee it'll always be eliminated?  I'd think they'd prefer definite safety.
Assignee

Comment 7

3 years ago
Thank you for reviewing!

Filed bug 1313651 and bug 1313652, for NumberValue and IntegerConstantValue.
See Also: → 1313651, 1313652
Assignee

Comment 8

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8357a5b2431329993612ecd1095bff70f1d1911
Bug 1311088 - Part 1: Add mozilla::SpecificNaNBits and JS::detail::CanonicalizedNaNBits. r=jwalden

https://hg.mozilla.org/integration/mozilla-inbound/rev/9581faa40dcaf5b090e12d581efc08deb839c8cd
Bug 1311088 - Part 2: Add JS::IsCanonicalized and remove JS::Value::setDoubleNoCheck. r=jwalden

https://hg.mozilla.org/integration/mozilla-inbound/rev/c2b8b8480075a26f7caeb6d2f4d49779a9632aa7
Bug 1311088 - Part 3: Do not check NaN in NumberValue(uint32_t i). r=jwalden
You need to log in before you can comment on or make changes to this bug.