209667 - can't hold a nsRefPtr to a class without empty ctor

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to nsDeriviedSafe

This patch fixes the problem in nsDerivedSafe

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to remove workarounds

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.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

[Axel Hecht]

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.

Comment 6

[jag (Peter Annema)]

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

[jag (Peter Annema)]

Hmmm, though I'm not sure if that rule also applies to constructors.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

[jag (Peter Annema)]

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.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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

[jag (Peter Annema)]

Comment on attachment 125822 [details] [diff] [review]
Patch to nsDeriviedSafe

r=jag

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

checked in without causing any redness, so all is well

Comment 13

[Johnny Stenback (:jst)]

IIRC, there's workarounds for this bug in the editor code too, not sure where I
saw that tho...

Comment 14

[[e-mail]]

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