817818 - Miscellaneous rooting fixes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

A collection of fixes that I have accumulated while working on various things.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Miscellaneous rooting fixes

I should really mark this f? instead of r?, since you're going to r- it for the UnrootedScript/UnrootedFunction stuff. I really ought to add some more general header-setting to bzexport...

Anyway, this r? is really a question about how to declare RawT and UnrootedT. I couldn't just use your existing ForwardDeclare(type) #define because:

1. The 'JS' removal in JSScript -> UnrootedScript
2. Namespaces. JSScript is in the global namespace. Unrooted<T> is inside js::gc::, I think. UnrootedScript probably ought to end up in JS or somewhere.
3. class vs struct -- currently, you forward-declare a class. JSScript is a struct. MSVC, at least, isn't going to be happy about that.

Comment 2

[Terrence Cole [:terrence]]

Comment on attachment 687951 [details] [diff] [review]
Miscellaneous rooting fixes

Review of attachment 687951 [details] [diff] [review]:
-----------------------------------------------------------------

I've currently got all of the changes you made to script pointers in a separate patch, so this saves me a bit of work.  I'm going to go ahead and r+ this under the assumption that we will be able to hammer out a decision on what to do about the forward declarations today.  Even if we don't, it's easy enough to fix later.

What I have right now is this unbelievable ugly hack:

# define ForwardDeclareJS(type)                                               \
    class JS##type;                                                           \
    namespace js {                                                            \
        typedef Unrooted<JS##type*> Unrooted##type;                           \
        typedef JS##type * Raw##type;                                         \
    }                                                                         \
    class JS##type

Also, thank you for the reminder abound |struct|!  Maybe this would be a good time to make these |class JSFoo { public: }|?

Comment 3

[Steve Fink [:sfink] [:s:]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/1d71ba26519f

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1d71ba26519f

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

This landed with the struct/class mismatch problem that spews warnings on MSVC :-(