Closed
Bug 209667
Opened 21 years ago
Closed 21 years ago
can't hold a nsRefPtr to a class without empty ctor
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
People
(Reporter: sicking, Assigned: sicking)
References
Details
Attachments
(2 files)
980 bytes,
patch
|
jag+mozilla
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
2.30 KB,
patch
|
axel
:
review+
peterv
:
superreview+
|
Details | Diff | Splinter Review |
Right now you'll get a warning on mac and compileerror on some ports if you do
the following:
class A {
A(int i);
Addref();
Release();
int GetI();
}
nsRefPtr<A> a = ...;
a->GetI(); <- warns or fails to compiles
The reason is that we're returning nsRefPtr::operator-> returns an
nsDeriviedSafe<A>, however that class can not be instantiated because it uses a
default ctor but inherits a class with only non-empty ctors (which makes the
default ctor not compile).
So far I've worked around this by adding a non-implemented empty ctor to A, but
that's not too neat.
A better solution is to add a non-implemented ctor to nsDeriviedSafe. This
should work fine since we have other non-implemented non-virtual functions in
that class.
Proposed patch comming up
Assignee | ||
Comment 1•21 years ago
|
||
This patch fixes the problem in nsDerivedSafe
Assignee | ||
Comment 2•21 years ago
|
||
This patch removes the workarounds that I know of in the tree. I'll check this
in together with attachment 125822 [details] [diff] [review] to make sure that it works on all platforms.
Assignee | ||
Updated•21 years ago
|
Attachment #125822 -
Flags: superreview?(dbaron)
Attachment #125822 -
Flags: review?(jaggernaut)
Assignee | ||
Updated•21 years ago
|
Attachment #125823 -
Flags: superreview?(peterv)
Attachment #125823 -
Flags: review?(axel)
Assignee | ||
Updated•21 years ago
|
Status: NEW → ASSIGNED
Comment on attachment 125822 [details] [diff] [review]
Patch to nsDeriviedSafe
This looks good to me. You might want to check with scc as well if you can
reach him.
Attachment #125822 -
Flags: superreview?(dbaron) → superreview+
Comment on attachment 125822 [details] [diff] [review]
Patch to nsDeriviedSafe
Actually, there's a typo in the first line here: (Derivied instead of Derived):
>+ This ctor exists to avoid compile errors and warnings about nsDeriviedSafe using the
>+ default ctor but inheriting classes without an empty ctor. See bug 209667.
Also, probably "inheriting from" is better than "inheriting".
Comment 5•21 years ago
|
||
Comment on attachment 125823 [details] [diff] [review]
Patch to remove workarounds
I wonder if you could make somebody from ports test this before checking it in.
Attachment #125823 -
Flags: review?(axel) → review+
Updated•21 years ago
|
Attachment #125823 -
Flags: superreview?(peterv) → superreview+
Assignee | ||
Updated•21 years ago
|
Blocks: nsCOMPtr_tracking
Comment 6•21 years ago
|
||
I don't quite understand what's going on here... operator-> returns a pointer
(nsDerivedSafe<A>*), the default constructor for nsDerivedSafe<A> shouldn't have
to be called.
Unless the compiler isn't following the rule that it shouldn't complain about
stuff in templates until that part of the template is actually used.
Comment 7•21 years ago
|
||
Hmmm, though I'm not sure if that rule also applies to constructors.
Right, the constructor is never called, but the compiler is warning based on an
attempt to construct that constructor. If we say that we implement the
constructor (which is impossible to do, in general, since we don't know how to
construct the base class), then the compiler won't care unless it actually needs
that implementation.
Comment 9•21 years ago
|
||
Comment on attachment 125822 [details] [diff] [review]
Patch to nsDeriviedSafe
Hmm, okay.
Shouldn't that be in a |private| section though?
r=jag if you change it to private or explain to me why it should be protected.
Assignee | ||
Comment 10•21 years ago
|
||
I'm afraid that if we put it in the private-section we will get warnings about
the class only having private ctors and no friends and therefor is impossible to
instantiate.
Comment 11•21 years ago
|
||
Comment on attachment 125822 [details] [diff] [review]
Patch to nsDeriviedSafe
r=jag
Attachment #125822 -
Flags: review?(jaggernaut) → review+
Assignee | ||
Comment 12•21 years ago
|
||
checked in without causing any redness, so all is well
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 13•21 years ago
|
||
IIRC, there's workarounds for this bug in the editor code too, not sure where I
saw that tho...
Comment 14•21 years ago
|
||
I found these two on a quick trawl thru lxr. (didn't see any in editor)
http://lxr.mozilla.org/seamonkey/source/content/shared/public/nsStyleContext.h#155
http://lxr.mozilla.org/seamonkey/source/content/xbl/src/nsXBLPrototypeBinding.h#192
You need to log in
before you can comment on or make changes to this bug.
Description
•