Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -4,6 +4,7 @@ AvoidConstParamsInDecls.cpp BracesAroundStatementsCheck.cpp ContainerSizeEmptyCheck.cpp + DeletedDefaultCheck.cpp ElseAfterReturnCheck.cpp FunctionSizeCheck.cpp IdentifierNamingCheck.cpp Index: clang-tidy/readability/DeletedDefaultCheck.h =================================================================== --- clang-tidy/readability/DeletedDefaultCheck.h +++ clang-tidy/readability/DeletedDefaultCheck.h @@ -0,0 +1,36 @@ +//===--- DeletedDefaultCheck.h - clang-tidy----------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETED_DEFAULT_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETED_DEFAULT_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// Checks when a constructor or an assignment operator is marked as '= default' +/// but is actually deleted by the compiler. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-deleted-default.html +class DeletedDefaultCheck : public ClangTidyCheck { +public: + DeletedDefaultCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETED_DEFAULT_H Index: clang-tidy/readability/DeletedDefaultCheck.cpp =================================================================== --- clang-tidy/readability/DeletedDefaultCheck.cpp +++ clang-tidy/readability/DeletedDefaultCheck.cpp @@ -0,0 +1,102 @@ +//===--- DeletedDefaultCheck.cpp - clang-tidy------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "DeletedDefaultCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace { + AST_MATCHER(FunctionDecl, isExplicitlyDefaulted) { + return Node.isExplicitlyDefaulted(); +} +} // namespace +namespace tidy { +namespace readability { + +void DeletedDefaultCheck::registerMatchers(MatchFinder *Finder) { + const auto NotTemplate = unless(hasAncestor( + cxxRecordDecl(::clang::ast_matchers::isTemplateInstantiation()))); + + // We should actually use isExplicitlyDefaulted, but it does not exist. + Finder->addMatcher( + cxxConstructorDecl(isDefaultConstructor(), isExplicitlyDefaulted(), + isDeleted(), NotTemplate) + .bind("default-constructor"), + this); + Finder->addMatcher( + cxxConstructorDecl(isCopyConstructor(), isExplicitlyDefaulted(), + isDeleted(), NotTemplate) + .bind("copy-constructor"), + this); + Finder->addMatcher( + cxxConstructorDecl(isMoveConstructor(), isExplicitlyDefaulted(), + isDeleted(), NotTemplate) + .bind("move-constructor"), + this); + Finder->addMatcher( + cxxMethodDecl(isCopyAssignmentOperator(), isExplicitlyDefaulted(), + isDeleted(), NotTemplate) + .bind("copy-assignment"), + this); + Finder->addMatcher( + cxxMethodDecl(isMoveAssignmentOperator(), isExplicitlyDefaulted(), + isDeleted(), NotTemplate) + .bind("move-assignment"), + this); +} + +void DeletedDefaultCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto* DefaultConstructor = + Result.Nodes.getNodeAs("default-constructor")) { + diag(DefaultConstructor->getLocStart(), + "this default constructor is marked '= default' but is actually " + "implicitly deleted, probably because an instance variable or " + "a base class is lacking a default constructor. You should either " + "remove this definition or explicitly mark it as '= delete'"); + } + if (const auto* CopyConstructor = + Result.Nodes.getNodeAs("copy-constructor")) { + diag(CopyConstructor->getLocStart(), + "this copy constructor is marked '= default' but is actually " + "implicitly deleted, probably because an instance variable or a base " + "class is not copyable. You should either remove this definition or " + "explicitly mark it as '= delete'"); + } + if (const auto* MoveConstructor = + Result.Nodes.getNodeAs("move-constructor")) { + diag(MoveConstructor->getLocStart(), + "this move constructor is marked '= default' but is actually " + "implicitly deleted, probably because an instance variable or a base " + "class is not copyable nor movable. You should either remove this " + "definition or explicitly mark it as '= delete'"); + } + if (const auto* copy_assignment = + Result.Nodes.getNodeAs("copy-assignment")) { + diag(copy_assignment->getLocStart(), + "this copy assignment operator is marked '= default' but is actually " + "implicitly deleted, probably because an instance variable or a base " + "class is not assignable, e.g. because marked as const. You should " + "either remove this definition or explicitly mark it as '= delete'"); + } + if (const auto* MoveAssignment = + Result.Nodes.getNodeAs("move-assignment")) { + diag(MoveAssignment->getLocStart(), + "this move assignment operator is marked '= default' but is actually " + "implicitly deleted, probably because an instance variable or a base " + "class is not assignable, e.g. because marked as const. You should " + "either remove this definition or explicitly mark it as '= delete'"); + } +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -13,6 +13,7 @@ #include "AvoidConstParamsInDecls.h" #include "BracesAroundStatementsCheck.h" #include "ContainerSizeEmptyCheck.h" +#include "DeletedDefaultCheck.h" #include "ElseAfterReturnCheck.h" #include "FunctionSizeCheck.h" #include "IdentifierNamingCheck.h" @@ -40,6 +41,8 @@ "readability-braces-around-statements"); CheckFactories.registerCheck( "readability-container-size-empty"); + CheckFactories.registerCheck( + "readability-deleted-default"); CheckFactories.registerCheck( "readability-else-after-return"); CheckFactories.registerCheck( Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -98,6 +98,7 @@ readability-avoid-const-params-in-decls readability-braces-around-statements readability-container-size-empty + readability-deleted-default readability-else-after-return readability-function-size readability-identifier-naming Index: docs/clang-tidy/checks/readability-deleted-default.rst =================================================================== --- docs/clang-tidy/checks/readability-deleted-default.rst +++ docs/clang-tidy/checks/readability-deleted-default.rst @@ -0,0 +1,21 @@ +.. title:: clang-tidy - readability-deleted-default + +readability-deleted-default +=========================== + +Checks that constructors and assignment operators marked as ``= default`` are +not actually deleted by the compiler. + +.. code:: c++ + class Example { + public: + // This constructor is deleted because I is missing a default value. + Example() = default; + // This is fine. + Example(const Example& Other) = default; + // This operator is deleted because I cannot be assigned (it is const). + Example& operator=(const Example& Other) = default; + private: + const int I; + }; + Index: test/clang-tidy/readability-deleted-default.cpp =================================================================== --- test/clang-tidy/readability-deleted-default.cpp +++ test/clang-tidy/readability-deleted-default.cpp @@ -0,0 +1,103 @@ +// RUN: %check_clang_tidy %s readability-deleted-default %t + +class NoDefault { +public: + NoDefault() = delete; + NoDefault(NoDefault&& Other) = delete; + NoDefault(const NoDefault& Other) = delete; +}; + +class MissingEverything { +public: + MissingEverything() = default; + // CHECK-MESSAGES: warning: this default constructor is marked '= default' but is actually implicitly deleted + MissingEverything(MissingEverything&& Other) = default; + // CHECK-MESSAGES: warning: this move constructor is marked '= default' but is actually implicitly deleted + MissingEverything(const MissingEverything& Other) = default; + // CHECK-MESSAGES: warning: this copy constructor is marked '= default' but is actually implicitly deleted + MissingEverything& operator=(MissingEverything&& Other) = default; + // CHECK-MESSAGES: warning: this move assignment operator is marked '= default' but is actually implicitly deleted + MissingEverything& operator=(const MissingEverything& Other) = default; + // CHECK-MESSAGES: warning: this copy assignment operator is marked '= default' but is actually implicitly deleted + +private: + NoDefault ND; +}; + +class NotAssignable { +public: + NotAssignable(NotAssignable&& Other) = default; + NotAssignable(const NotAssignable& Other) = default; + NotAssignable& operator=(NotAssignable&& Other) = default; + // CHECK-MESSAGES: warning: this move assignment operator is marked '= default' but is actually implicitly deleted + NotAssignable& operator=(const NotAssignable& Other) = default; + // CHECK-MESSAGES: warning: this copy assignment operator is marked '= default' but is actually implicitly deleted + +private: + const int I = 0; +}; + +class Movable { +public: + Movable() = default; + Movable(Movable&& Other) = default; + Movable(const Movable& Other) = delete; + Movable& operator=(Movable&& Other) = default; + Movable& operator=(const Movable& Other) = delete; +}; + +class NotCopyable { +public: + NotCopyable(NotCopyable&& Other) = default; + NotCopyable(const NotCopyable& Other) = default; + // CHECK-MESSAGES: warning: this copy constructor is marked '= default' but is actually implicitly deleted + NotCopyable& operator=(NotCopyable&& Other) = default; + NotCopyable& operator=(const NotCopyable& Other) = default; + // CHECK-MESSAGES: warning: this copy assignment operator is marked '= default' but is actually implicitly deleted +private: + Movable M; +}; + +template +class Templated { +public: + // No warning here, it is a templated class. + Templated() = default; + Templated(Templated&& Other) = default; + Templated(const Templated& Other) = default; + Templated& operator=(Templated&& Other) = default; + Templated& operator=(const Templated& Other) = default; + + class InnerTemplated { + public: + // This class is not in itself templated, but we still don't have warning. + InnerTemplated() = default; + InnerTemplated(InnerTemplated&& Other) = default; + InnerTemplated(const InnerTemplated& Other) = default; + InnerTemplated& operator=(InnerTemplated&& Other) = default; + InnerTemplated& operator=(const InnerTemplated& Other) = default; + private: + T TVar; + }; + + class InnerNotTemplated { + public: + // This one could technically have warnings, but currently doesn't. + InnerNotTemplated() = default; + InnerNotTemplated(InnerNotTemplated&& Other) = default; + InnerNotTemplated(const InnerNotTemplated& Other) = default; + InnerNotTemplated& operator=(InnerNotTemplated&& Other) = default; + InnerNotTemplated& operator=(const InnerNotTemplated& Other) = default; + private: + int I; + }; + +private: + const T TVar{}; +}; + +void Foo() { + Templated V1; + Templated::InnerTemplated V2; + Templated::InnerNotTemplated V3; +}