Bug 11159 - erroneous warning in copy ctor with virtual inheritance
erroneous warning in copy ctor with virtual inheritance
Status: RESOLVED DUPLICATE of bug 5645
Product: gcc
Classification: Unclassified
Component: c++
3.3
: P2 normal
: ---
Assigned To: Not yet assigned to anyone
: diagnostic
: 11804 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2003-06-11 19:08 UTC by Boris Kolpackov
Modified: 2008-02-13 11:13 UTC (History)
10 users (show)

See Also:
Host: i386-gnu-linux
Target: i386-gnu-linux
Build: i386-gnu-linux
Known to work:
Known to fail:
Last reconfirmed: 2007-11-16 13:58:57


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Boris Kolpackov 2003-06-11 19:08:29 UTC
Compiling the following code 

struct A
{
  A ();
};

struct B : virtual A
{
  B ();
};

struct C : virtual A
{
  C ();
};

struct D : B, C
{
  D (D const&){}
};

reults in the following diagnistic:

#g++ -W -c ./copyctor.cpp

copyctor.cpp: In copy constructor `D::D(const D&)':
copyctor.cpp:18: warning: base class `struct A' should be explicitly 
   initialized in the copy constructor
copyctor.cpp:18: warning: base class `struct B' should be explicitly 
   initialized in the copy constructor
copyctor.cpp:18: warning: base class `struct C' should be explicitly 
   initialized in the copy constructor
Comment 1 Wolfgang Bangerth 2003-06-16 14:26:56 UTC
What is wrong with the warning?
W.
Comment 2 Boris Kolpackov 2003-06-16 15:27:58 UTC
AFAIK standard says that if there is a default ctor availbale then it should be
used for virtual base. This warning is at least inconsistent with the fact that
there is no such warning for any other ctor type. For example if I replace
definition of class D above with something like this

class D : B, C
{
  D () {}
  D (int) {}
};

there won't be any diagnostic.

-boris
Comment 3 Wolfgang Bangerth 2003-06-16 15:52:48 UTC
Yes, I think I can sort-of agree. I mean, the warning is not wrong, but
it's not overly helpful in this case. It is just thought of as a reminder
that you need to initialize virtual base classes in every derived class,
which is different from classes without virtual bases. So it is good style
to make it explicit. If the base class had no default constructor, it
would be an outright error, by the way.

I leave it to others to decide what to do with this. I think, it's easy to
work around this, it removes a potential source in your code, and it improves
your code quality (stylewise), so the warning does indeed have its justification.

Gaby, your call?

W.
Comment 4 Boris Kolpackov 2003-06-16 20:49:52 UTC
> Yes, I think I can sort-of agree. I mean, the warning is not wrong, but
> it's not overly helpful in this case. It is just thought of as a reminder
> that you need to initialize virtual base classes in every derived class,
> which is different from classes without virtual bases.

This applies to any ctor. However warning is issued only for copy ctor.

> So it is good style to make it explicit.

This is arguable. For instance one may have a huge hierarchy (like syntax 
tree) and it's designed to use default ctor then making calls to those ctors 
explicit is nothing but code bloat. 

> I leave it to others to decide what to do with this. I think, it's easy to
> work around this, it removes a potential source in your code, and it 
> improves your code quality (stylewise), so the warning does indeed have 
> its justification.

The idea behind this warning may have its justification. However its 
implementation is inconsistent.
Comment 5 Boris Kolpackov 2003-06-17 14:52:32 UTC
BTW, if you look at the original example you will notice that B and C are not
virtual bases of D. After trying the following example 

struct A {};

struct B : A
{
  B (B const&) {}
};

I got this:

bash-2.05a$ g++ -W -c ./copyctor.cpp 
copyctor.cpp: In copy constructor `B::B(const V&)':
copyctor.cpp:20: warning: base class `struct A' should be explicitly 
   initialized in the copy constructor

So apparently it has nothing to do with virtual inheritance. Also I still 
believe there is no ground to issue this warning.

-boris
Comment 6 Andrew Pinski 2004-04-26 19:46:32 UTC
Your last example does not give a warning on 2.95.3, 3.0.4, 3.2.3, 3.3.3, 3.4.0 or the 
mainline.

Your first example still gives a warning on the mainline.
Comment 7 Andrew Pinski 2004-04-27 01:55:58 UTC
*** Bug 11804 has been marked as a duplicate of this bug. ***
Comment 8 Nigel Stewart 2004-10-01 16:42:27 UTC
It would be very useful to be able to specifically
suppress this warning message - 3rd party headers
are downing out the more useful information in 
our build logs.
Comment 9 William R. Otte 2006-03-09 22:44:31 UTC
In some cases, this warning can also be impossible to address and may be buggy/erroneous.  Consider the following example:
========
struct A
{
  A ();
};

struct B : virtual A
{
  B ();
};

struct C :  virtual B
{
  C ();
};

template <typename Base>
struct E : Base
{
  E ();

  E (E const &)
  {
  };
};

E<C> foo;
E<C> bar (foo);
====
Produces the following diagnostic:

