Closed
Bug 871995
Opened 11 years ago
Closed 11 years ago
Fix rooting compilation failure in ObjectWrapperParent.cpp
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: sfink, Assigned: sfink)
Details
Attachments
(1 file)
8.35 KB,
patch
|
till
:
review+
sfink
:
checkin+
|
Details | Diff | Splinter Review |
js/ipc/ObjectWrapperParent.cpp fails to compile with the exact rooting enabled due to the patch from bug 871182: Rooted<AutoResolveFlag> is not a valid type. Sadly, this is only detected when you enable the rooting analysis, because only then does the template instantiation notice that it requires a root 'kind', which is really because you can't use any arbitrary T for Rooted<T>; you can only use T for which a couple of instantiations are defined (RootMethods<T>::RootKind or something like that is one; something that would actually trace your T is another.)
Assignee | ||
Comment 1•11 years ago
|
||
The solution is to have AutoResolveFlag change its mObj field from JSObject* to Rooted<JSObject*>. It's an Auto* class, which implies a stack lifetime, so it can contain Rooted fields. This file is also |using namespace JS| but explicitly qualifying all kinds of stuff with JS::. I removed the ones that are unambiguous. Some, like JS::Class, I left qualified because I think it's helpful documentation. Perhaps it would better to qualify all of them and stop using the namespace? I don't know what the standards are for that, but I doubt it matters much.
Attachment #749268 -
Flags: review?(tschneidereit)
Comment 2•11 years ago
|
||
Comment on attachment 749268 [details] [diff] [review] Fix rooting compilation failure in ObjectWrapperParent.cpp Review of attachment 749268 [details] [diff] [review]: ----------------------------------------------------------------- When I was thinking "how does this work", I should have taken that as a warning sign, perhaps. ::: js/ipc/ObjectWrapperParent.cpp @@ +353,5 @@ > return false; > *to = OBJECT_TO_JSVAL(obj); > return true; > } > Not that I care too much to make you touch this file again, but there's some rogue whitespace here.
Attachment #749268 -
Flags: review?(tschneidereit) → review+
Assignee | ||
Updated•11 years ago
|
Attachment #749268 -
Flags: checkin+
Assignee | ||
Comment 3•11 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/01072a33f2ed
Comment 4•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/01072a33f2ed
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in
before you can comment on or make changes to this bug.
Description
•