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)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: arai, Assigned: arai)
References
Details
Attachments
(3 files)
3.55 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
2.58 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
781 bytes,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
separated from bug 1304191 comment #40 and bug 1304191 comment #41.
Assignee | ||
Comment 1•8 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•8 years ago
|
||
Applied the comment in bug 1304191.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Attachment #8803689 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 3•8 years ago
|
||
Attachment #8803690 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 4•8 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•8 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?
Updated•8 years ago
|
Attachment #8803689 -
Flags: review?(jwalden+bmo) → review+
Updated•8 years ago
|
Attachment #8803690 -
Flags: review?(jwalden+bmo) → review+
Updated•8 years ago
|
Attachment #8803691 -
Flags: review?(jwalden+bmo) → review+
Comment 6•8 years ago
|
||
(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•8 years ago
|
||
Thank you for reviewing! Filed bug 1313651 and bug 1313652, for NumberValue and IntegerConstantValue.
Assignee | ||
Comment 8•8 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
Comment 9•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c8357a5b2431 https://hg.mozilla.org/mozilla-central/rev/9581faa40dca https://hg.mozilla.org/mozilla-central/rev/c2b8b8480075
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•