297146 - gcc warning in nsTString.h

Status: RESOLVED WORKSFORME

(Core :: XPCOM, defect)

Description

[Braden]

gcc complains (via warning) that nsTAdoptingString_CharT's copy constructor does
not explicitly invoke the base class copy constructor.

Comment 1

[Darin Fisher]

Is this causing any problems?

Comment 2

[Braden]

Attachment: Patch

Comment 3

[Braden]

Problems as in "causing incorrect code to be generated"? Not AFAIK.

Problems as in "causing gcc to emit a warning where it could be avoided with a
reasonable fix"? Yes... It's just an annoyance.

Comment 4

[Darin Fisher]

Comment on attachment 185738 [details] [diff] [review]
Patch

>Index: public/nsTString.h

>       nsTAdoptingString_CharT( const self_type& str )
>+        : nsTXPIDLString_CharT(str)
>         {
>           *this = str;
>         }

So, this seems to result in Assign(str) being called twice.  That
can't be right.

Comment 5

[Braden]

Um, yeah. Apparently I wasn't paying very close attention. Sorry. Revision coming.

Comment 6

[Braden]

Actually, the patch shouldn't introduce any problem that isn't already there. As
I understand it, the base class copy constructor is being called implicitly in
the current code.

Comment 7

[Darin Fisher]

Well then... it seems we found another bug ;-)

Comment 8

[Braden]

Attachment: Patch

I think it is sufficient simply to eliminate the explicit assignment to *this.
The effect of operator= seems only to be a call to Assign; which as you note is
called in a base class.

Comment 9

[Darin Fisher]

Comment on attachment 185920 [details] [diff] [review]
Patch

Wait a second... this isn't correct either.  operator= is special in that it
mutates |str|.	This code changes seems to break that.

Comment 10

[Darin Fisher]

I don't understand why the original code was wrong.  It is calling the default
constructor for the base-class, which is what we want.  Then it calls operator=,
which transfers ownership of the string buffer.

I think the problem is that GCC is complaining about something benign.

I think this bug is INVALID.  We should find a way to instruct GCC to shut-up.

Comment 11

[Braden]

In that case, the right thing is probably to call the default ctor explicitly.

Comment 12

[Darin Fisher]

ok

Comment 13

[Atul Aggarwal]

Could not find any warning in current logs of nightly build for linux and macos.

Closing this issue.