[QoI] warn about non-static, non-inline, non-template function definitions in header files

[zygoloid]

Bugzilla Link	10004
Version	trunk
OS	Linux
CC	@gnzlbg

Extended Description

clang -Wall -Wextra produces no warnings for this:

// a.h
struct S {
void f();
};

void S::f() {}

// a.cpp
#include "a.h"
void g() {
S s;
s.f();
}

This issue is a particular efficiency drain since it doesn't manifest until link time, and requires a rebuild of all includers of the header.

Ideally, any definition which wholly comes from a header file and produces a non-weak symbol should warn. We'll need to be careful to weed out macros defined in the main source file and expanded in #included files.

[llvmbot]

You'd have to be careful with this. Here is a class from the boost library (noncopyable.hpp):

namespace noncopyable_ // protection from unintended ADL
{
class noncopyable
{
protected:
noncopyable() {}
~noncopyable() {}
private: // emphasize the following members are private
noncopyable( const noncopyable& );
const noncopyable& operator=( const noncopyable& );
};
}

This class is intended to be inherited from. It is supposed to guard against unwanted assignment and copy construction. It has intentionally empty constructor and destructor definitions, which shouldn't cause any issues.

[zygoloid]

I don't see how boost::noncopyable is in any way a special case. The constructor and destructor are inline, and hence produce weak symbols, so no warning would be emitted. The copy constructor and copy assignment are not defined, so no warning would be emitted.

[llvmbot]

Ahh, sorry, I misunderstood.

[gnzlbg]

+1, I just found a couple of functions that were not marked inline when trying to use modules on a large code base. I got linker errors that were tedious to track down.

It would be great if this was at least an opt-in warning (e.g. even if its not enabled by default).

[gnzlbg]

Also, this warning should be enabled by default when using -fmodules since otherwise modules do not work.