Bug 732936 lets users feed ArrayBuffer to js-ctypes. We should do the same with TypedArray.

After toying with the idea, I tend to believe that we should accept to convert any typed array to any array/pointer type, without attempting to add a veneer of type-safety.

Comment on attachment 666181 [details] [diff] [review] Feeding typed arrays to js-ctypes Review of attachment 666181 [details] [diff] [review]: ----------------------------------------------------------------- Yep, looks correct. r=me with stricter implicit conversions. ::: js/src/ctypes/CTypes.cpp @@ +2194,5 @@ > + > + // FIXME: Should we need to, we can obtain the size of an element in the > + // array as follows: > + // JS_GetTypedArrayByteLength(obj, cx)/JS_GetTypedArrayLength(obj, cx) > + *static_cast<void**>(buffer) = JS_GetArrayBufferViewData(valObj, cx); Please check that the pointer type is consistent (or void *). Users can easily use ctypes.cast if they really want a reinterpret_cast. Or just pass the ArrayBuffer, which is more like a void*. @@ +2304,5 @@ > + // Check that array is consistent with type, then > + // copy the array. As with C arrays, data is *not* > + // copied back to the ArrayBuffer at the end of a > + // function call, so do not expect this to work > + // as an inout argument. ISTR arguments of array type are silently converted to pointers, because that's actually what C does! These two declarations are identical in every way: void f(int x[]); void f(int *x); So I think a TypedArray actually *will* work as an in-out argument to such a function, because this particular code won't get called--the pointer code above runs instead. @@ +2309,5 @@ > + uint32_t sourceLength = JS_GetTypedArrayByteLength(valObj, cx); > + size_t elementSize = CType::GetSize(baseType); > + size_t arraySize = elementSize * targetLength; > + if (arraySize != size_t(sourceLength)) { > + JS_ReportError(cx, "typed array length does not match source array length"); Please check that the types really match, not just that the length is the same. It should be pretty easy; treat a Uint32Array just like a uint32_t.array(length). ::: toolkit/components/ctypes/tests/unit/test_jsctypes.js.in @@ +1765,5 @@ > + [new Uint32Array(c_arraybuffer), ctypes.uint32_t], > + [new Float32Array(c_arraybuffer), ctypes.float32_t], > + [new Float64Array(c_arraybuffer), ctypes.float64_t] > + ]) { > + let [view, item_type] = sample; For what it's worth, you can say 'for (let [view, item_type] of ...)'

(In reply to Jason Orendorff [:jorendorff] from comment #3) > ::: toolkit/components/ctypes/tests/unit/test_jsctypes.js.in > @@ +1765,5 @@ > > + [new Uint32Array(c_arraybuffer), ctypes.uint32_t], > > + [new Float32Array(c_arraybuffer), ctypes.float32_t], > > + [new Float64Array(c_arraybuffer), ctypes.float64_t] > > + ]) { > > + let [view, item_type] = sample; > > For what it's worth, you can say 'for (let [view, item_type] of ...)' Good point.

Attaching a new version that performs strict type-checking. Note that we cannot convert a Int32Array to a int[] or int* even on a platform on which int is 32 bits. I believe that allowing this would be a misfeature, what do you think?

Comment on attachment 667403 [details] [diff] [review] Feeding typed arrays to js-ctypes, v2 Review of attachment 667403 [details] [diff] [review]: ----------------------------------------------------------------- It still doesn't check for exact type match when converting a typedarray to a ctypes array. r=me with that. ::: js/src/ctypes/CTypes.cpp @@ +2202,5 @@ > + TypeCode elementTypeCode; > + switch (JS_GetTypedArrayType(valObj, cx)) { > + case TypedArray::TYPE_INT8: > + elementTypeCode = TYPE_int8_t; > + break; Factor this code into a function and use the same function in the array case.

(In reply to David Rajchenbach Teller [:Yoric] from comment #5) > Attaching a new version that performs strict type-checking. > Note that we cannot convert a Int32Array to a int[] or int* even on a > platform on which int is 32 bits. I believe that allowing this would be a > misfeature, what do you think? It'd be inconsistent with the existing design. I'm ok with revisiting that design decision later.

Ah, my bad, forgot to check type-conversion with arrays. Attaching a new version. I took the opportunity to improve testing of ArrayBuffer/typed array to array conversions.

Same patch, *after* qref.

I just realized that my previous tests fail to cover the critical case in which the typed array does not start at offset 0 of the buffer. Here are the additional tests.

Comment on attachment 668435 [details] [diff] [review] 2. More tests Gold star.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7be4e7680b42 https://hg.mozilla.org/integration/mozilla-inbound/rev/03e4c6401c35

https://hg.mozilla.org/mozilla-central/rev/7be4e7680b42 https://hg.mozilla.org/mozilla-central/rev/03e4c6401c35