+"0x" erroneously yields zero

RESOLVED FIXED in mozilla24

Status

()

defect
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: richard.gibson, Assigned: Waldo)

Tracking

(Blocks 1 bug)

21 Branch
mozilla24
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Reporter

Description

6 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.22 (KHTML, like Gecko) Ubuntu Chromium/25.0.1364.160 Chrome/25.0.1364.160 Safari/537.22

Steps to reproduce:

cast string "0x" to number with unary `+` operator (similar to but distinct from #582643)


Actual results:

a result of 0 (zero)


Expected results:

a result of NaN (http://www.ecma-international.org/ecma-262/5.1/#sec-9.3.1)
Assignee

Comment 1

6 years ago
Huh, we get this wrong?  That's kind of amazing after all this time.  :-)

This should be pretty easy to fix, excepting that the fix is to a centralized method, and making sure all the callers have the semantics desired here might take a little work.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter

Comment 2

6 years ago
Well, it *did* take a weird path to hit this:
* jQuery bug with filtering by a selector containing escaped whitespace: http://bugs.jquery.com/ticket/13894
* Fix in Sizzle CSS unescaping logic: https://github.com/jquery/sizzle/commit/00a7e0d8d66fb7915a24a2270033b8efc9e9c774#diff-3
Assignee

Comment 3

6 years ago
Turns out with methodjit gone, StringToNumberType is only used one place, only with T = double.  Simplify!
Assignee: general → jwalden+bmo
Status: NEW → ASSIGNED
Attachment #750778 - Flags: review?(evilpies)
Assignee

Comment 4

6 years ago
Attachment #750786 - Flags: review?(evilpies)
Comment on attachment 750786 [details] [diff] [review]
2 - Detect no-hex-digits as NaN

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

::: js/src/jsnum.cpp
@@ +1361,5 @@
>          const jschar *endptr;
>          double d;
> +        if (!GetPrefixInteger(cx, bp + 2, end, 16, &endptr, &d) ||
> +            endptr == bp + 2 ||
> +            SkipSpace(endptr, end) != end)

This is not super obvious. A comment describing the last two parts could be good.

::: js/src/tests/ecma_5/Number/tonumber-string-hex.js
@@ +12,5 @@
> + * BEGIN TEST *
> + **************/
> +
> +assertEq(+"0x", NaN);
> +assertEq(+"\n0x", NaN);

I feel like we should probably test some other space.
Attachment #750786 - Flags: review?(evilpies) → review+
Comment on attachment 750778 [details] [diff] [review]
1 - Eliminate jsnuminlines.h

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

nice
Attachment #750778 - Flags: review?(evilpies) → review+
https://hg.mozilla.org/mozilla-central/rev/23741f85220a
https://hg.mozilla.org/mozilla-central/rev/8ed13280c811
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.