This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: a barely useful option?


[ Usefulness of -Weffc++ ]

For a snapshot or two, -Wall on C++ code implied -Weffc++.  I convinced
Jason to take it out again with roughly the argument below.  Note that
this argument doesn't apply to all the -Weffc++ warnings, only to certain ones.

> With a reasonably implemented standard library this options is quite
> useful! My implementation of the IOStream library currently issues only
> warnings if deprecated library features are used even with maximum warning
> level (-W -Wall -Weffc++ -ansi -pedantic).

While any given piece of code can be improved in this respect, it would
be a very bad idea for any organization to mandate no -Weffc++ warnings,
as there are some cases where the only way to achieve this is to make
one's code substantially worse.  Example: you decide to write a set of
template classes to handle reference counting, using the envelope and
letter pattern.  We want to use templates, but we also don't wnat object
code bloat.  So the design looks like

class BaseRefCountRep
	the base representation object.  It has one field, a reference count,
	and has a virtual destructor.

class BaseRefCount
	the base envelope object.  It has one field, a pointer to
	BaseRefCountRep.  Its assignment operator, copy constructor, and
	destructor implement the usual reference counting; the BaseRefCountRep
	is deleted when the reference count drops to zero.

template<class T> class RefCount
	privately derived from BaseRef.  There are no additional data fields.
	We assure that the pointed-to BaseRefCountRep is actually
	a RefCountRep<T>.

template<class T> class RefCountRep
	privately derived from BaseRefCount.  It has an additional data
        field, a T.

One problem with this design is that, because of the lack of overloaded
operator dot, we can't make a RefCount<T> look just like a T.  It's
still posssible, though, to derive from RefCount<T> a class that forwards
methods to the representation object.

So what's the problem?  The problem is that BaseRefCount does not have
a virtual destructor.  *This is deliberate.*  What it means is that,
at the implementation level, a RefCount<T> is only a pointer.  This means
that it can be passed around in a register, that the ADDRESSOF
optimizations present in GNU C++ will greatly improve inline functions
that use the class, etc.  But -Weffc++ will not shut up until this is
changed.  It requires that any derived-from object have a virtual
destructor.  But that is neither needed nor wanted in this case: we
*want* the base destructor to be used for all derived classes, because
it would never make sense for any of the derived classes to be anything
but a pointer!  (Yes, someone could misuse the class.  But we simply
document the class explaining how it is to be used).

Note that -Wall already warns about classes that have virtual functions
but no virtual destructor.  But -Weffc++ warns about any base class
that has a non-virtual destructor, in all cases.  At the least, it makes
no sense to issue this warning in the case of private derivation, because
in that case no user will associate a Derived with a Base reference (only
code implementing the class can do that).

If the implementation were effortless, the warning could be put in but
modified to behave as follows: OK, so the base class has a non-virtual
destructor.  Consider this code:

	Base* b = new Derived(constructor-args);
	....
	delete b;

will the right thing happen?  Well, if Derived has no additional data
members and does not declare a destructor, the right thing does happen, so
the compiler should never issue a warning in such a case.






virtual destructor
> >  useful, I am not sure whether it's the option that can be removed or
> >  the stdlibc++ fixed according to Scott Meyers' Effective C++ books.
> 
> I think the library should be fixed. In any case, even if the library is
> not fixed, the warning should not be removed: There will be a library which
> will not cause any warnings and especially new users of C++ will benefit
> from this warning.
> 
> Regards,
>   Dietmar
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]