If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

OdinMonkey: reading from float view gives doublish but writing requires double?

RESOLVED FIXED in mozilla24

Status

()

Core
JavaScript Engine
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Assigned: luke)

Tracking

(Blocks: 2 bugs, {testcase})

Trunk
mozilla24
x86_64
Mac OS X
testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
function g(stdlib, foreign, heap)
{
  "use asm"
  var Float32ArrayView = new stdlib.Float32Array(heap)
  function f()
  {
    Float32ArrayView[0] = Float32ArrayView[1];
  }
  return f;
}

/Users/jruderman/Desktop/asmjs/e.js:7:0 warning: asm.js type error: doublish is not double

(Is this an intentional deviation from the spec?)
(Reporter)

Comment 1

4 years ago
In contrast, intish works fine:

function g(stdlib, foreign, heap)
{
  "use asm"
  var Int32ArrayView = new stdlib.Int32Array(heap)
  function f()
  {
    Int32ArrayView[0] = Int32ArrayView[1];
  }
  return f;
}

Comment 2

4 years ago
My reading of the spec is that both intish heaps and doublish heaps should work, and both of these examples validate in http://turtlescript.github.cscott.net/docco/asm-llvm.html

(Maybe I should make asm-llvm an online checker?)
(Assignee)

Comment 3

4 years ago
Yeah, we talked about it on irc and I think this is just a simple bug in Odin.
(Assignee)

Comment 4

4 years ago
Created attachment 760095 [details] [diff] [review]
fix and test
Assignee: general → luke
Status: NEW → ASSIGNED
Attachment #760095 - Flags: review?(bbouvier)
(Reporter)

Comment 5

4 years ago
>             return f.failf(lhs, "%s is not a subtype of intish", rhsType.toChars());

vs

>+            return f.failf(lhs, "%s is not doublish", rhsType.toChars());

seems like a minor inconsistency.
Comment on attachment 760095 [details] [diff] [review]
fix and test

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

Nice! As Jesse pointed out, it would be also nice that the messages have the same form.
And still for the purposes of consistency, should we change the enum element |ArrayStore_Double| to |ArrayStore_Doublish|?
Attachment #760095 - Flags: review?(bbouvier) → review+
(Assignee)

Comment 7

4 years ago
Mmm, by sense of style seems weak at 2am.  Thanks guys
(Assignee)

Comment 8

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/1eff67ffe6b9

Comment 9

4 years ago
https://hg.mozilla.org/mozilla-central/rev/1eff67ffe6b9
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.