636363 - ANI: offer a simple, statically-typed way to create instances

Status: RESOLVED FIXED

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Steven Johnson]

Existing usage in Flash/AIR has a convoluted, printf/varargs approach to convert C++ values into Atom, call into the class contructor, and return the result. We should instead have nativegen emit wrappers, so C++ code can do something like

    FooObject* foo = toplevel->builtinClasses()->get_FooClass()->construct(1, 3.14, "Happy");

Comment 1

[Steven Johnson]

Attachment: Part 1: generate "synthetic" classes for non-native builtin classes

Previously, non-native builtin classes all appeared in the classmanifest as "ClassClosure*", which is accurate but not useful for achieving our goal; this patch creates a "synthetic" wrapper for each non-native builtin class that the next patch will build on. Note that these are truly synthetic, as these C++ classes are never instantiated as such; however, since there's no mechanism for them to live outside the class manifest we should be safe. Also note that they are emitted in a new header file; this is necessary as they must come after ClassClosure in the inclusion process.

Comment 2

[Steven Johnson]

Attachment: Part 2: generate "constructObject" methods for all classes

The patch also converts a handful of existing usecases to use the new call; most usage will happen in Flash/AIR glue code.

Comment 3

[Christopher A. Brichford]

Your generated class should probably also define a private operator= and a private copy constructor unless you can show that those won't work given you current method.  It may be sufficient to show that a private operator= and a private copy construct on the ScriptObject base class will prevent all subclasses from be assigned to or copy constructed.

Comment 4

[Steven Johnson]

(In reply to comment #3)
> Your generated class should probably also define a private operator= and a
> private copy constructor unless you can show that those won't work given you
> current method.  It may be sufficient to show that a private operator= and a
> private copy construct on the ScriptObject base class will prevent all
> subclasses from be assigned to or copy constructed.

Good call, I'll add that and see if there are any red flags.

Comment 5

[Steven Johnson]

Attachment: Part 3 -- generate private operator= and copy-ctor

Comment 6

[Tamarin Bot]

changeset: 5995:48b099e59e48
user:      Steven Johnson <[e-mail]>
summary:   Bug 636363 - ANI: offer a simple, statically-typed way to create instances (r=rreitmai)

http://hg.mozilla.org/tamarin-redux/rev/48b099e59e48

Comment 7

[Tommy Reilly]

This is awesome!    Will the machinery be reusable for a generic callin mechanism?    Ideally any method could be invoked from C++ like this, although it would probably want to be limited to methods flagged in the actionscript via some metadata.

Should there be an opt-out mechanism, like maybe you don't want folks to construct from C++ for some reason?

Comment 8

[Steven Johnson]

(In reply to comment #7)
> This is awesome!    Will the machinery be reusable for a generic callin
> mechanism?    Ideally any method could be invoked from C++ like this, although
> it would probably want to be limited to methods flagged in the actionscript via
> some metadata.

Yep. I haven't decided how/what to expose yet; exposing everything is probably overkill.

> Should there be an opt-out mechanism, like maybe you don't want folks to
> construct from C++ for some reason?

Hmm, good idea, maybe will add in the future.

Comment 9

[Steven Johnson]

re-opening because it occurs to me that supporting optional args would be desirable and simple: just generate multiple overloaded constructObjects, dropping an arg off each time.

Comment 10

[Steven Johnson]

Attachment: Part 4: support optional-arguments ctors

Comment 11

[Jeff Mott]

Passing all the arguments isn't so bad, not sure it's worth increasing the amount of generated code.

Comment 12

[Steven Johnson]

(In reply to comment #11)
> Passing all the arguments isn't so bad, not sure it's worth increasing the
> amount of generated code.

Agreed; main motivation is nontrivial amount of existing glue code that takes advantage of the default args; fixing this requires inspecting the AS source and inserting the proper default values. The amount of generated code is pretty much a nonfactor since its inlined and thus theoretically only a price we pay when used.

Comment 13

[Christopher A. Brichford]

(In reply to comment #12)
> (In reply to comment #11)
> > Passing all the arguments isn't so bad, not sure it's worth increasing the
> > amount of generated code.
> 
> Agreed; main motivation is nontrivial amount of existing glue code that takes
> advantage of the default args; fixing this requires inspecting the AS source
> and inserting the proper default values. The amount of generated code is pretty
> much a nonfactor since its inlined and thus theoretically only a price we pay
> when used.

The two are semantically different passing a argument that you think is the default value is not the same as letting the vm get the real default value from method info.  What the current patch does is not only *not* a code size issue, it is more correct.

Comment 14

[Jeff Mott]

Sounds good then.

Comment 15

[Rick Reitmaier]

Comment on attachment 515814 [details] [diff] [review]
Part 4: support optional-arguments ctors

Seems reasonable but I'm with each passing patch I'm getting more concerned over our use (excessive?) of REALLY_INLINE.  Still tempted to do a speed/size analysis on this aspect when I find some time.

Comment 16

[Steven Johnson]

(In reply to comment #15)
> Seems reasonable but I'm with each passing patch I'm getting more concerned
> over our use (excessive?) of REALLY_INLINE.  Still tempted to do a speed/size
> analysis on this aspect when I find some time.

Fair point -- the theory I'm operating under on this is that (1) it allows for pay-as-you-go (no need to worry about dodgy linkers missing dead-strip opportunities), (2) "unused" calls we generate have zero cost (aside perhaps some increased compile-time), and (3) the net code size of an inlined constructObject() call of this sort is comparable to that of the old varargs-printf-style constructObject() call.

Of those assumptions, the first two seem pretty sound, but the third probably needs verification.

Comment 17

[Steven Johnson]

6027:b37477d7dcf7