test.cpp: In copy constructor 'E<Base>::E(const E<Base>&) [with Base = C]':
test.cpp:27:   instantiated from here
test.cpp:21: warning: base class 'struct A' should be explicitly initialized in the copy constructor
test.cpp:21: warning: base class 'struct B' should be explicitly initialized in the copy constructor
test.cpp:21: warning: base class 'struct C' should be explicitly initialized in the copy constructor

If I make an immediately obvious fix by explicitly initializing Base:

========
struct A
{
  A ();
};

struct B : virtual A
{
  B ();
};

struct C :  virtual B
{
  C ();
};

template <typename Base>
struct E : Base
{
  E ();

  E (E const &)
    : Base ()
  {
  };
};

E<C> foo;
E<C> bar (foo);

====

I am presented with the following diagnostic:
test.cpp: In copy constructor 'E<Base>::E(const E<Base>&) [with Base = C]':
test.cpp:28:   instantiated from here
test.cpp:21: warning: base class 'struct A' should be explicitly initialized in the copy constructor
test.cpp:21: warning: base class 'struct B' should be explicitly initialized in the copy constructor

In this case, E can't explicitly initialize A or B, because E can't assume that either is present as a parent of Base.  This is also likely erroneous because E shouldn't have to explicitly initialize the parents of Base... That should be handled by the copy constructor of Base.



Comment 10 Roman Fietze 2006-04-04 12:26:36 UTC
The following snippet also gives me errors, and here, as a developer, I did all I could do:
-----
#include <sstream>
class MyStream : public std::ostringstream
{
  public:
    inline MyStream() : std::ostringstream() {}
    inline MyStream(const MyStream &ms) : std::ostringstream()
        { std::ostringstream::str(ms.str()); }
};
-----

The result, as before:

$ gcc --version
gcc (GCC) 3.3.5 20050117 (prerelease) (SUSE Linux)
...
$ g++ -W -c mystream.cc
mystream.cc: In copy constructor `MyStream::MyStream(const MyStream&)':
mystream.cc:13: warning: base class `struct std::basic_ios<char, 
   std::char_traits<char> >' should be explicitly initialized in the copy 
   constructor

I do not have access to std::basic_ios from MyStream. Solution here?
Comment 11 Manuel López-Ibáñez 2007-01-09 14:50:28 UTC
There is an unreviewed patch to name this warning in the patch queue:
http://gcc.gnu.org/ml/gcc-patches/2007-01/msg00520.html

I guess it doesn't solve all the inconsistencies mentioned here but at least it can be individually disable.

Also, it could have better testcases and perhaps an apter name: feedback is always welcome.
Comment 12 Manuel López-Ibáñez 2007-02-09 14:03:28 UTC
Also related to this warning: PR 5645.
Comment 13 Jonathan Wakely 2007-11-14 13:27:20 UTC
(In reply to comment #10)
> 
> I do not have access to std::basic_ios from MyStream. Solution here?

Yes you do.  The warning can be prevented by initialising the virtual base:

    MyStream() : std::ios(), std::ostringstream() {} 

However in general I agree that the warnings are bogus. If there is no suitable constructor for the virtual base you get an error, so the warning is only issued when the behaviour is perfectly well-defined, and in many cases probably exactly what the author of the code wanted.

Even the text of the warning is misleading. Why "should" it be explicitly initialised, when the standard says it will be implicitly initialised if a suitable default constructor exists?

If you want to use a non-default constructor for the virtual base then you need to call it explicitly. But if the default constructor is what you want, why should you have to do extra work?
Comment 14 Manuel López-Ibáñez 2007-11-16 13:58:57 UTC
(In reply to comment #13)
> Even the text of the warning is misleading. Why "should" it be explicitly
> initialised, when the standard says it will be implicitly initialised if a
> suitable default constructor exists?
> 
> If you want to use a non-default constructor for the virtual base then you need
> to call it explicitly. But if the default constructor is what you want, why
> should you have to do extra work?
> 

I wonder as well. Removing this warning will close at least two PRs...
Comment 15 Manuel López-Ibáñez 2008-02-13 11:13:02 UTC
I am going to close this as a duplicate of 5645 and post a patch there that includes the testcases of both PRs. Both bugs are about the definition (or lack of it) of this warning.

*** This bug has been marked as a duplicate of 5645 ***
Comment 16 Jason Merrill 2008-02-14 23:11:49 UTC
Subject: Bug 11159

Author: jason
Date: Thu Feb 14 23:11:04 2008
New Revision: 132324

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=132324
Log:
        PR c++/5645
        PR c++/11159
        * class.c (type_has_user_nondefault_constructor): New fn.
        * cp-tree.h: Declare it.
        * init.c (emit_mem_initializers): Use it for -W warning about
        missing base initializer.

Added:
    trunk/gcc/testsuite/g++.dg/warn/pr11159.C
    trunk/gcc/testsuite/g++.dg/warn/pr5645.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/class.c
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/cp/init.c
    trunk/gcc/testsuite/g++.dg/warn/Wreorder-1.C