627192 - some createInstance() overrides ignore the prototype arg

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Steven Johnson]

SampleClass::createInstance (& friends) and DictionaryClass::createInstance ignore the passed-in prototype, and use this->prototypePtr() instead. This seems wrong, but I can't find any explanation for why it should be this way. In the case of Dictionary, the oldest instance of the file is like that -- perhaps a typo that never got corrected? May not matter (much) for normal code but it's hard to see why it should deviate either

Comment 1

[Lars T Hansen]

Haven't looked at Player code, but all calls to ScriptObject::createInstance (which is virtual) pick up cls->prototypePtr() and pass that for the delegate.  So semantically it appears to make no difference, indeed the delegate argument appears to be redundant.

Comment 2

[Steven Johnson]

I audited all the call sites and definitions in Flash/AIR; it is correct that the args are always ivtable()/prototypePtr() at all call sites (except for a degenerate Sampler case), so these args are really redundant.

For that matter, it occurs to me that we can (and probably should) generate these with nativegen, since we know exactly when they should be generated and how (to wit, if both cls and instance are defined). The gotcha is that there are a handful of overrides in player code that do more than just a new:

-- unconditionally throw an error (to make an abstract base class)
-- lazy initialization of related data structures
-- conditionally throw an error (to prevent construction in some circumstances, or error checking)
-- pass an extra argument to the instance's ctor (typically 'this')

The first two could be handled without too much drama; the last two are a bit trickier.

Comment 3

[Lars T Hansen]

Re abstract classes, observe that different native classes throw different errors for this problems (which is one of the things that's been holding up the spec on abstract classes, sadly).  So if you add a [native(..., abstract, ...)] annotation it needs to be [native(..., abstract="TypeError", ...)] or something like that to spec the error type.  Unless we decide that that's just silly.

IMO variations can most reasonably be handled if you decide *not* to boil the ocean by insisting on handling the problem in all cases but instead create a system that handles almost all cases and is well-specified enough to allow the rest of the cases to be implemented by hand, cf how exact tracing is done.  Then over time implement additional features (proper abstract classes, say, or private constructors) that carve into the bulk of hand-written cases.

Comment 4

[Steven Johnson]

(In reply to comment #3)
> IMO variations can most reasonably be handled if you decide *not* to boil the
> ocean by insisting on handling the problem in all cases but instead create a
> system that handles almost all cases and is well-specified enough to allow the
> rest of the cases to be implemented by hand, cf how exact tracing is done. 
> Then over time implement additional features (proper abstract classes, say, or
> private constructors) that carve into the bulk of hand-written cases.

Yep. What I'm leaning towards now is allowing a "createHook" to be specified, allowing some classes to specify a method that can be called prior to creation, which I think will get pretty much all the weird cases.