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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

separated from bug 1304191 comment #40 and bug 1304191 comment #41.

Comment 1

[Tooru Fujisawa [:arai]]

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

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Part 1: Add mozilla::SpecificNaNBits and JS::detail::CanonicalizedNaNBits.

Applied the comment in bug 1304191.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Part 2: Add JS::IsCanonicalized and remove JS::Value::setDoubleNoCheck.

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Part 3: Do not check NaN in NumberValue(uint32_t i).

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)

Comment 5

[Tooru Fujisawa [:arai]]

btw, what's the point of adding IntegerConstantValue separated from Int32Value or NumberValue? (bug 1304191 comment #41)
can't we just make NumberValue constexpr?

Comment 6

[Jeff Walden [:Waldo]]

(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.

Comment 7

[Tooru Fujisawa [:arai]]

Thank you for reviewing!

Filed bug 1313651 and bug 1313652, for NumberValue and IntegerConstantValue.

Comment 8

[Tooru Fujisawa [:arai]]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c8357a5b2431
https://hg.mozilla.org/mozilla-central/rev/9581faa40dca
https://hg.mozilla.org/mozilla-central/rev/c2b8b8480075