Why does clang-tidy recommend deleted member functions should be public?

The modernize-use-equals-delete check despite not saying much about it in the docs thinks that deleted member functions should be public (https://github.com/llvm-mirror/clang-tools-extra/blob/master/test/clang-tidy/modernize-use-equals-delete.cpp#L145) and it’s thus going off on Qt’s Q_DISABLE_COPY (http://doc.qt.io/qt-5/qobject.html#Q_DISABLE_COPY) macro.

The guidelines from Qt are that this macro be private, not public, and it’s unclear to us why clang-tidy thinks so. Could anyone shed some light on the reasoning?

Thanks,

Vadim

There’s nothing inherently wrong with deleting member functions that are private, but if they are public, you will get a better error message (namely that the function is deleted) instead of an error message that says that the functions are private.

AFAIK, Qt supports C++11, but also previous versions. I don’t know how they implemented Q_DISABLE_COPY, but it probably expands to copy constructor and copy assignment declarations before C++11, and if you enable C++11 mode, deletes them instead. In pre-C++11, the declarations would have to be private, so you get an error at compile time and not at link time.

If you’re only compiling with C++11, it should be safe to put the macro in public. Alternately, you can always delete the copy constructor and assignment yourself.

Thank you for the prompt clarification, Nicolas!

Another reason to prefer making these "public" is non-technical: they
affect fundamental aspects of the client interface of a class, and hiding
them in "private" as if they were implementation details is unhelpful.
Unfortunately necessary in C++98, but no longer needed with C++11.

-- James

Long before ‘deleted’ was added to the C++ language, reserving implicitly defined members as ‘private’ members was a very common pattern. Indeed, I can find many examples where I (and others) routinely used this pattern. For example, in from code I wrote in 1994 I have the following example (which I still use today – if it ain’t broke don’t fix it):

class MemoryPool {

private: // Interface: remove copying and address-of from the interface

MemoryPool ( const MemoryPool& );

MemoryPool& operator = ( const MemoryPool& );

MemoryPool* operator & ();

};

Without this pattern, these methods were implicitly defined whether you liked it or not; but with the consistent use of this pattern if these methods were referenced by a non-member there was a compile-time error (access constraint), and if a member accidentally referenced them there was a link-time error (undefined symbol), ‘deleted’ just elevated this to being also a compile-time error.

So aside from necessity in modern C++, I would expect that this pattern still persists, and that ‘deleted’ has been added (probably as a macro whose definition is dependent on C++ Standard version for portability) to such declarations to bring it up to date with the Standard, but they are almost certainly more likely to be ‘private’. This would make ‘private/deleted’ more likely than ‘public/deleted’, especially in portable legacy code-bases. The incredibly helpful ‘overloaded’ is often used in a similar way with a Standard dependent macro definition.

MartinO