Re: [boost] Supressing warnings

From: Oliver Kullmann (O.Kullmann_at_[hidden])
Date: 2006-06-27 14:40:49

On Tue, Jun 27, 2006 at 02:37:13PM +0100, Reece Dunn wrote:
> Oliver Kullmann wrote:
> > > NOTE: Instead of disabling the warnings, you can remove the warning by
> > > updating the code. For example:
> > > 
> > > void foo( int bar ){} // unreferenced parameter
> > > void foo( int bar ){ bar; } // no warning!
> > >
> > 
> > this is FALSE:
> 
> No - it is one way to prevent the bug.
>

bug ? there is no bug here.
I guess you meant "warning" --- but your "repair" is meaningless,
since it introduces a meaningless expression.
And, for good reasons, g++ warns about this(!) (at least versions 4.0.2 and above):

Test.cpp: In function ‘void foo(int)’:
Test.cpp:1: warning: statement has no effect

One should not "repair" code just because of the stupidity of one compiler.

 
> > void foo(int){} 
> 
> This doesn't work if you have:
> 
> /** @brief something
>    * @param bar Oh dear, this is missing!
>    */
> void foo( int bar ){}
> 
> Either not specifying bar or commenting it out will mean that Doxygen will not
> be able to identify the bar parameter.
> 

again, this has nothing to do with the code, but is a weakness of Doxygen;
so one can write a request w.r.t. Doxygen, but should not harm the code.

> 
> > > class foo
> > > {
> > >    bar & m_bar;
> > >    foo( bar & b ): m_bar( b ){}
> > > 
> > >    // disable the no default compiler assignment operator warning:
> > >    inline foo & operator=( const foo & );
> > > };
> > 
> > again this is a perversion: the C++ should follow the LANGUAGE, and the special
> > handling of the *special member function* is important part of C++.
> 
> No. You miss the issue here. The problem is that m_bar is a *reference* and
> *you can't assign a reference*!
>

yes, that what I mean: in this case explicitely disabling the copy assignment
is what is *needed*, and in this case a compiler warning is good, and helps
us improving the code.
 
> > In the above example, first "inline" shouldn't be there, and then it's not
> > about warnings, but about semantics: either we want the implicitly declared
> > copy assignment operator, and then we *must not* add that line, or we don't
> > want it, and then we *must* add the line --- there is no choice.
> 
> The inline *should* be there.

I meant really the keyword "inline", that is, I meant it should be

// disable the no default compiler assignment operator warning:
foo & operator=( const foo & );

since in-class definitions are automatically inline, so in-class definitions
should never use the keyword "inline".

> This is like deriving from boost::noncopyable which
> uses the above trick to prevent copying. In this case, I don't think it is possible to
> use noncopyable as this is about the default assignment operator in foo. By adding
> the line:
> 
>    inline foo & operator=( const foo & );
> 
> I am telling the compiler that I am providing my own assignment operator, so
> the compiler doesn't automatically generate one. This is the classic way to
> implement non-copyable behaviour (as well as providing a similar copy constructor).
>

I know --- as I said, the "inline" is not needed (and thus misleading --- "for what is
this there?" will the maintenance programmer ask --- "some strange language issues???").
 
> In fact, this highlights a potential issue in the code. The standard says that you
> can't change a reference, so the assignment operator will copy values not what
> is being pointed to. This can lead to a bug if you assign one foo to another. In
> this case the warning has identified a potential problem and the above will identify
> any places where that problem is manifest.
>

yes, this warning is justified. And different from the above examples, here we really
improve the code, by stating explicitely that this class has no copy assignment operator.
 
Oliver