593413 - Have a default templatized createInstance to remove boilerplate

Status: RESOLVED INVALID

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Tommy Reilly]

We have tons of these:
    ScriptObject* ByteArrayClass::createInstance(VTable *ivtable,
                                                 ScriptObject *prototype)
    {
        return new (core()->GetGC(), ivtable->getExtraSize()) ByteArrayObject(ivtable, prototype);
    }

Why not have:

template <class INSTANCE>
class ClassClosure
{
    INSTANCE* ByteArrayClass::createInstance(VTable *ivtable,
                                                 ScriptObject *prototype)
    {
        return new (core()->GetGC(), ivtable->getExtraSize()) INSTANCE(ivtable, prototype);
    }

};

In fact ByteArrayClass might be able to go away and be replaced with:

typedef ByteArrayClass ClassClosure<ByteArrayObject>;

Comment 1

[Lars T Hansen]

+1.  It would simplify finding the exceptions to the rule (which seems to be a common exercise).

Comment 2

[Edwin Smith]

Analyzer to find createInstance() overrides.

http://hg.mozilla.org/users/edwsmith_adobe.com/analyzers/file/tip/createInstance.js

Hand inspection of the resulting classes in TR shows all but one or two are pure boilerplate.  A few more (ArrayClass, RegExpClass) pass generic default args to their ArrayObject/RegExpObject constructors, so could be made generic.

I also noticed that ObjectVectorClass copies the type parameter to each *instance* of ObjectVector.  Seems more appropraiate for the parameter to live on the instance vtable.

Comment 3

[Lars T Hansen]

This will be useful, but you might as well hold off - the exactgc work touches the createInstance methods extensively, you'll have massive merge conflicts (at least in Tamarin).

Comment 4

[Tommy Reilly]

While we're at we should really figure out a way to get around having to write a boilerplate ctor to, ie:

    DomainObject::DomainObject(VTable *vtable, ScriptObject *delegate)
        : ScriptObject(vtable, delegate)
    {}

Maybe the existing macros provided by the nativegen stuff could provide.   The ctor should probably be private too and  only createInstance should call it no.    Actually we should probably declare and not implement a private default constructor too.

Comment 5

[Lars T Hansen]

In general the ctor must be protected, because some native classes subclass other native classes.

And not all constructors are trivial, though the majority are.  So there needs to be an escape hatch.

The private default constructor will be redundant if there's another constructor present, no?

Comment 6

[Tommy Reilly]

(In reply to comment #3)
> This will be useful, but you might as well hold off - the exactgc work touches
> the createInstance methods extensively, you'll have massive merge conflicts (at
> least in Tamarin).

Yeah, not actively working on it, we can re-evaluate once exactgc lands.

Comment 7

[Tommy Reilly]

(In reply to comment #5)
> In general the ctor must be protected, because some native classes subclass
> other native classes.
>
> And not all constructors are trivial, though the majority are.  So there needs
> to be an escape hatch.

Yeah, this goes for createInstance and ctor (obviously).

> The private default constructor will be redundant if there's another
> constructor present, no?

My concern was more about developer adding their own and using it.    If we declare a private one and don't implement it we can keep that from happening.

Comment 8

[Dan Smith]

Is this still useful given Steven's ANI changes to auto-generate a createInstance equivalent?

Comment 9

[Tommy Reilly]

no