Closed Bug 1311088 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: arai, Assigned: arai)

References

Details

Attachments

(3 files)

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
Applied the comment in bug 1304191.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Attachment #8803689 - Flags: review?(jwalden+bmo)
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)
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.
Thank you for reviewing!

Filed bug 1313651 and bug 1313652, for NumberValue and IntegerConstantValue.
See Also: → 1313651, 1313652
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.