Closed
Bug 1025395
Opened 7 years ago
Closed 6 years ago
mach's webidl-example uses NS_IMPL_CYCLE_COLLECTION_INHERITED_0, which no longer exists
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: qdot, Assigned: bzbarsky)
References
Details
Attachments
(3 files)
mach needs to be updated to use NS_ADDREF_IMPL_INHERITED, then list the attributes that should be collected. This will require figuring out types from the attributes in the webidl interface.
Updated•7 years ago
|
Component: mach → DOM
![]() |
Assignee | |
Comment 1•7 years ago
|
||
The example generator doesn't output any data members. It also doesn't output NS_ADDREF_IMPL_INHERITED_0 anywhere...
Reporter | ||
Comment 2•7 years ago
|
||
Reporter | ||
Comment 3•7 years ago
|
||
Reporter | ||
Comment 4•7 years ago
|
||
I've attached the webidl and the corresponding cpp file put out by running mach webidl-example. There's a NS_ADDREF_IMPL_INHERITED_0 there.
![]() |
Assignee | |
Updated•7 years ago
|
Attachment #8440324 -
Attachment mime type: text/x-c++src → text/plain
![]() |
Assignee | |
Updated•7 years ago
|
Attachment #8440323 -
Attachment mime type: text/x-csrc → text/plain
![]() |
Assignee | |
Comment 5•7 years ago
|
||
> There's a NS_ADDREF_IMPL_INHERITED_0 there.
There is? Where?
I see NS_IMPL_ADDREF_INHERITED and I see NS_IMPL_CYCLE_COLLECTION_INHERITED_0. Did you mean the latter?
If so, that never existed, as far as I know. You're supposed to get a compile error and actually use the right thing there that CCs your members, or remove the CC bits if you have no members to cycle collect... I'm definitely up for changes to the output to make that clearer.
Reporter | ||
Comment 6•7 years ago
|
||
Wow. Ok. I seem to have caught some sort of macro related dyslexia, sorry about that. They're like, all right next to each other. :| Will update bug. Anyways, I meant NS_IMPL_CYCLE_COLLECTION_INHERITED_0. To fix it for the moment, I ended up just using NS_IMPL_CYCLE_COLLECTION_CLASS to initially get things to compile, but it sounds like the solution might be to have an error macro in there for when people try to just cut/paste like I did?
Reporter | ||
Updated•7 years ago
|
Summary: mach's webidl-example uses NS_ADDREF_IMPL_INHERITED_0, which no longer exists → mach's webidl-example uses NS_IMPL_CYCLE_COLLECTION_INHERITED_0, which no longer exists
![]() |
Assignee | |
Comment 7•7 years ago
|
||
Yeah, that might make sense.
Comment 8•6 years ago
|
||
The code that outputs NS_IMPL_CYCLE_COLLECTION_INHERITED_0 is here http://mxr.mozilla.org/mozilla-central/source/dom/bindings/Codegen.py#13395 13391 if self.parentIface: 13392 ccImpl = dedent(""" 13393 13394 // Only needed for refcounted objects. 13395 NS_IMPL_CYCLE_COLLECTION_INHERITED_0(${nativeType}, ${parentType}) 13396 NS_IMPL_ADDREF_INHERITED(${nativeType}, ${parentType}) 13397 NS_IMPL_RELEASE_INHERITED(${nativeType}, ${parentType}) 13398 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(${nativeType}) 13399 NS_INTERFACE_MAP_END_INHERITING(${parentType}) 13400 13401 """) maybe that should be NS_IMPL_CYCLE_COLLECTION_INHERITED(${nativeType}, ${parentTypes}) with parentTypes being the list of parent types? Maybe this depends on whether the classes in the list are nsISupports or only the last one? What does webidl-example output for the members of the list parentTypes?
![]() |
Assignee | |
Comment 9•6 years ago
|
||
> maybe that should be No, it shouldn't. See comment 5. Maybe we should put that into an actual comment in the generated code....
Comment 10•6 years ago
|
||
How about issuing something like: #error if the class has refcounted members then add them variables NS_IMPL_CYCLE_COLLECTION_INHERITED(${nativeType}, ${parentType}, mMember1, mMember2) // nsCOMPtr<nsIMember1Class> mMember1; see WebIDL_bindings instead of the current NS_IMPL_CYCLE_COLLECTION_INHERITED_0 code that yields a cryptic error message."... expected constructor, destructor, or type conversion before ‘MozExternalRefCountType’" -Axel
![]() |
Assignee | |
Comment 11•6 years ago
|
||
Or maybe we should actually have a NS_IMPL_CYCLE_COLLECTION_INHERITED_0? I see some service worker code working around its lack with an mCCDummy member...
Flags: needinfo?(continuation)
Comment 12•6 years ago
|
||
(In reply to Not doing reviews right now from comment #11) > Or maybe we should actually have a NS_IMPL_CYCLE_COLLECTION_INHERITED_0? I > see some service worker code working around its lack with an mCCDummy > member... I think that can be eliminated. I have a patch.
Flags: needinfo?(continuation)
Comment 13•6 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #12) > I think that can be eliminated. I have a patch. I filed bug 1166488. Anyways, the basic idea is that generally any time you might want to use INHERITED_0 you can instead just not declare any CC or ISUPPORTS stuff and inherit it all from your parent. I suppose there could be some situation wherein you implement an additional nsIFoo interface, but don't have any additional CCed members, and then maybe you'd need it. Also, you may want to override AddRef and Release to get better error messages, again without having any additional CCed members. So I guess there are some use cases.
![]() |
Assignee | |
Comment 14•6 years ago
|
||
Attachment #8607758 -
Flags: review?(peterv)
![]() |
Assignee | |
Updated•6 years ago
|
Assignee: kyle → bzbarsky
Status: NEW → ASSIGNED
Updated•6 years ago
|
Attachment #8607758 -
Flags: review?(peterv) → review+
Comment 16•6 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2579eae7696b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Updated•2 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•