636517 - If you forget to add a create function when using GC_AS3_EXACT you crash assert in hard to understand ways

Status: RESOLVED FIXED

(Tamarin Graveyard :: Garbage Collection (mmGC), defect, P1)

Description

[Tommy Reilly]

We should either:

1) make them add it by not compiling somehow
2) provide it for them 
3) invoke new directly since the generated createInstanceProc will be the only caller and that's what the non-exact case does.

Comment 1

[Tommy Reilly]

For now I've just used macros to get these into the code with little typing, then I don't feel like I've undone the work Steven did to remove the createInstance methods.   I think we can do better.

Comment 2

[Steven Johnson]

I could live with any of these; IMHO (3) is best since AS3 objects should (generally) only be created via the class constructor anyway.

Comment 3

[Tommy Reilly]

So #3 is a simple tweak to nativegen.py, but then I have to remove all the create methods in tamarin that are otherwise not used, Lar's you down with this plan?

Comment 4

[Steven Johnson]

Would be interested to know what is in the "otherwise not used" category... hopefully nothing.

Comment 5

[Tommy Reilly]

I meant in tamarin core, we have custom create functions for a bunch of different core builtins there.

Comment 6

[Tommy Reilly]

Attachment: Make things better

- you can never forget MMgc::kExact now
- still have protected ctors' but objects have to friend the class
- all create calls never called from non-generated code are gone

All in all should be more foolproof for our users.

Comment 7

[Tommy Reilly]

note b/c this forces MMgc::kExact this patch is blocked by the dictionary issue

Comment 8

[Lars T Hansen]

Comment on attachment 514951 [details] [diff] [review]
Make things better

I don't understand what "hosed" means in the title.  Does it not compile?  Does it crash?  What happens?

Technically the patch seems fine.

In terms of usability I'm not 100% sure it's a good idea because it creates yet another special case for the user: you should write a create method /except/ if it's a glue class in which case it will be generated for you...

(Not taking a position on the review until the bug's been clarified further.)

Comment 9

[Tommy Reilly]

(In reply to comment #8)
> I don't understand what "hosed" means in the title.  Does it not compile?  Does
> it crash?  What happens?

It means you end calling some C++ ancestors create method instead of yours and your object ends up smaller than you need, your ctor didn't run and you usually get overwrite asserts.

> In terms of usability I'm not 100% sure it's a good idea because it creates yet
> another special case for the user: you should write a create method /except/ if
> it's a glue class in which case it will be generated for you...

No I think the new rule is you never write a create method unless you want to.   For GC_AS3_EXACT the generated will invoke new directly so don't bother with a create method unless your objects are created from C++ (doesn't happen except in the tamarin core).   Really I think the use of a create method is just a "best practice" for GC_CPP_EXACT classes.

Comment 10

[Lars T Hansen]

Comment on attachment 514951 [details] [diff] [review]
Make things better

I get it.

Comment 11

[Tommy Reilly]

changeset: 5987:4bdf638db180
user:      Tommy Reilly <[e-mail]>
summary:   Bug 636438 - make GC_AS3_EXACT tracing not require create method
(r=stejohns,sr=lhansen)

http://hg.mozilla.org/tamarin-redux/rev/4bdf638db180

Comment 12

[Tommy Reilly]

I screwed the bug #'s but there she is