752397 - Add a way to safely go from a (Auto)FallibleTArray to a corresponding const ns(Auto)Tarray.

Status: NEW

(Core :: XPCOM, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Right now the code looks more or less like this:

  nsTArray<Foo> arg0;
  FallibleTArray<Foo> temp;
  if (!temp.SetCapacity(something_I_computed)) {
    // return error
  }
  // Fill in the entries of temp
  arg0.SwapElements(temp);
  CallSomeDOMMethod(arg0);

This is because we want to do a fallible allocation (since "something_I_computed" is controlled by web script), but want to allow callees to just use nsTArray& in the normal way.

Now the thing is, I have no problem with requiring the callees to take a |const nsTArray&|.  So with that in mind, we have a few options:

1) Leave as-is.  For short arrays, the malloc/free costs for the buffer are pretty steep (order of half the time spent in the binding code in the webgl uniform setters, say).

2) Use auto arrays for both arg0 and temp.  This gets rid of the malloc/free for short arrays, but replaces them with memcpy and generally running more code in the SwapElements implementation.

3) Create an AutoFallibleTArray, but do some reinterpret_casting to pass it in.  Like so, say:

  AutoFallibleTArray<Foo, 16> temp;
  if (!temp.SetCapacity(something_I_computed)) {
    // return error
  }
  // Fill in the entries of temp
  const nsAutoTArray<Foo, 16> & arg0 = 
    *reinterpret_cast<const nsAutoTArray<Foo, 16>*>(&temp);
  CallSomeDOMMethod(arg0);

or some such.  I _think_ the actual binary layout of nsAutoTArray and AutoFallibleTArray is the same, and as long as people don't const_cast the const away this should all just work...

Comment 1

[Justin Lebar (not reading bugmail)]

> and as long as people don't const_cast the const away this should all just work...

I think even if they do cast the const away, it should work.  The layout of fallible and infallible auto arrays is exactly the same; only the allocator is different.

The clean thing to do from the perspective of this code would be to subclass TArray into an immutable piece, and replace the reinterpret cast with static_cast to immutable_thing (maybe that's a method on the tarray itself).

But that might be more work than it's worth, since the reinterpret cast works fine for the moment...

Comment 2

[Justin Lebar (not reading bugmail)]

> The layout of fallible and infallible auto arrays is exactly the same; only the allocator 
> is different.

And specifically, we already require that all allocators share the same free() (and realloc(), although I don't think that's explicitly mentioned), so the infallible array can realloc or free the fallible array's memory.

Comment 3

[Boris Zbarsky [:bzbarsky]]

OK.  I'll do the reinterpret_cast for now (this is all code that hasn't even been reviewed yet), but should we mutate this bug to cover adding the TArray API to be able to use static_cast?

Comment 4

[Justin Lebar (not reading bugmail)]

> should we mutate this bug to cover adding the TArray API to be able to use static_cast?

SGTM!

Comment 5

[Boris Zbarsky [:bzbarsky]]

What I've done in bindings now is bug 755636, so this no longer blocks binding stuff.