Author: Carlos Galvez Date: 2022-08-11T07:46:04Z New Revision: 9ae5896d9673a54f4a6cf656624891bafc192857
URL: https://github.com/llvm/llvm-project/commit/9ae5896d9673a54f4a6cf656624891bafc192857 DIFF: https://github.com/llvm/llvm-project/commit/9ae5896d9673a54f4a6cf656624891bafc192857.diff LOG: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check Flags uses of const-qualified and reference data members in structs. Implements rule C.12 of C++ Core Guidelines. Differential Revision: https://reviews.llvm.org/D126880 Added: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp Modified: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp new file mode 100644 index 0000000000000..5f340736c8dde --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp @@ -0,0 +1,38 @@ +//===--- AvoidConstOrRefDataMembersCheck.cpp - clang-tidy -----------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "AvoidConstOrRefDataMembersCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +void AvoidConstOrRefDataMembersCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + fieldDecl(hasType(hasCanonicalType(referenceType()))).bind("ref"), this); + Finder->addMatcher( + fieldDecl(hasType(qualType(isConstQualified()))).bind("const"), this); +} + +void AvoidConstOrRefDataMembersCheck::check( + const MatchFinder::MatchResult &Result) { + if (const auto *MatchedDecl = Result.Nodes.getNodeAs<FieldDecl>("ref")) + diag(MatchedDecl->getLocation(), "member %0 of type %1 is a reference") + << MatchedDecl << MatchedDecl->getType(); + if (const auto *MatchedDecl = Result.Nodes.getNodeAs<FieldDecl>("const")) + diag(MatchedDecl->getLocation(), "member %0 of type %1 is const qualified") + << MatchedDecl << MatchedDecl->getType(); +} + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h new file mode 100644 index 0000000000000..7fd94aa7daa26 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h @@ -0,0 +1,38 @@ +//===--- AvoidConstOrRefDataMembersCheck.h - clang-tidy ---------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCONSTORREFDATAMEMBERSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCONSTORREFDATAMEMBERSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +/// Const-qualified or reference data members in classes should be avoided, as +/// they make the class non-copy-assignable. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.html +class AvoidConstOrRefDataMembersCheck : public ClangTidyCheck { +public: + AvoidConstOrRefDataMembersCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCONSTORREFDATAMEMBERSCHECK_H diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt index 745eeb8d49f7b..832b0bf9d9e5e 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS ) add_clang_library(clangTidyCppCoreGuidelinesModule + AvoidConstOrRefDataMembersCheck.cpp AvoidGotoCheck.cpp AvoidNonConstGlobalVariablesCheck.cpp CppCoreGuidelinesTidyModule.cpp diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp index 1c468b5cf0481..fd28d59980a7a 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -14,6 +14,7 @@ #include "../modernize/AvoidCArraysCheck.h" #include "../modernize/UseOverrideCheck.h" #include "../readability/MagicNumbersCheck.h" +#include "AvoidConstOrRefDataMembersCheck.h" #include "AvoidGotoCheck.h" #include "AvoidNonConstGlobalVariablesCheck.h" #include "InitVariablesCheck.h" @@ -47,6 +48,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule { void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck<modernize::AvoidCArraysCheck>( "cppcoreguidelines-avoid-c-arrays"); + CheckFactories.registerCheck<AvoidConstOrRefDataMembersCheck>( + "cppcoreguidelines-avoid-const-or-ref-data-members"); CheckFactories.registerCheck<AvoidGotoCheck>( "cppcoreguidelines-avoid-goto"); CheckFactories.registerCheck<readability::MagicNumbersCheck>( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 840e3aac32578..e5ba204ebe827 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -99,6 +99,11 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`cppcoreguidelines-avoid-const-or-ref-data-members + <clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members>` check. + + Warns when a struct or class uses const or reference (lvalue or rvalue) data members. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst new file mode 100644 index 0000000000000..b79872ad56d7d --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst @@ -0,0 +1,49 @@ +.. title:: clang-tidy - cppcoreguidelines-avoid-const-or-ref-data-members + +cppcoreguidelines-avoid-const-or-ref-data-members +================================================= + +This check warns when structs or classes have const-qualified or reference +(lvalue or rvalue) data members. Having such members is rarely useful, and +makes the class only copy-constructible but not copy-assignable. + +Examples: + +.. code-block:: c++ + + // Bad, const-qualified member + struct Const { + const int x; + } + + // Good: + class Foo { + public: + int get() const { return x; } + private: + int x; + }; + + // Bad, lvalue reference member + struct Ref { + int& x; + }; + + // Good: + struct Foo { + int* x; + std::unique_ptr<int> x; + std::shared_ptr<int> x; + gsl::not_null<int> x; + }; + + // Bad, rvalue reference member + struct RefRef { + int&& x; + }; + +The check implements +`rule C.12 of C++ Core Guidelines <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c12-dont-make-data-members-const-or-references>`_. + +Further reading: +`Data members: Never const <https://quuxplusone.github.io/blog/2022/01/23/dont-const-all-the-things/#data-members-never-const>`_. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index a7c6247ab96bf..7d2b8bbe44353 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -176,6 +176,7 @@ Clang-Tidy Checks `clang-analyzer-valist.Unterminated <clang-analyzer/valist.Unterminated.html>`_, `concurrency-mt-unsafe <concurrency/mt-unsafe.html>`_, `concurrency-thread-canceltype-asynchronous <concurrency/thread-canceltype-asynchronous.html>`_, + `cppcoreguidelines-avoid-const-or-ref-data-members <cppcoreguidelines/avoid-const-or-ref-data-members.html>`_, `cppcoreguidelines-avoid-goto <cppcoreguidelines/avoid-goto.html>`_, `cppcoreguidelines-avoid-non-const-global-variables <cppcoreguidelines/avoid-non-const-global-variables.html>`_, `cppcoreguidelines-init-variables <cppcoreguidelines/init-variables.html>`_, "Yes" diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp new file mode 100644 index 0000000000000..01aed9dfdc1e2 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp @@ -0,0 +1,169 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-const-or-ref-data-members %t +namespace std { +template <typename T> +struct unique_ptr {}; + +template <typename T> +struct shared_ptr {}; +} // namespace std + +namespace gsl { +template <typename T> +struct not_null {}; +} // namespace gsl + +struct Ok { + int i; + int *p; + const int *pc; + std::unique_ptr<int> up; + std::shared_ptr<int> sp; + gsl::not_null<int> n; +}; + +struct ConstMember { + const int c; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'const int' is const qualified [cppcoreguidelines-avoid-const-or-ref-data-members] +}; + +struct LvalueRefMember { + int &lr; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' of type 'int &' is a reference +}; + +struct ConstRefMember { + const int &cr; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' of type 'const int &' is a reference +}; + +struct RvalueRefMember { + int &&rr; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' of type 'int &&' is a reference +}; + +struct ConstAndRefMembers { + const int c; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'const int' is const qualified + int &lr; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' of type 'int &' is a reference + const int &cr; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' of type 'const int &' is a reference + int &&rr; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' of type 'int &&' is a reference +}; + +struct Foo {}; + +struct Ok2 { + Foo i; + Foo *p; + const Foo *pc; + std::unique_ptr<Foo> up; + std::shared_ptr<Foo> sp; + gsl::not_null<Foo> n; +}; + +struct ConstMember2 { + const Foo c; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'const Foo' is const qualified +}; + +struct LvalueRefMember2 { + Foo &lr; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' of type 'Foo &' is a reference +}; + +struct ConstRefMember2 { + const Foo &cr; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' of type 'const Foo &' is a reference +}; + +struct RvalueRefMember2 { + Foo &&rr; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' of type 'Foo &&' is a reference +}; + +struct ConstAndRefMembers2 { + const Foo c; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'const Foo' is const qualified + Foo &lr; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' of type 'Foo &' is a reference + const Foo &cr; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' of type 'const Foo &' is a reference + Foo &&rr; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' of type 'Foo &&' is a reference +}; + +using ConstType = const int; +using RefType = int &; +using ConstRefType = const int &; +using RefRefType = int &&; + +struct WithAlias { + ConstType c; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' of type 'ConstType' (aka 'const int') is const qualified + RefType lr; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: member 'lr' of type 'RefType' (aka 'int &') is a reference + ConstRefType cr; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: member 'cr' of type 'ConstRefType' (aka 'const int &') is a reference + RefRefType rr; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'rr' of type 'RefRefType' (aka 'int &&') is a reference +}; + +template <int N> +using Array = int[N]; + +struct ConstArrayMember { + const Array<1> c; + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: member 'c' of type 'const Array<1>' (aka 'const int[1]') is const qualified +}; + +struct LvalueRefArrayMember { + Array<2> &lr; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'lr' of type 'Array<2> &' (aka 'int (&)[2]') is a reference +}; + +struct ConstLvalueRefArrayMember { + const Array<3> &cr; + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: member 'cr' of type 'const Array<3> &' (aka 'const int (&)[3]') is a reference +}; + +struct RvalueRefArrayMember { + Array<4> &&rr; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'rr' of type 'Array<4> &&' (aka 'int (&&)[4]') is a reference +}; + +template <typename T> +struct TemplatedOk { + T t; +}; + +template <typename T> +struct TemplatedConst { + T t; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: member 't' of type 'const int' is const qualified +}; + +template <typename T> +struct TemplatedConstRef { + T t; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: member 't' of type 'const int &' is a reference +}; + +template <typename T> +struct TemplatedRefRef { + T t; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: member 't' of type 'int &&' is a reference +}; + +template <typename T> +struct TemplatedRef { + T t; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: member 't' of type 'int &' is a reference +}; + +TemplatedOk<int> t1{}; +TemplatedConst<const int> t2{123}; +TemplatedConstRef<const int &> t3{123}; +TemplatedRefRef<int &&> t4{123}; +TemplatedRef<int &> t5{t1.t}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits