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