Re: PATCH RFA: Support -Wmissing-declarations in C++

[Gabriel Dos Reis <gdr at cs dot tamu dot edu>]

On Thu, 14 Dec 2006, Ian Lance Taylor wrote:

| Gabriel Dos Reis <gdr@cs.tamu.edu> writes:
|
| > (2) That assumption used to hold 20 years ago for C++.  However, it is less
| >     verified for modern C++.  For good or bad, most of libraries
| >     come in header files (because they are either templates or inline
| >     functions) and there no longer is a clear separation between
| >     implementation and "interfaces" (and those are subject to pretty
| >     strong ODR requirements, which makes the kind of mistakes you
| >     would like to prevent very unlikely, but the implementation would
| >     generate lot of noise).
| >
| >     Consequently, the premise (1) when mechanically enforced without
| >     considerations of how modern C++ programs are written will act
| >     like a virus: it will fired up not only in the implementation
| >     files that was supposed to be checked, but also in third party
| >     libraries used as components (because many of them come in header
| >     files).  [This has been happening for real with -Weffc++.]
| >
| >     You can reduce the virus effect by not flagging inline functions,
| >     function templates (whether declared inline of not).
| >
| >     It is also helpful to consider libraries freely available to see
| >     how people write codes and how it will affect them.
|
| The code already ignores templates.  I modified it to also ignore
| functions explicitly declared inline.

I would like you also don't warn function definitions that are inline,
but without the keyword "inline" appearing in the text.  See below.
In C++, there is no point distinhuising between functions that are
"explicilty" declared inline and those that are not.

| What do you think of this version of the patch?

Does it handle the following correctly (e.g. no warning)?

    template<typename T>
       struct ArithOps {
          friend T operator+(T lhs, T rhs)           // #1
          {
              T t = lhs;
              t += rhs;
              return t;
          }
       };

    struct Int : ArithOps<Int> {
      // ....

         Int& operator+=(Int) { return *this; }
    };

    int main()
    {
       Int x, y;
       Int z = x + y;
     }

The operator+ in line #1 is not a function template, it is not a
member function, it is an inline (even if the keyword "inline" is
syntactically missing).   It is a member of the global scope.
Typically, it is NOT declared before being defined.

[...]

| (I will try to say again what I tried to say earlier: this warning is
| for a particular coding style.  If you don't use that coding style,
| then don't use this warning.  gcc has dozens of warnings.  I think
| it's silly to argue that each warning not included in -Wall
| constitutes a moral hazard.  People who enable warnings get what they
| asked for.)

The trouble with that reasoning is that it does not match reality.
People who use coding style switches mechanically enforced by the
compiler don't think they deserve what they get. Another trouble is
that they tend to act like viruses and spread through third-party
libraries forcing people to code in particular way, even when that
does not just make sense (refer to my previous messages as to how that
spreads).  Please again consider -Weffc++ to see what I mean.
What would be silly is that we do not think interactions with
idiomatic constructs carefully through.

-- Gaby