I've given up on my original dream of implementing ALL the binary data semantics using self-hosting, but it should still be possible to self-host most or all of the various utility functions. This will require at least a new memcpy intrinsic that allows us to move data around, and possibly a few others. Self-hosting will of course make execution more efficient (no C++ to JS barrier, etc) but also permits those functions to be used in parallel execution, presuming we implement the intrinsics properly.

Convert from enums to #defines. Also convert from JSObject* and HandleObject to JSObject& in many cases. These two changes are both rote and somewhat entangled with respect to the lines of code they touch.

Move ConvertAndCopyTo and binary fill into self-hosted code. More to come.

Adjust tests. There is a discrepancy between our current typed arrays, which convert `[]` to `NaN`, and the spec function `ToNumber()`, which seems to convert `[]` to `0` (or at least `+[]` is `0`). I have to consult with dherman, but I believe our typed arrays are not obeying the spec in this regard and should (eventually) be adjustd.

Adding dependency on bug 914220 because the patches here are rebased on top of those, not because of a logical dependency.

Comment on attachment 802361 [details] [diff] [review] Bug898362-Part1-ExposeConstants.diff Review of attachment 802361 [details] [diff] [review]: ----------------------------------------------------------------- For my sanity's sake, I mostly just skimmed the changes in TypedObject.cpp. I gather the vast majority of them would just cause compiler errors if anything went wrong. ::: js/src/builtin/TypedObjectConstants.h @@ +56,5 @@ > +#define JS_TYPEREPR_SLOTS 4 > + > +// These constants are for use exclusively in JS code. In C++ code, > +// prefer TypeRepresentation::Scalar etc, since that allows you to > +// write a switch which will receive a warning if you omit a case. s/receive/cause/? @@ +63,5 @@ > +#define JS_TYPEREPR_ARRAY_KIND 2 > + > +// These constants are for use exclusively in JS code. In C++ code, > +// prefer ScalarTypeRepresentation::TYPE_INT8 etc, since that allows > +// you to write a switch which will receive a warning if you omit a and here @@ +79,5 @@ > +/////////////////////////////////////////////////////////////////////////// > +// Slots for typed objects > + > +#define JS_TYPEDOBJ_SLOT_TYPE_OBJ 0 // Type object for a given typed object > +#define JS_TYPEDOBJ_SLOT_OWNER 2 // Owner of data (if null, this is owner) Why no slot 1?

Comment on attachment 802363 [details] [diff] [review] Bug898362-Part2-SelfHost.diff Review of attachment 802363 [details] [diff] [review]: ----------------------------------------------------------------- This looks pretty nice! ::: js/src/builtin/TypedObject.cpp @@ +197,5 @@ > return atom->asPropertyName(); > } > > +static JSFunction * > +SelfHostedFunction(JSContext *cx, const char *name) This should really take a PropertyName, with most or all of the names added to the atom table. Re-atomizing the names every time they're used will probably have meaningful overhead. Also, now that I think about it, would you mind moving this into SelfHosting and adding the declaration to jscntxt.h (or adding a SelfHosting.h, perhaps)? This would be useful in more places right now (I know the Intl stuff could use it) and will become more so over time. @@ +217,3 @@ > /* > + * Overwrites the contents of `block` at offset `offset` with `val` > + * converted to the type `typeObj` Full stop at the end, please. @@ +1750,5 @@ > + cx, > + &type->getReservedSlot(JS_TYPEOBJ_SLOT_STRUCT_FIELD_TYPES).toObject()); > + RootedValue fieldTypeVal(cx); > + if (!JSObject::getElement(cx, fieldTypes, fieldTypes, > + fieldIndex, &fieldTypeVal)) Either don't wrap the if, or put braces around the then. ::: js/src/builtin/TypedObject.h @@ +219,5 @@ > +// size) > +// > +// Intrinsic function. Copies size bytes from the data for > +// `sourceTypedObj` at `sourceOffset` into the data for > +// `targetTypedObj` at `targetOffset`. Nit: please keep the in-file style for function documentation. I.e. /*\n * comment\n */ @@ +228,5 @@ > +// Usage: StoreScalar(targetTypedObj, targetOffset, value) > +// > +// Intrinsic function. Stores value (which must be an int32 or uint32) > +// by `scalarTypeRepr` (which must be a type repr obj) and stores the > +// value at the memory for `targetTypedObj` at offset `targetOffset`. Same here ::: js/src/builtin/TypedObject.js @@ +48,5 @@ > +// > +// Most `TypedObjectPointers` methods are written in a "chaining" > +// style, meaning that they return `this`. This is true even though > +// they mutate the receiver in place, because it makes for prettier > +// code. While I generally don't care too much about comment style, this one threw me off. It makes the comment look like it introduces a new section of code, not an individual function. Maybe lose the first line of many "/"s and remove the empty line between the comment and the function? @@ +57,5 @@ > + this.owner = owner; > + this.offset = offset; > +} > + > +MakeConstructible(TypedObjectPointer, {}); Not that it matters too much, but I'd probably initialize the prototype like so: MakeConstructible(TypedObjectPointer, { copy: function[..], reset: function[..], kind: function[..], #ifdef DEBUG toString: function[..], #endif [..] }); @@ +86,5 @@ > +/////////////////////////////////////////////////////////////////////////// > +// moveTo(propName) > +// > +// Adjusts `this` in place so that it points at the property > +// `propName`. Throws if there is no such property. Returns `this`. See my comment on comment style above. @@ +160,5 @@ > +// set(fromValue) > +// > +// Assigns `fromValue` to the memory pointed at by `this`, adapting it > +// to `typeRepr` as needed. This is the most general entry point and > +// works for any type. See my comment on comment style above. @@ +215,5 @@ > + } > + > + ThrowError(JSMSG_CANT_CONVERT_TO, > + typeof(fromValue), > + this.typeRepr.toSource()) No need to wrap this, plus missing semicolon. @@ +263,5 @@ > +function ConvertAndCopyTo(destTypeRepr, > + destTypeObj, > + destTypedObj, > + destOffset, > + fromValue) { Brace on new line, please.

Carrying over r+ from till

Clean try run: https://tbpl.mozilla.org/?tree=Try&rev=134073866259