Closed Bug 580051 Opened 11 years ago Closed 11 years ago

TABLESWITCH can be confused by fatvals

Categories

(Core :: JavaScript Engine, defect)

Other Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jorendorff, Assigned: luke)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

The implementation of TABLESWITCH assumes there are no integer-valued double Values, which isn't true anymore. Easy enough to fix.

var a = Math.sin(Math.PI/2);
assertEq(a, 1);
switch (a) {
  case 1: break;
  default: throw "FAIL";
}

(It would be nice to find a way to test this that doesn't involve inexact math, but this is the first way I found to trigger the bug.)
Attached patch fixSplinter Review
Thanks Jason!
Assignee: general → lw
Status: NEW → ASSIGNED
Attachment #458480 - Flags: review?(jorendorff)
Comment on attachment 458480 [details] [diff] [review]
fix

It seems the local variable d is unnecessary.

Please change the test to say this:

  var a = Math.sin(Math.PI/2);  // spec does not specify precise answer here...
  if (a === 1) {    // ...but if a === 1 here...
      switch (a) {
        case 1: break;  // ...then it must also match here
        default: throw "FAIL";
      }
  }

and add a second test like this:

  var arr = Float32Array(1);
  arr[0] = 15;
  var a = arr[0];
  assertEq(a, 15);
  switch (a) {
    case 15: break;
    default: throw "FAIL";
  }

r=me with that.
Attachment #458480 - Flags: review?(jorendorff) → review+
http://hg.mozilla.org/tracemonkey/rev/73922e37b733
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/73922e37b733
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.